Page MenuHome

UI: Widget Text Set Cursor Position
Needs RevisionPublic

Authored by Harley Acheson (harley) on Tue, Nov 26, 3:54 AM.
Subscribers
None
Tokens
"Like" token, awarded by Tetone."Love" token, awarded by HooglyBoogly."Like" token, awarded by billreynish.

Details

Summary

We can click our mouse into an existing number or text input to set a new insertion point (text caret). It is currently not very predicable on where that insertion point will go. It might go to the right or left of where you click but might not go to where you expected it to.

The following illustrates the the problem and the changes here, with the top showing current behavior, and the bottom after this patch. Green areas, when clicked will move the caret to the left. Red areas will move the selection to the right.

The reason that this happens is because the current code ignores the width of each letter but uses a fixed-width nudge to set the left-right division. This patch properly uses character width and an estimated inter-character space to get closer to ideal.

Note that the Text Editor already behaves properly as above. That editor uses a fixed-width font so the position of character center would always be the same.

Diff Detail

Repository
rB Blender

Event Timeline

Campbell Barton (campbellbarton) requested changes to this revision.EditedTue, Dec 3, 7:50 AM

I found removing the current offsets gives results closer to the middle of each character:

diff --git a/source/blender/editors/interface/interface_handlers.c b/source/blender/editors/interface/interface_handlers.c
index b34188684e6..4902410757d 100644
--- a/source/blender/editors/interface/interface_handlers.c
+++ b/source/blender/editors/interface/interface_handlers.c
@@ -2858,8 +2858,7 @@ static void ui_textedit_set_cursor_pos(uiBut *but, uiHandleButtonData *data, con
       startx += UI_DPI_ICON_SIZE / aspect;
     }
   }
-  /* But this extra .05 makes clicks in between characters feel nicer. */
-  startx += ((UI_TEXT_MARGIN_X + 0.05f) * U.widget_unit) / aspect;
+  startx += (UI_TEXT_MARGIN_X * U.widget_unit) / aspect;
 
   /* mouse dragged outside the widget to the left */
   if (x < startx) {
@@ -2870,7 +2869,7 @@ static void ui_textedit_set_cursor_pos(uiBut *but, uiHandleButtonData *data, con
     while (i > 0) {
       if (BLI_str_cursor_step_prev_utf8(str, but->ofs, &i)) {
         /* 0.25 == scale factor for less sensitivity */
-        if (BLF_width(fstyle->uifont_id, str + i, (str_last - str) - i) > (startx - x) * 0.25f) {
+        if (BLF_width(fstyle->uifont_id, str + i, (str_last - str) - i) > (startx - x)) {
           break;
         }
       }

However using the with of the string is limited.

For *correct* results that take the bounding box of each character into account, we would need a to add a function to the BLF API,

We could have general function that that takes a callback which receives the character-boundbox, the string, the string offset in bytes,
which keeps being called as long as the callback is returning true.

This would simplify the code which needs to keep track of cdist, cdist_prev, removing the loop and having a single call which pin-points the space between two characters.

source/blender/editors/interface/interface_handlers.c
2863

Relying on the size of a single character could be error prone, depending too much on the font in use.

This revision now requires changes to proceed.Tue, Dec 3, 7:50 AM

@Campbell Barton (campbellbarton) - I found removing the current offsets gives results closer to the middle of each character:

That does help a bit, but I'm testing in extreme cases. My biggest target is in placing the caret properly when clicking on the right side of the "i" in "MiM"

However using the width of the string is limited. For *correct* results that take the bounding box of each character into account, we would need a to add a function to the BLF API,

I think I specifically need to consider glyph width (or just bearing) as apposed to character length which is a bounding box that includes both glyph width and bearings. I'm fairly certain that is the problem with my little "i" above, in that it is half glyph and half bearings. Could be wrong.

Might explore being able to get a character's glyph width, glyph height, bearings, advance, pitch, etc. Might come in handy every so often.

We could have general function that that takes a callback which receives the character-boundbox, the string, the string offset in bytes,
which keeps being called as long as the callback is returning true.
This would simplify the code which needs to keep track of cdist, cdist_prev, removing the loop and having a single call which pin-points the space between two characters.

That does sound cool. Will look into that for sure.