UI: Improve 3d text selection #109229

Manually merged
Campbell Barton merged 30 commits from guishe/blender:improve-3dtext-selection into main 2023-07-06 05:28:40 +02:00
Contributor

Improve 3D text selection & feedback when moving the mouse outside of the text.
Previously mouse selection only worked if the mouse was over the text, now there is more margin to select characters or lines.


Old behavior

New behavior

Improve 3D text selection & feedback when moving the mouse outside of the text. Previously mouse selection only worked if the mouse was over the text, now there is more margin to select characters or lines. ---- Old behavior <video src="/attachments/11370e5e-b247-4d3d-bbc5-1cc98dd8ff28" title="2023-06-30 17-40-38.mp4" controls></video> New behavior <video src="/attachments/38e0bfdb-6ead-4341-929b-b793430fa608" title="2023-06-30 17-40-38.mp4" controls></video>
Guillermo Venegas added 1 commit 2023-06-22 07:02:02 +02:00
Author
Contributor

Here other tests

Here other tests
Member

maybe @Harley is interested? :)

maybe @Harley is interested? :)
Iliya Katushenock added this to the Python API project 2023-06-22 09:58:31 +02:00
Iliya Katushenock added the
Interest
Text Editor
label 2023-06-22 09:58:35 +02:00
Iliya Katushenock modified the project from Python API to User Interface 2023-06-22 09:59:57 +02:00
Iliya Katushenock removed the
Interest
Text Editor
label 2023-06-22 10:00:01 +02:00
Harley Acheson requested review from Harley Acheson 2023-06-22 17:05:31 +02:00
Member

Hey, that is a nice improvement. Selections that start outside of the text block was something we wanted. Thanks for working on this.

There are a couple things that need improving though:

Increase the "Line Spacing" to something like 2.0. The selection of the line is too biased to the line below. As in a selection vertically between the lines should select the one above if slightly closer to it, or below if lower than the midpoint. Current code is not perfect either, but this PR is a bit worse.

LineSelection.gif

Similarly, when clicking on the right side of a character we want the cursor inserted to the right. This makes selection much more forgivable and is how it works elsewhere in the interface.

Midpoint.gif

With both of these it is probably just a matter of adding a small amount - like half the character width for the horizontal issue - to your tested values. Note that doing so can sometimes cause complications for the first character.

If you add a curve then set your "Text on Curve". This can make selection quite odd if outside the text block.

OnCurve.gif

This looks to be solvable. Clicking to the left of the block should only ever select the first character this might just need another check.

This last part seems mean. But if we move from requiring a click inside the block to allowing it outside users will give us bug reports if it doesn't work all over.

Hey, that is a nice improvement. Selections that start outside of the text block was something we wanted. Thanks for working on this. There are a couple things that need improving though: Increase the "Line Spacing" to something like 2.0. The selection of the line is too biased to the line below. As in a selection vertically between the lines should select the one above if slightly closer to it, or below if lower than the midpoint. Current code is not perfect either, but this PR is a bit worse. ![LineSelection.gif](/attachments/1ec8445f-a7fb-4309-ba19-866093952965) Similarly, when clicking on the right side of a character we want the cursor inserted to the right. This makes selection much more forgivable and is how it works elsewhere in the interface. ![Midpoint.gif](/attachments/11c31906-c15d-440c-a57d-da40179c26b8) With both of these it is probably just a matter of adding a small amount - like half the character width for the horizontal issue - to your tested values. Note that doing so can sometimes cause complications for the first character. If you add a curve then set your "Text on Curve". This can make selection quite odd if outside the text block. ![OnCurve.gif](/attachments/bdf2de2c-3b27-4788-b42c-90e5ea502d89) This looks to be solvable. Clicking to the left of the block should only ever select the first character this might just need another check. This last part seems mean. But if we move from requiring a click inside the block to allowing it outside users will give us bug reports if it doesn't work all over.
Guillermo Venegas added 1 commit 2023-06-22 22:18:06 +02:00
Guillermo Venegas added 1 commit 2023-06-22 22:57:29 +02:00
Author
Contributor

I made a change so that when textoncurve is assigned it looks for the closest character instead, I don't think it's the ideal solution, I'm going to analyze a better way to do it

In the video is assigned a circle curve

I made a change so that when textoncurve is assigned it looks for the closest character instead, I don't think it's the ideal solution, I'm going to analyze a better way to do it In the video is assigned a circle curve
Member

Hey, that is getting much better!

With normal text and character spacing set to 1.0 the placement of the cursor is perfect. Clicking on the right side of an "M" or "i" behaves exactly as expected. But increase the character spacing to 1.5 and it doesn't go to the right at the right side of the letter but the right side of the total space.

Spacing.gif

Vertical line selection looks a bit better but still seems out. Although to be honest is probably close to how it is in current code. This seems picky but it can affect how it feels for multiline selections. You select from one line then drag down to include something from one below. Where the change occurs from single to multiline should feel right.

Multiline.gif

Text on a curve seems good enough to me. As long as selecting outside the text block doesn't make it worse I wouldn't worry about things like a spiral of text. Granted I didn't test much so you might be seeing a problem I haven't noticed yet.

