VFONT: Text Selection Operator #106915
No reviewers
Labels
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
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#106915
Loading…
Reference in New Issue
No description provided.
Delete Branch "Harley/blender:TextObjectSelection"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
An operator to allow interactive text selection for 3D Text Objects.
This is from the code of Yash Dabhade (yashdabhade) for GSoC 2022
with corrections and simplifications. Also includes double-click for
word selection.
This started with Yash's code with Campbell's last review comments addressed, and simplified as much as possible. There are also some corrections so that it can no longer crash and it now handles changes to font size.
This also allows selection of words by double-clicking:
@ -780,2 +780,3 @@
bool *r_text_free,
struct CharTrans **r_chartransdata)
struct CharTrans **r_chartransdata,
float cursor_location[2],
Group return arguments last.
NOTE: I think it's a bit cleaner to add
VFontCursor_Params
struct which can be NULL. It can contain the 2D cursor location & the array offsetr_caret
. Normally I'd avoid a params being used for both input & output, but an exception could be made here as calculating the cursor placement is a special case and it avoids this function taking so many arguments (which is already getting a bit out of hand).@ -781,1 +781,3 @@
struct CharTrans **r_chartransdata)
struct CharTrans **r_chartransdata,
float cursor_location[2],
int *r_caret_pos)
The term
caret
is a bit awkward - not used elsewhere in Blender, I have a slight preference to rename tocursor_offset
orcursor_index
, and accept thatcursor_location
is a different kind of cursor location.@ -914,0 +916,4 @@
int caret_pos = -1;
float caret_pos_min_dist = 0;
if (cursor_location && ef != NULL && ob != NULL) {
cursor_location && ef != NULL && ob != NULL
is awkward. Prefer to use a boolean at the start of this function.Assertions for a correct state can be there too, instead of in the middle of the function.
const bool calc_cursor_offset = ... optional cursor arg(s) ... ;
Then reuse for the rest of the function, otherwise it's not clear if it's expected that
cursor_location
is set butr_caret
is null... (using a single cursor params arg would help, as suggested already).@ -1128,0 +1139,4 @@
if (cursor_location) {
float offset_loc[2] = {cursor_location[0] - xof, cursor_location[1] - yof};
float di = len_squared_v2(offset_loc);
These names aren't so helpful,
dist_sq_offset
, could be:dist_sq_cursor
.@ -1128,0 +1140,4 @@
if (cursor_location) {
float offset_loc[2] = {cursor_location[0] - xof, cursor_location[1] - yof};
float di = len_squared_v2(offset_loc);
float di2 = len_squared_v2(cursor_location);
Can't this be calculated once?
@ -1799,0 +1824,4 @@
ef = cu->editfont;
BLI_assert(ef->len >= 0);
cur_loc_pos = BKE_vfont_cursor_to_caret_pos(ob, curs_loc);
Early exit in the case input motion sets the cursor motion to the same position it was set to previously (avoids excessive updates for small mouse-motions).
Yes, that cleaned up very nicely.
I banished that word from here. Although that might be a nice refactor one day.
I sidestepped the issue by using "string_offset" instead.
@ -734,6 +734,14 @@ typedef struct VFontToCurveIter {
int status;
} VFontToCurveIter;
/* Used when translating a mouse cursor location to a position within the string. */
picky Prefer doxy-comments (
/** ... */
for struct members too), write in full sentences.@ -736,1 +736,4 @@
/* Used when translating a mouse cursor location to a position within the string. */
typedef struct VFontCursor_Params {
/* Mouse cursor location as input. */
Note what coordinate space this is in.
Note what coordinate space this is in.
@ -737,0 +738,4 @@
typedef struct VFontCursor_Params {
/* Mouse cursor location as input. */
float cursor_location[2];
/* string character position as output. */
Note what the offset is applied to (
EditFont::textbuf
, typically applied toEditFont::pos
& selection range).@ -780,2 +788,3 @@
bool *r_text_free,
struct CharTrans **r_chartransdata)
struct CharTrans **r_chartransdata,
struct VFontCursor_Params *cursor_params)
Order
r_
prefixed arguments last.@ -912,2 +921,4 @@
i = 0;
float cursor_min_dist = 0;
Naming makes it seem this distance isn't squared, I normally name these
*_best
&*_test
,cursor_sq_dist_best
for e.g.@ -914,0 +925,4 @@
float cursor_sq_dist = 0;
if (cursor_params) {
cursor_params->r_string_offset = -1;
cursor_sq_dist = len_squared_v2(cursor_params->cursor_location);
Why is this set to anything at all? besides
FLT_MAX or similar.
@ -1128,0 +1148,4 @@
float offset_loc[2] = {cursor_params->cursor_location[0] - xof,
cursor_params->cursor_location[1] - yof};
float cursor_sq_offset = len_squared_v2(offset_loc);
if (cursor_sq_dist <= cursor_min_dist && cursor_sq_dist < cursor_sq_offset) {
This block could use some explanation, I'm not sure why it's necessary to reset the cursor offset when finding the closest cursor position.
It would seem to make sense to first set the cursor offset to an invalid value (-1) and the distance to FLT_MAX, then to simply calculate to closest cursor position. There may be good reason not to do this, but in that case the reason should be noted in comments.
@ -1799,0 +1824,4 @@
ef = cu->editfont;
BLI_assert(ef->len >= 0);
cur_loc_pos = BKE_vfont_cursor_to_string_offset(ob, curs_loc);
picky can set a
const int
here.@ -1799,0 +1871,4 @@
static int font_selection_set_modal(bContext *C, wmOperator *UNUSED(op), const wmEvent *event)
{
switch (event->type) {
case LEFTMOUSE: // release
May as well check value is release in this case.
Did so.
Yes, indeed works with just FLT_MAX
I did manage to simplify it a bit more to just the needed parts, added some comments on the behavior but otherwise couldn't do much more with it.
Fixed a small bug that was hard to notice. If the very first touch down was on the left side of the first character this would not set and show the cursor. Subsequent interaction would work and and select that first character, just not that first touch down. But fixed.
@ -73,2 +73,4 @@
struct CharTrans **r_chartransdata);
bool BKE_vfont_to_curve_nubase(struct Object *ob, int mode, struct ListBase *r_nubase);
int BKE_vfont_cursor_to_string_offset(struct Object *ob, float cursor_location[2]);
Naming could still be clearer, both input and output are cursors and both can have offsets.
The term
string
could be text renamed totext
, as this is what the edit-font member is called.Suggest:
BKE_vfont_cursor_to_text_index
to make it clear the output is an index in an array (any coordinate space can have an offset).@ -914,0 +933,4 @@
float cursor_sq_dist_test = 0;
if (cursor_params) {
cursor_params->r_string_offset = -1;
cursor_sq_dist_test = len_squared_v2(cursor_params->cursor_location);
Why is this distance relevant? From what I can see only the distance to the characters should have to be measured. The distance from the mouse cursor the objects center doesn't seem like it should be used.
Text boxes can offset text so the absolute distance to the object center doesn't seem relevant.
@ -1128,0 +1162,4 @@
}
else if (cursor_sq_dist_test < cursor_sq_dist_best) {
/* Left of text, including left-side of first character. */
cursor_params->r_string_offset = -1;
Checking if the cursor is to the left should probably not be done with distances (I'd guess this would fail with text-box offsets) and instead check if the cursor is actually to the left of all characters. Although I'm not even sure this is needed. Perhaps for an initial pass just get basics working and worry about moving the cursor outside text regions after since it could get quite involved with multiple text boxes.
@ -1799,0 +1806,4 @@
Curve *cu = obedit->data;
EditFont *ef = cu->editfont;
/* Calculate a plane from the text object's orientation. */
A utility function would be clearer,
font_cursor_text_index_from_event(...)
for e.g. to split out the plane-projection from the text offset logic.@ideasman42
Okay I officially give up trying to figure out what Yash was doing there, so dug out all the "squared distance" stuff. The code before the per-character loop is all removed and then I just wrote simpler code for finding the string offset. Could probably be made prettier but seems to test okay.
What it does not do though, and I miss a bit, is being able to start a drag to the left or right of all the text. This requires selection of the text itself which seems okay. Maybe is better since it doesn't involve clicks outside the text so won't interfere as much with 3D cursor placement?
@ -737,0 +741,4 @@
* This is an optional argument to `vfont_to_curve` for getting the text
* offset into the string at a mouse cursor location. Used for getting
* text cursor (caret) position or selection range.
* \{ */
Missed a closing
/** \} */
comment - both before and when the group ends.@ -911,6 +928,7 @@ static bool vfont_to_curve(Object *ob,
}
i = 0;
Can leave as-is.
@ -1796,6 +1796,148 @@ void FONT_OT_text_insert(wmOperatorType *ot)
/** \} */
static void font_cursor_text_index_from_event(bContext *C,
This can return the index from
BKE_vfont_cursor_to_text_index
as the caller doesn't need to know the intermediate value.@ -1799,0 +1820,4 @@
r_curs_loc[1] = mouse_loc[1] / cu->fsize;
}
/* -------------------------------------------------------------------- */
The group can enclose the utility function (doesn't seem needed to define a new group).
@ -1128,0 +1146,4 @@
if (cursor_params) {
float midw = (twidth / 2.0f);
if (cursor_params->cursor_location[1] >= yof - (0.25f * linedist) &&
This probably wont work with text-on-curve, suggest to avoid all bounding box checks unless there very well tested with different configurations. For initial working selection I don't think they're needed.
Requesting changes as key-map conflict with 3D cursor isn't resolved yet.
Got it! There was a fundamental flaw in Yash's approach of comparing against the mouse position within that loop that creates each character: After that loop is complete those character positions are translated by horizontal alignment, vertical alignment, and by Text on Curve.
This now just checks mouse position against each character's position after the translating is all done. So this works with all alignments and with Text on Curve.
WIP: VFONT: Text Selection Operatorto VFONT: Text Selection OperatorGetting a build error:
@ -5611,6 +5611,7 @@ def km_font(params):
{"properties": [("delta", 1)]}),
("font.change_character", {"type": 'DOWN_ARROW', "value": 'PRESS', "alt": True, "repeat": True},
{"properties": [("delta", -1)]}),
("font.select_word", {"type": 'LEFTMOUSE', "value": 'DOUBLE_CLICK'}, None),
Better make this part of the tools key-map too. Seems strange this would be accessible without regular selection.
@ -1262,0 +1269,4 @@
cursor='CROSSHAIR',
icon="ops.generic.select_box",
widget=None,
keymap=("3D View Tool: Text Select"),
This should be left an empty tuple for the name to be generated (as is done with other tools).