Pointer and cursor misalignment in widget.Entry

[adrianre12]

Describe the bug:

I noticed that selecting around "i" is difficult in Entry. Looking at the code I can see a mistake

func (e *Entry) cursorColAt(text []rune, pos fyne.Position) int {
	for i := 0; i < len(text); i++ {
		str := string(text[0 : i+1])
		wid := fyne.MeasureText(str, theme.TextSize(), e.textStyle()).Width + theme.Padding()
		if wid > pos.X+theme.Padding() {
			return i
		}
	}
	return len(text)
}

themePadding() is being added to wid and added to pos.X in the if statement. This effectively cancels out theme.Padding()
It should only be added to wid and not added in the if statement.

To Reproduce:

Steps to reproduce the behaviour:
In the Demo select the Entry tab and enter "wiiw" then repeatedly click along the first "w". You will see that the cursor will jump to before the first "i" 1/2 way along the "w".
Place your pointer between the two "ii" and click, the cursor will jump to after the second "i"
Slowly move the pointer back over the first "i" while clicking. The cursor will jump to the beginning of the first "i" when the pointer is over the second half of the "w"

With the theme.Padding() removed from if statement

		if wid > pos.X {

The cursor will jump to where the pointer is. i.e. if it is between the "ii" then it will be placed between them.

Device (please complete the following information):

• OS: Windows
• Version: 10
• Go version: 1.15.6
• Fyne version: 2.0

[andydotxyz]

I think the method is more wrong than you realise. Can you try the following replacement and see what you think?

func (e *Entry) cursorColAt(text []rune, pos fyne.Position) int {
	for i := 0; i < len(text); i++ {
		str := string(text[0 : i])
		wid := fyne.MeasureText(str, theme.TextSize(), e.textStyle()).Width
		charWid := fyne.MeasureText(string(text[i]), theme.TextSize(), e.textStyle()).Width
		if pos.X < theme.Padding()*2 + wid + (charWid/2) {
			return i
		}
	}
	return len(text)
}

[adrianre12]

It is OK and I think we should go with it.
I can see why my fix was wrong and yours is technically correct, but I think there is one other thing at play here too, the kerning.
When clicking through the "w" the cursor jump does not happen 1/2 way through the character but on the downslope of the second "v" of the "w". If you take into account the Kerning, it is 1/2 way between the start of the "w" and "i".
The coincidence of 1 padding being nearly the same width as the kerning is why my change appeared correct and if I change yours to only have 1 * padding instead of 2 * padding it feels better. Of course, this could go horribly wrong at different font size or padding.

[andydotxyz]

If you change the offset by theme.Padding() does it not mess up tapping between the "i"s?

[adrianre12]

I now have a magnifying glass out to see it. the each 'i' and space between the 'ii' on my 1920x1080 monitor is 2 pixels. The pointer vertical is 1 pixel wide.
With 2 * padding, the first kern pixel puts the cursor before the first 'i' and the second pixel puts the cursor between the 'ii'
With 1 * padding, the space before the first 'i', to the space between the 'ii' puts the cursor between the 'ii'.
If anything the 1* padding improves getting a hit between the 'ii' but makes the selecting before the 'ii' slightly harder.

I tried if pos.X < theme.Padding()*2+wid+(charWid/2)-1 { which makes it perfect for me, but who knows if it would be right on a screen with different resolution or with different font size.
I think how you have it now is good enough, how does it work for you?

[andydotxyz]

Using theme.Padding()*2 I tested at scale=4 and it seemed almost exactly right.
This does imply a kerning calculation and "-1px" might be the best of all the options, but I'm not sure it's worth the additional calcualtions.

FYNE_SCALE=4 go run .

[andydotxyz]

I placed that fix on release/v2.0.x for testing :)