Hey, that is getting **much better**! With normal text and character spacing set to 1.0 the placement of the cursor is perfect. Clicking on the right side of an "M" or "i" behaves exactly as expected. But increase the character spacing to 1.5 and it doesn't go to the right at the right side of the letter but the right side of the total space. ![Spacing.gif](/attachments/ac2da172-4863-4d54-8f23-3a26e61ab844) Vertical line selection looks a bit better but still seems out. Although to be honest is probably close to how it is in current code. This seems picky but it can affect how it feels for multiline selections. You select from one line then drag down to include something from one below. Where the change occurs from single to multiline should feel right. ![Multiline.gif](/attachments/12623e0a-3706-4fcb-9ddf-8e5efeb36cec) Text on a curve seems good enough to me. As long as selecting outside the text block doesn't make it worse I wouldn't worry about things like a spiral of text. Granted I didn't test much so you might be seeing a problem I haven't noticed yet.
Guillermo Venegas added 4 commits 2023-06-23 23:07:29 +02:00
Author
Contributor

Now its use the actual size of the of the character to decide if the cursor is on its left or right.
between characters

Selection between different lines now uses the half-size space between lines.
between lines with space greater than 1
between lines with space 1

Something to take into consideration is that the selection is not clamped if the mouse is above or below the text, if I select from above the text it will not necessarily select from position 0, should I change it?
from top selection

Selection on curve seems fine to me
selection text on curve

