feat: enhance text editing capabilities
closes: #226 #227 #288
Added several enhancements to the text-editing features:
- Added the ability to move the cursor by clicking with the mouse
- Added text selection (via multiple methods, covering both keyboard and mouse)
- Added cross-line movement and selection support(When the cursor moves to a new line, it is inserted using the character index instead of the byte index.)
- Added copy and paste functionality (including primary clipboard)
- Added a border highlight for the current text range
- Adjusted the auto-wrapping logic so that it matches the image boundaries and is no longer affected by canvas resizing
Since the middle mouse button is already used for panning, primary clipboard can only be triggered with
Shift+Insert. BecauseCtrl+Cis used to copy the image to the clipboard, text copy is changed toCtrl+Shift+C. Since reading the clipboard is an asynchronous operation, aSender<SketchBoardInput>was introduced to the tool to send refresh notifications.This will affect all types implementing thepub trait Tool.
https://github.com/user-attachments/assets/47bbe1b2-ef0a-40e5-bd70-cc98bdd2eed0
Thanks for picking this up. This will take a bit to review.
I have three pre-review remarks. Maybe they're nonsense, and maybe I'll change my opinion later. ;)
- I'm thinking that when Text tool is active, personally I wouldn't expect Ctrl+C to trigger an image copy. So IMO it's fine if Ctrl+C in text does text copy and overrides image copy. @gabm @fabienjuif do you guys have any opinion on this?
- I'm leaning towards Shift+Insert for the primary clipboard.
- I've not investigated any further, when I add a manual new line, the whole tool becomes sluggish, even after leaving the text tool. I observed something similar with certain font settings way earlier, so it might depend on the font choice. Have you observed this behaviour?
I'm thinking that when Text tool is active, personally I wouldn't expect Ctrl+C to trigger an image copy. So IMO it's fine if Ctrl+C in text does text copy and overrides image copy. @gabm @fabienjuif do you guys have any opinion on this?
Agreed, I also expect Ctrl+C to perform text copying immediately. Since events are bubbling currently, we likely need to implement event propagation blocking.Or maybe I haven't discovered it yet.
I'm leaning towards Shift+Insert for the primary clipboard.
Should we use Shift+Insert instead of Insert, or add it as a new shortcut?
I've not investigated any further, when I add a manual new line, the whole tool becomes sluggish, even after leaving the text tool. I observed something similar with certain font settings way earlier, so it might depend on the font choice. Have you observed this behaviour?
No.This hasn't happened to me before.Perhaps we need more test results.
I'm leaning towards Shift+Insert for the primary clipboard.
Should we use
Shift+Insertinstead ofInsert, or add it as a new shortcut?
This would be a replacement. Shift+Insert is the combination I got in muscle memory, it works in many editors and terminals. I'm fairly sure it's not consistently linked to the PRIMARY clipboard, but we wouldn't be alone using it that way. foot is one example where it works exactly like this, just to name one.
I think we could also use Ctrl+Insert to copy to Primary, which again works in some locations. But we could always consider adding that later, so no pressure here please ;)
AFAICT, standalone Insert itself is rather linked to toggle overwrite/insert in editors, which might be counter-intuitive for at least some users if we use it to paste, hence the replacement.
I've not investigated any further, when I add a manual new line, the whole tool becomes sluggish, even after leaving the text tool. I observed something similar with certain font settings way earlier, so it might depend on the font choice. Have you observed this behaviour?
No.This hasn't happened to me before.Perhaps we need more test results.
Yeah let's see if anyone else can comment on this. But I'll also try to investigate this further when I get a chance, it would be good to understand what's happening.
Added the feature to stop event propagation.
Map Ctrl+C to copy (instead of Ctrl+Shift+C).
Map Middle-Click to paste the primary clipboard.(Currently, middle-click pastes the primary clipboard into active editor, regardless of where the click occurs)
https://github.com/user-attachments/assets/778a7731-6bcd-4d35-a706-fee90d5b307b
About Stop Propogation
Added two states to ToolUpdateResult for stopping event propagation:
StopPropagationRedrawAndStopPropagation
Apologies for the newly introduced conflicts. I can help resolve them when we get closer to merging.
Here's some remarks for now:
- The lagging seems related to font selection, I don't really see an issue with e.g. monospace. I think we can leave it like that and troubleshoot elsewhere separately
- The box lags a bit if I press Shift+Enter twice
- The box remains when switching to a different tool via mouse
- With the latest change, Enter/Esc from within text editing don't seem to do anything any longer
This is still not the actual code review, which will take more time. I'm sorry for that.
Got it. I’ll take a look when I have time.
The box lags a bit if I press Shift+Enter twice
What does lag look like?
I don't have this situation. Maybe it's also related to the font?
Could you tell me which font you used?
- The box remains when switching to a different tool via mouse
- With the latest change, Enter/Esc from within text editing don't seem to do anything any longer
Fixed
The box lags a bit if I press Shift+Enter twice
What does lag look like?
I don't have this situation. Maybe it's also related to the font?
Could you tell me which font you used?
I used "lag" in two different contexts, sorry for the lack of clarity.
Boundary box "lagging behind" double new line
This is not "lag" like the second item below, and perhaps I worded this poorly. When pressing enter, the box isn't immediately enclosing the cursor. Perhaps the recording below shows it.
Considering the line above "Added a border highlight for the current text range" this might actually work as intended?
https://github.com/user-attachments/assets/87bbfcd4-0ff0-492a-a95e-50bae8c26994
Sluggishness after entering multiline text
It's most apparent when I use
[font]
family="Roboto"
style="Regular"
which should be available because we ship it, but I have not double checked in any way if the font is actually being loaded. If I swap this to monospace, the effect is still present to a certain degree, but Satty doesn't become unusable and if I didn't know about it I probably wouldn't have noticed. Using Arial, I see no issues at all.
Recording below: Before entering text, drawing things and panning responds fine. I enter a single line of text and both are still fine. Then I enter text with multiple new lines, and afterwards drawing and panning gets quite unresponsive. Adding multiple single line texts does not cause this, so it seems to be specifically related to newlines.
This looks a bit like #244 where a missing font caused a performance decline. If I may speculate, I think the issue is not new new, and we may end up having to address it outside of this PR, but something might amplify it.
https://github.com/user-attachments/assets/a8fc7434-3031-4f18-9acc-5cf2460e4ef1
Regarding the review, I don't think I'll have any code specific changes, but I think we could use a few more comments. I'd like to suggest a few places, but I'm afraid I won't find much time next week.
Boundary box "lagging behind" double new line
Currently, the final line break is not being captured. Perhaps we need to include it in the selection?
Sluggishness after entering multiline text
In my case, I tried the Roboto font, and there's no noticeable difference compared to other fonts. However, I noticed that when there's more text content, Satty starts to lag. I suspect it might be a general rendering performance issue, but it becomes more apparent with text rendering.
Maybe we need a new way to render canvas.
Currently, the final line break is not being captured. Perhaps we need to include it in the selection?
I'm undecided if there's a clear "right" or "wrong" way to do this. I think it would look better, but of course I'm saying that now without actually having seen it.
Put it this way, if you think including the line break is better, and it's not too tricky to do, then feel free to change it. Otherwise I'm happy to leave it as it is and see if anyone ever feels like opening an issue for it. If anything, it's cosmetic.
Sluggishness after entering multiline text
In my case, I tried the Roboto font, and there's no noticeable difference compared to other fonts. However, I noticed that when there's more text content, Satty starts to lag. I suspect it might be a general rendering performance issue, but it becomes more apparent with text rendering.
Maybe we need a new way to render canvas.
Yeah, maybe. I'm still hoping to understand better what is happening here, maybe we're trying to load fonts too often, and this leads to additional processing of the existing text if any. Perhaps I can have a closer look on the back of #298.
Each redraw invokes a complete render cycle, generating considerable computational overhead.
Each redraw invokes a complete render cycle, generating considerable computational overhead.
I think we can optimise that, I'd like to get a better understanding myself of what we do vs what femtovg does in their examples.
That said, I just pushed a commit to PR #298 that removed repeated font loading from ensure_canvas and tried the change on top of this branch. I think this is quite the difference.
I think what I'm saying is that once #298 is merged we don't have a problem here.
https://github.com/user-attachments/assets/1696df57-21ca-43db-bfee-9eb2fd7056ba
Thanks for the PR :)