Now its use the actual size of the of the character to decide if the cursor is on its left or right. [between characters](https://projects.blender.org/attachments/dea80645-6b13-4143-8f7b-1fb9478922aa) Selection between different lines now uses the half-size space between lines. [between lines with space greater than 1](https://projects.blender.org/attachments/bb19077f-dfe4-4d8c-ad53-0d81908dec61) [between lines with space 1](https://projects.blender.org/attachments/6e7786a0-9fed-464c-80bc-f0160c9beaa5) Something to take into consideration is that the selection is not clamped if the mouse is above or below the text, if I select from above the text it will not necessarily select from position 0, should I change it? [from top selection](https://projects.blender.org/attachments/9f6c8f21-7d53-42f9-8783-66ce5703b93f) Selection on curve seems fine to me [selection text on curve](https://projects.blender.org/attachments/4e8238f2-daf6-4d00-b6a4-e90a33c1c38d)
Author
Contributor

Selection between lines seems strange because we can't really tell where lines end or start.

That can be fixed if the selection highlight includes that region.

Selection between lines seems strange because we can't really tell where lines end or start. That can be fixed if the selection highlight includes that region.
Guillermo Venegas added 1 commit 2023-06-23 23:25:18 +02:00
Member

Now its use the actual size of the of the character to decide if the cursor is on its left or right.

That looks perfect. The character selection works nicely no matter the character width or spacing.

Selection between different lines now uses the half-size space between lines.

Yes, that works way better than current code.

Something to take into consideration is that the selection is not clamped if the mouse is above or below the text, if I select from above the text it will not necessarily select from position 0, should I change it?

I'm not sure. I don't have any programs that allow selection outside of the text area so I can't find any precedents for this. It does feel a bit odd to drag my mouse horizontally above the text area and have it select text. Not sure if it feels like a bug or a feature. On balance maybe a feature? Dragging like this above the text is similar to dragging between lines of text when the line spacing is high. I'd say leave this how it is. Once this seems complete and working well we can give it to Campbell to look at and he might say otherwise.

A thing that seems to have broken between your versions is the selection the line when clicking to the right of the text area. Clicking to the left correctly places the cursor at the beginning of the nearest line. But clicking to the right seems to select the line below:

LineSelect.gif

> Now its use the actual size of the of the character to decide if the cursor is on its left or right. That looks perfect. The character selection works nicely no matter the character width or spacing. > Selection between different lines now uses the half-size space between lines. Yes, that works way better than current code. > Something to take into consideration is that the selection is not clamped if the mouse is above or below the text, if I select from above the text it will not necessarily select from position 0, should I change it? I'm not sure. I don't have any programs that allow selection outside of the text area so I can't find any precedents for this. It does feel a bit odd to drag my mouse horizontally above the text area and have it select text. Not sure if it feels like a bug or a feature. On balance maybe a feature? Dragging like this above the text is similar to dragging between lines of text when the line spacing is high. I'd say leave this how it is. Once this seems complete and working well we can give it to Campbell to look at and he might say otherwise. A thing that seems to have broken between your versions is the selection the line when clicking to the right of the text area. Clicking to the left correctly places the cursor at the beginning of the nearest line. But clicking to the right seems to select the line below: ![LineSelect.gif](/attachments/55e918f0-d61d-421f-a829-b5ec30cbb3a5)
Guillermo Venegas added 1 commit 2023-06-24 01:28:00 +02:00
Author
Contributor

Fixed.

Testing in the last line, it can be confusing that you can select from before the first character on the last line like this

Fixed. Testing in the last line, it can be confusing that you can select from before the first character on the last line like this
Member

Testing in the last line, it can be confusing that you can select from before the first character on the last line like this

If this feels strange then we could consider doing this like the only thing that is close to it...

Right now this PR treats a drag from above the text to the middle of the first line as setting the start of the selection at that point. Basically just setting the cursor position and anchoring the new selection from there as you drag further.

We don't have any exact precedents to this in other programs, but we do experience something that feels close. When any text editor has any blank lines before and after the block you are selecting from. In this case dragging from above and to the middle of the first line will select from the start of the line to the cursor position. However, doing so from below selects from the end of the line to the cursor position.

TextAboveBelow.gif

Of course it isn't the exact same situation in that the selection is actually starting from the line above or below, but might be what our brain is expecting. This PR as it is right now does behave exactly like this if you add a blank line above and below, so feels funny when it acts differently without those blank lines.

> Testing in the last line, it can be confusing that you can select from before the first character on the last line like this If this feels strange then we could consider doing this like the only thing that is close to it... Right now this PR treats a drag from above the text to the middle of the first line as setting the start of the selection at that point. Basically just setting the cursor position and anchoring the new selection from there as you drag further. We don't have any exact precedents to this in other programs, but we do experience something that feels close. When any text editor has any blank lines before and after the block you are selecting from. In this case dragging from above and to the middle of the first line will select from the start of the line to the cursor position. However, doing so from below selects from the end of the line to the cursor position. ![TextAboveBelow.gif](/attachments/b5172143-8f1d-466c-9f32-a74bc457e1d3) Of course it isn't the exact same situation in that the selection is actually starting from the line above or below, but might be what our brain is expecting. This PR as it is right now does behave exactly like this if you add a blank line above and below, so feels funny when it acts differently without those blank lines.
Author
Contributor

Blender text editor behaves the same way, it should be fine

Blender text editor behaves the same way, it should be fine
Guillermo Venegas added 1 commit 2023-06-24 23:13:45 +02:00
Member

@guishe - Blender text editor behaves the same way, it should be fine

For sure. I quite like how it works right now in this PR. A great improvement.

Code-wise, should change your comments to c-style "/* */". I know this file has some uses of single-line c++ style "//" but I'd treat those as being in error.

Your code has three blocks that all depend on cursor_params. I'd personally enclose all of it in a if (cursor_params).." and then have additional tests inside for slen == 0``, 'cu->textoncurve`, etc. This would make it a bit clearer to me that all the code is for this one purpose.

The "0.915f" is very specific; definitely needs a comment even if just a trial and error guess. Although when testing I get a nicer result with this:

    const float interline_offset = ((linedist - 0.5f) / 2.0f) * font_size;
    // Loop until find the line where the mouse is over
    for (i = 0; i <= slen; i++, ct++) {
      if (cursor_params->cursor_location[1] >=
          ((chartransdata[i].yof * font_size) - interline_offset))
      {
        break;
      }
    }

This gives me a perfect 0.25 offset when the line-spacing is 1.0f, but still works great over 1.0f. It also has a really nice transition when the line-spacing is less than 1.0 and the selection boxes overlap.

> @guishe - Blender text editor behaves the same way, it should be fine For sure. I quite like how it works right now in this PR. A great improvement. Code-wise, should change your comments to c-style "/* */". I know this file has some uses of single-line c++ style "//" but I'd treat those as being in error. Your code has three blocks that all depend on `cursor_params`. I'd personally enclose all of it in a `if (cursor_params).." and then have additional tests inside for `slen == 0``, 'cu->textoncurve`, etc. This would make it a bit clearer to me that all the code is for this one purpose. The "0.915f" is very specific; definitely needs a comment even if just a trial and error guess. Although when testing I get a nicer result with this: ```Cpp const float interline_offset = ((linedist - 0.5f) / 2.0f) * font_size; // Loop until find the line where the mouse is over for (i = 0; i <= slen; i++, ct++) { if (cursor_params->cursor_location[1] >= ((chartransdata[i].yof * font_size) - interline_offset)) { break; } } ``` This gives me a perfect 0.25 offset when the line-spacing is 1.0f, but still works great over 1.0f. It also has a really nice transition when the line-spacing is less than 1.0 and the selection boxes overlap.
First-time contributor

@guishe hi, i suggestion, in windows we can select the whole line by 3 clicks .. i hope to see this in 3d text

  • another improvement is to be able to select words in different lines by using the usual shortcut Shift and drag to select the second part and the others like in microsoft word...
@guishe hi, i suggestion, in windows we can select the whole line by 3 clicks .. i hope to see this in 3d text + another improvement is to be able to select words in different lines by using the usual shortcut Shift and drag to select the second part and the others like in microsoft word...
Guillermo Venegas added 1 commit 2023-06-28 04:36:49 +02:00
Guillermo Venegas added 1 commit 2023-06-28 04:37:32 +02:00
Author
Contributor

The "0.915f" is very specific; definitely needs a comment even if just a trial and error guess.

I did this by visually testing how much space between lines made it look like they barely touched the selection borders, but your solution is better

added also a fix, before the scale was not taken into account when it was with a curve

> The "0.915f" is very specific; definitely needs a comment even if just a trial and error guess. I did this by visually testing how much space between lines made it look like they barely touched the selection borders, but your solution is better added also a fix, before the scale was not taken into account when it was with a curve
Guillermo Venegas added 1 commit 2023-06-28 04:41:42 +02:00
Member

The behavior here is such a nice improvement.

This is probably a perfect time to bring Campbell in to take a look.

The behavior here is such a nice improvement. This is probably a perfect time to bring Campbell in to take a look.
Harley Acheson requested review from Campbell Barton 2023-06-29 21:33:00 +02:00
Harley Acheson approved these changes 2023-06-29 21:33:11 +02:00
Member

@ideasman42 - This is such a nice improvement.

This mostly adds the ability to start a selection outside of the text area, and that greatly improves selection since you don't have to start off so precisely. It also improves on the location of the transition point between lines when selecting multiples. Because text on a curve is handled separately it also supports very complex arrangements better.

@ideasman42 - This is such a nice improvement. This _mostly_ adds the ability to start a selection outside of the text area, and that greatly improves selection since you don't have to start off so precisely. It also improves on the location of the transition point between lines when selecting multiples. Because text on a curve is handled separately it also supports very complex arrangements better.
Campbell Barton requested changes 2023-06-30 02:35:09 +02:00
Campbell Barton left a comment
Owner

This causes selection not to work when multiple text boxes are used.

Supporting this might be reasonably involved as text boxes can overlap, so simply picking the text box under the cursor wont always work properly.

A possible solution could be to use this new method of selection but compute it for each text-box, then pick the closest match.

This causes selection not to work when multiple text boxes are used. Supporting this might be reasonably involved as text boxes can overlap, so simply picking the text box under the cursor wont always work properly. A possible solution could be to use this new method of selection but compute it for each text-box, then pick the closest match.
Member

This causes selection not to work when multiple text boxes are used.

With this patch applied to main and with two text boxes arranged vertically in order I see it working well:

TwoBoxes1.gif

But if the second box is moved above the first then we can't select in the second box, which is something that works in current code, but breaks here:

TwoBoxes2.gif

> This causes selection not to work when multiple text boxes are used. With this patch applied to main and with two text boxes arranged vertically **in order** I see it working well: ![TwoBoxes1.gif](/attachments/e65bdd0e-8216-47ca-acfd-90b197fd19a2) But if the second box is moved above the first then we can't select in the second box, which is something that works in current code, but breaks here: ![TwoBoxes2.gif](/attachments/1bb21626-3752-4a44-be89-8ce94c0ca055)
Member

@guishe:

The breaking of selection when the second text box is above the first is just because your "Loop until find the line where the mouse is over" is only checking for the cursor location being greater than the bottom of the text and interline offset. It works if you check if it is in the range between the top and bottom. Without testing much I get close to proper behavior with this:

      for (i = 0; i <= slen; i++, ct++) {
        float bottom = (chartransdata[i].yof * font_size) - interline_offset;
        float top = bottom + linedist;
        if ((cursor_params->cursor_location[1] >= bottom) &&
            (cursor_params->cursor_location[1] <= top))
        {
          break;
        }
      }

With above I get selection working as it does today, when you start within the text blocks. It also works as you want when starting to the right or left of the text boxes. But selecting above or below either box will always select the last line of the second box if it is above the first.

@guishe: The breaking of selection when the second text box is above the first is just because your "Loop until find the line where the mouse is over" is only checking for the cursor location being _greater_ than the bottom of the text and interline offset. It works if you check if it is in the range between the top and bottom. Without testing much I get close to proper behavior with this: ```Cpp for (i = 0; i <= slen; i++, ct++) { float bottom = (chartransdata[i].yof * font_size) - interline_offset; float top = bottom + linedist; if ((cursor_params->cursor_location[1] >= bottom) && (cursor_params->cursor_location[1] <= top)) { break; } } ``` With above I get selection working as it does today, when you _start within the text blocks_. It also works as you want when starting to the right or left of the text boxes. But selecting above or below either box will always select the last line of the second box if it is above the first.
Guillermo Venegas force-pushed improve-3dtext-selection from 4ed13db0e2 to 4ec8ad653f 2023-06-30 21:45:29 +02:00 Compare
Author
Contributor

I update and now gets the nearest char if cu->totbox > 1.

@Harley i will give it a try

I update and now gets the nearest char if `cu->totbox > 1`. @Harley i will give it a try
Author
Contributor

this happens if the cursor is above the text
video

this happens if the cursor is above the text [video](/attachments/37ecdd69-f0fe-4300-bd99-b44554ed7e35)
Author
Contributor

with the closest character
video

but with the closest char interline selection can be a bit off
video

with the closest character [video](/attachments/07948480-8afd-4a2c-837f-3b6ee13480a2) but with the closest char interline selection can be a bit off [video](/attachments/6ef792b7-5434-45e1-9e72-2e87c967cefd)
Guillermo Venegas added 1 commit 2023-06-30 22:48:05 +02:00
Member

Actually.... I am having great results with only the following (diff from main):

diff --git a/source/blender/blenkernel/intern/vfont.c b/source/blender/blenkernel/intern/vfont.c
index 05e20d5245c..c80087ad32e 100644
--- a/source/blender/blenkernel/intern/vfont.c
+++ b/source/blender/blenkernel/intern/vfont.c
@@ -1749,33 +1749,21 @@ static bool vfont_to_curve(Object *ob,
 
   if (cursor_params) {
     cursor_params->r_string_offset = -1;
-    for (i = 0; i <= slen; i++, ct++) {
-      info = &custrinfo[i];
-      ascii = mem[i];
-      if (info->flag & CU_CHINFO_SMALLCAPS_CHECK) {
-        ascii = towupper(ascii);
-      }
-      ct = &chartransdata[i];
-      che = find_vfont_char(vfd, ascii);
-      float charwidth = char_width(cu, che, info);
-      float charhalf = (charwidth / 2.0f);
-      if (cursor_params->cursor_location[1] >= (ct->yof - (0.25f * linedist)) * font_size &&
-          cursor_params->cursor_location[1] <= (ct->yof + (0.75f * linedist)) * font_size)
-      {
-        /* On this row. */
-        if (cursor_params->cursor_location[0] >= (ct->xof * font_size) &&
-            cursor_params->cursor_location[0] <= ((ct->xof + charhalf) * font_size))
-        {
-          /* Left half of character. */
-          cursor_params->r_string_offset = i;
-        }
-        else if (cursor_params->cursor_location[0] >= ((ct->xof + charhalf) * font_size) &&
-                 cursor_params->cursor_location[0] <= ((ct->xof + charwidth) * font_size))
-        {
-          /* Right half of character. */
-          cursor_params->r_string_offset = i + 1;
+    if (slen > 0) {
+      int best_match = -1;
+      float closest_distance = FLT_MAX;
+      for (i = 0; i <= slen; i++) {
+        const float char_location[2] = {
+            chartransdata[i].xof * font_size,
+            chartransdata[i].yof * font_size + (font_size * 0.3f),
+        };
+        const float distance = len_squared_v2v2(cursor_params->cursor_location, char_location);
+        if (closest_distance > distance) {
+          best_match = i;
+          closest_distance = distance;
         }
       }
+      cursor_params->r_string_offset = best_match;
     }
   }
Actually.... I am having great results with only the following (diff from main): ``` diff --git a/source/blender/blenkernel/intern/vfont.c b/source/blender/blenkernel/intern/vfont.c index 05e20d5245c..c80087ad32e 100644 --- a/source/blender/blenkernel/intern/vfont.c +++ b/source/blender/blenkernel/intern/vfont.c @@ -1749,33 +1749,21 @@ static bool vfont_to_curve(Object *ob, if (cursor_params) { cursor_params->r_string_offset = -1; - for (i = 0; i <= slen; i++, ct++) { - info = &custrinfo[i]; - ascii = mem[i]; - if (info->flag & CU_CHINFO_SMALLCAPS_CHECK) { - ascii = towupper(ascii); - } - ct = &chartransdata[i]; - che = find_vfont_char(vfd, ascii); - float charwidth = char_width(cu, che, info); - float charhalf = (charwidth / 2.0f); - if (cursor_params->cursor_location[1] >= (ct->yof - (0.25f * linedist)) * font_size && - cursor_params->cursor_location[1] <= (ct->yof + (0.75f * linedist)) * font_size) - { - /* On this row. */ - if (cursor_params->cursor_location[0] >= (ct->xof * font_size) && - cursor_params->cursor_location[0] <= ((ct->xof + charhalf) * font_size)) - { - /* Left half of character. */ - cursor_params->r_string_offset = i; - } - else if (cursor_params->cursor_location[0] >= ((ct->xof + charhalf) * font_size) && - cursor_params->cursor_location[0] <= ((ct->xof + charwidth) * font_size)) - { - /* Right half of character. */ - cursor_params->r_string_offset = i + 1; + if (slen > 0) { + int best_match = -1; + float closest_distance = FLT_MAX; + for (i = 0; i <= slen; i++) { + const float char_location[2] = { + chartransdata[i].xof * font_size, + chartransdata[i].yof * font_size + (font_size * 0.3f), + }; + const float distance = len_squared_v2v2(cursor_params->cursor_location, char_location); + if (closest_distance > distance) { + best_match = i; + closest_distance = distance; } } + cursor_params->r_string_offset = best_match; } } ```
Author
Contributor

As it is now, if we check that there are no curves or multiple boxes, it is easier to select lines if the mouse is to the right of the text.

But I don't know if that can be a bit confusing, why can't do the same on text objects with curves or multiple text boxes?

As it is now, if we check that there are no curves or multiple boxes, it is easier to select lines if the mouse is to the right of the text. <video src="https://projects.blender.org/attachments/7027e6a5-59d0-4fcc-8105-000ed0be10ed" title="2023-06-30 13-58-54.mp4" controls></video> But I don't know if that can be a bit confusing, why can't do the same on text objects with curves or multiple text boxes?
Member

I couldn't find anything not working well with just the single block of code for everything I tested with single blocks, multiple text frames, text on curve, etc. Selecting from the right, left, top, or bottom all worked great.

I couldn't find anything not working well with just the single block of code for everything I tested with single blocks, multiple text frames, text on curve, etc. Selecting from the right, left, top, or bottom all worked great.
Author
Contributor

This type of selection on a simple 3D text object with no curve or multiple boxes, it's nice to be able to select a line without getting too close to it.

This happens if the mouse is too much far to the right whit getting the colesest one.

This type of selection on a simple 3D text object with no curve or multiple boxes, it's nice to be able to select a line without getting too close to it. <video src="/attachments/9e52a550-d89c-4128-a4b9-cd0484cd495b" title="2023-06-30 17-40-38.mp4" controls></video> This happens if the mouse is too much far to the right whit getting the colesest one. <video src="/attachments/97f5476e-51ea-461a-ba27-d47424adb80a" title="2023-06-30 17-43-49.mp4" controls></video>
Member

Yikes, yes I was not testing with blocks of text that include any lines that were short enough from their neighbors to show that behavior.

When in that short-distance loop and you think you've found the new shortest one, could you then check that the vertical position of cursor_params->cursor_location is within the range of char_location? If not don't make it best match. That way you only get a best match on the same line?

Yikes, yes I was not testing with blocks of text that include any lines that were short enough from their neighbors to show that behavior. When in that short-distance loop and you think you've found the new shortest one, could you then check that the vertical position of cursor_params->cursor_location is within the range of char_location? If not don't make it best match. That way you only get a best match on the same line?
Member

What I'm now testing with is the idea of finding both the closest character to the cursor as well as the closest character on the same line as the cursor. Then just using the closest one on the same line if there is one, or the nearest character if not.

So far it seems to do what you want, selecting the correct line if clicking on the right, and the correct character if clicking above or below.

diff --git a/source/blender/blenkernel/intern/vfont.c b/source/blender/blenkernel/intern/vfont.c
index 05e20d5245c..97342086563 100644
--- a/source/blender/blenkernel/intern/vfont.c
+++ b/source/blender/blenkernel/intern/vfont.c
@@ -1749,33 +1749,37 @@ static bool vfont_to_curve(Object *ob,
 
   if (cursor_params) {
     cursor_params->r_string_offset = -1;
-    for (i = 0; i <= slen; i++, ct++) {
-      info = &custrinfo[i];
-      ascii = mem[i];
-      if (info->flag & CU_CHINFO_SMALLCAPS_CHECK) {
-        ascii = towupper(ascii);
-      }
-      ct = &chartransdata[i];
-      che = find_vfont_char(vfd, ascii);
-      float charwidth = char_width(cu, che, info);
-      float charhalf = (charwidth / 2.0f);
-      if (cursor_params->cursor_location[1] >= (ct->yof - (0.25f * linedist)) * font_size &&
-          cursor_params->cursor_location[1] <= (ct->yof + (0.75f * linedist)) * font_size)
-      {
-        /* On this row. */
-        if (cursor_params->cursor_location[0] >= (ct->xof * font_size) &&
-            cursor_params->cursor_location[0] <= ((ct->xof + charhalf) * font_size))
-        {
-          /* Left half of character. */
-          cursor_params->r_string_offset = i;
+    if (slen > 0) {
+      int best_match = -1;
+      int best_match_line = -1;
+      float closest_distance = FLT_MAX;
+      float closest_distance_line = FLT_MAX;
+      const float interline_offset = ((linedist - 0.5f) / 2.0f) * font_size;
+      for (i = 0; i <= slen; i++) {
+        const float char_location[2] = {
+            chartransdata[i].xof * font_size,
+            chartransdata[i].yof * font_size + (font_size * 0.3f),
+        };
+
+        /* Find closest of all characters to this location. */
+        const float distance = len_squared_v2v2(cursor_params->cursor_location, char_location);
+        if (distance < closest_distance) {
+          best_match = i;
+          closest_distance = distance;
         }
-        else if (cursor_params->cursor_location[0] >= ((ct->xof + charhalf) * font_size) &&
-                 cursor_params->cursor_location[0] <= ((ct->xof + charwidth) * font_size))
+
+        /* Find closest character on this line. */
+        const float interline_offset = ((linedist - 0.5f) / 2.0f) * font_size;
+        float bottom = (chartransdata[i].yof * font_size) - interline_offset;
+        float top = bottom + linedist;
+        if (distance < closest_distance_line && (cursor_params->cursor_location[1] < top) &&
+            (cursor_params->cursor_location[1] > bottom))
         {
-          /* Right half of character. */
-          cursor_params->r_string_offset = i + 1;
+          best_match_line = i;
+          closest_distance_line = distance;
         }
       }
+      cursor_params->r_string_offset = (best_match_line != -1) ? best_match_line : best_match;
     }
   }
What I'm now testing with is the idea of finding both the closest character to the cursor as well as the closest character _on the same line as the cursor_. Then just using the closest one on the same line if there is one, or the nearest character if not. So far it seems to do what you want, selecting the correct line if clicking on the right, and the correct character if clicking above or below. ``` diff --git a/source/blender/blenkernel/intern/vfont.c b/source/blender/blenkernel/intern/vfont.c index 05e20d5245c..97342086563 100644 --- a/source/blender/blenkernel/intern/vfont.c +++ b/source/blender/blenkernel/intern/vfont.c @@ -1749,33 +1749,37 @@ static bool vfont_to_curve(Object *ob, if (cursor_params) { cursor_params->r_string_offset = -1; - for (i = 0; i <= slen; i++, ct++) { - info = &custrinfo[i]; - ascii = mem[i]; - if (info->flag & CU_CHINFO_SMALLCAPS_CHECK) { - ascii = towupper(ascii); - } - ct = &chartransdata[i]; - che = find_vfont_char(vfd, ascii); - float charwidth = char_width(cu, che, info); - float charhalf = (charwidth / 2.0f); - if (cursor_params->cursor_location[1] >= (ct->yof - (0.25f * linedist)) * font_size && - cursor_params->cursor_location[1] <= (ct->yof + (0.75f * linedist)) * font_size) - { - /* On this row. */ - if (cursor_params->cursor_location[0] >= (ct->xof * font_size) && - cursor_params->cursor_location[0] <= ((ct->xof + charhalf) * font_size)) - { - /* Left half of character. */ - cursor_params->r_string_offset = i; + if (slen > 0) { + int best_match = -1; + int best_match_line = -1; + float closest_distance = FLT_MAX; + float closest_distance_line = FLT_MAX; + const float interline_offset = ((linedist - 0.5f) / 2.0f) * font_size; + for (i = 0; i <= slen; i++) { + const float char_location[2] = { + chartransdata[i].xof * font_size, + chartransdata[i].yof * font_size + (font_size * 0.3f), + }; + + /* Find closest of all characters to this location. */ + const float distance = len_squared_v2v2(cursor_params->cursor_location, char_location); + if (distance < closest_distance) { + best_match = i; + closest_distance = distance; } - else if (cursor_params->cursor_location[0] >= ((ct->xof + charhalf) * font_size) && - cursor_params->cursor_location[0] <= ((ct->xof + charwidth) * font_size)) + + /* Find closest character on this line. */ + const float interline_offset = ((linedist - 0.5f) / 2.0f) * font_size; + float bottom = (chartransdata[i].yof * font_size) - interline_offset; + float top = bottom + linedist; + if (distance < closest_distance_line && (cursor_params->cursor_location[1] < top) && + (cursor_params->cursor_location[1] > bottom)) { - /* Right half of character. */ - cursor_params->r_string_offset = i + 1; + best_match_line = i; + closest_distance_line = distance; } } + cursor_params->r_string_offset = (best_match_line != -1) ? best_match_line : best_match; } } ```
Guillermo Venegas added 1 commit 2023-07-01 18:05:05 +02:00
Guillermo Venegas added 1 commit 2023-07-01 18:06:44 +02:00
Guillermo Venegas added 2 commits 2023-07-01 18:46:00 +02:00
Member

That's working quite well now, even when multiple text boxes are overlapping.

That's working quite well now, even when multiple text boxes are overlapping.
Guillermo Venegas requested review from Campbell Barton 2023-07-01 23:26:46 +02:00
Guillermo Venegas added 2 commits 2023-07-01 23:26:47 +02:00
Guillermo Venegas added 1 commit 2023-07-01 23:57:54 +02:00
Campbell Barton requested changes 2023-07-04 04:47:50 +02:00
Campbell Barton left a comment
Owner

This function now runs some quite involved logic when it's unnecessary.

  • When the cursor isn't being placed, cursor structures shouldn't be created.
  • Advanced text-box selection should only run when there are 2 or more text-boxes.
This function now runs some quite involved logic when it's unnecessary. - When the cursor isn't being placed, cursor structures shouldn't be created. - Advanced text-box selection should only run when there are 2 or more text-boxes.
@ -16,10 +16,12 @@
#include "MEM_guardedalloc.h"
#include "BLI_blenlib.h"

Prefer including only necessary headers.

Prefer including only necessary headers.
guishe marked this conversation as resolved
@ -790,6 +792,16 @@ static float vfont_descent(const VFontData *vfd)
return vfd->em_height - vfont_ascent(vfd);
}
struct TextboxBounds {

This reads as a general structure when it's a special case for cursor placement.

This should be named in a way that reflects that, e.g. TextBoxBounds_Cursor.

This reads as a general structure when it's a special case for cursor placement. This should be named in a way that reflects that, e.g. `TextBoxBounds_Cursor`.
guishe marked this conversation as resolved
@ -934,6 +946,16 @@ static bool vfont_to_curve(Object *ob,
}
i = 0;
struct TextboxBounds *tb_bounds = MEM_malloc_arrayN(

This is allocated even when the cursor isn't being placed.

This is allocated even when the cursor isn't being placed.
guishe marked this conversation as resolved
Guillermo Venegas added 2 commits 2023-07-04 22:19:03 +02:00
Guillermo Venegas requested review from Campbell Barton 2023-07-04 22:21:44 +02:00
Campbell Barton requested changes 2023-07-05 02:50:48 +02:00
Campbell Barton left a comment
Owner

Noticed some issues still,

Text boxes with zero height aren't handled properly (I could attach a blend file, but it's fairly easy to redo with 2x text boxes and the first one is zero height).

There are some logical issues too, noted inline.

Attached a patch that resolves warnings and accounts for some text boxes having no text (these are skipped).

Noticed some issues still, Text boxes with zero height aren't handled properly (I could attach a blend file, but it's fairly easy to redo with 2x text boxes and the first one is zero height). There are some logical issues too, noted inline. Attached a patch that resolves warnings and accounts for some text boxes having no text (these are skipped).
@ -1073,2 +1091,4 @@
CLAMP_MIN(maxlen, lineinfo[lnr].x_min);
if (tb_bounds != NULL) {
tb_bounds[curbox].xmax = max_ff(tb_bounds[curbox].xmax, xof);

This doesn't look right, the value is initialized at zero, shouldn't it be initialized to -FLT_MAX so the initial value doesn't impact the range? (boxes may be placed below zero).

This doesn't look right, the value is initialized at zero, shouldn't it be initialized to `-FLT_MAX` so the initial value doesn't impact the range? (boxes may be placed below zero).
@ -1074,1 +1092,4 @@
if (tb_bounds != NULL) {
tb_bounds[curbox].xmax = max_ff(tb_bounds[curbox].xmax, xof);
tb_bounds[curbox].ymin = yof;

Worth noting in a comment that a min_ff(..) isn't needed as the value only ever increases.

Worth noting in a comment that a `min_ff(..)` isn't needed as the value only ever increases.
@ -1778,1 +1843,4 @@
}
/* Loop until find the first character to the right of the mouse (using the character
* midpoint on the x-axis as a reference). */
for (i; i <= end && yof == chartransdata[i].yof; i++) {

i does nothing (removed in patch, attached).

`i` does nothing (removed in patch, attached).
Guillermo Venegas added 3 commits 2023-07-05 20:20:09 +02:00
Guillermo Venegas added 1 commit 2023-07-05 20:22:40 +02:00
Guillermo Venegas reviewed 2023-07-05 20:28:52 +02:00
@ -1334,0 +1363,4 @@
if (tb_bounds_for_cursor != NULL) {
for (curbox = 0; curbox < cu->totbox; curbox++) {
TextBoxBounds_ForCursor *tb_bounds = &tb_bounds_for_cursor[curbox];
if (tb_bounds->last_char_index == -1) {
Author
Contributor

It was also necessary to wait for the text to be aligned, now the bounding box is calculated after that

It was also necessary to wait for the text to be aligned, now the bounding box is calculated after that
Guillermo Venegas added 1 commit 2023-07-05 22:09:05 +02:00
Campbell Barton requested changes 2023-07-06 03:32:35 +02:00
Campbell Barton left a comment
Owner

There is an issue where the previous text boxes last index could be used even if that text box is empty, the attached patch resolves this, it also:

  • Moves the bounds into local space, to be more straightforward (having the TextBox and it's bounds be in the un-scaled coordinates of the cursor_location).
  • Additional doc-strings, comments.
  • Replace ternary operator which assign value to it's self with an if-statement.
  • Replace ternary operators with min_ii(..).
  • Variable renaming.
There is an issue where the previous text boxes last index could be used even if that text box is empty, the attached patch resolves this, it also: - Moves the `bounds` into local space, to be more straightforward (having the TextBox and it's bounds be in the un-scaled coordinates of the `cursor_location`). - Additional doc-strings, comments. - Replace ternary operator which assign value to it's self with an if-statement. - Replace ternary operators with min_ii(..). - Variable renaming.
Guillermo Venegas added 1 commit 2023-07-06 04:38:49 +02:00
Author
Contributor

Can't reproduce having in between a textbox with 0 characters (maybe if there is just a break line can make that feeling of a empty text-box), only last textboxes will have 0 chars

Having height = 0, the textbox will have no line limit, even with a very small value, the textbox will have at least one line, only the last few textboxes will contain 0 characters

image

Can't reproduce having in between a textbox with 0 characters (maybe if there is just a break line can make that feeling of a empty text-box), only last textboxes will have 0 chars Having height = 0, the textbox will have no line limit, even with a very small value, the textbox will have at least one line, only the last few textboxes will contain 0 characters ![image](/attachments/9db750fe-b71b-4ca2-817a-c153c763b6fc)
672 KiB

I didn't check if it caused a problem in practice but the possibility of using an empty text boxes char_index_last which would be -1 could cause bugs, so it's better to track the last-valid-index, not just pick the value of the last index and assume it's valid.

I didn't check if it caused a problem in practice but the possibility of using an empty text boxes `char_index_last` which would be `-1` could cause bugs, so it's better to track the last-valid-index, not just pick the value of the last index and assume it's valid.
Campbell Barton approved these changes 2023-07-06 05:28:16 +02:00
Campbell Barton manually merged commit 5b3ce7b740 into main 2023-07-06 05:28:40 +02:00
Guillermo Venegas deleted branch improve-3dtext-selection 2023-07-06 18:02:40 +02:00
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
5 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#109229
No description provided.