Fixed the positioning of cursor in Textbox: no approximation #24065
Conversation
There was a problem hiding this comment.
All the logic as well as caching should be done (lazily) in the Text object and provided via a function self.disp_text.char_index_at(x) or similar, where x is a relative coordinate inside the text bbox.
It's the responsibility of the text object to do this. Note that a user could do text_box.disp_text.set_fontsize(100), which needs to invalidate the char_size cache, but the text_box itself does not see when this gets called.
|
Hi @timhoffm , I tried to follow your suggestions, let me know what you think about it! edit: There are still problems when I change the fontsize after creation, trying to solve them |
|
Now it works in cases like this one: but there is still a problem related to How can the correct sizes of the boundingboxes be found? edit: I notice that the renderer changes throughout the execution with The solutions I see right now it's dropping the caching, but this still doesn't change the fact that the coordinates are incorrectly calculated before showing the figure or It would be even good in my opinion rolling back to the first change so that the method would belong just to the Textbox object so that no problem of this sort would appear |
There was a problem hiding this comment.
There can still be some edge cases like ligatures or letter spacing, where the position is not just the sum of character widths. However, I believe they are negligible in practical use cases. This approach is better than the previous guessing, and I think good enough for the use case.
|
Thanks @timhoffm for the review, I applied your suggested changes, everything should be good to go! |
65a9a9b to
971d59e
Compare
In the process a new function for the Text object called _char_index_at has been created Co-authored-by: Tortar <68152031+Tortar@users.noreply.github.com> Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
|
I see some edge cases (a Ctrl-c at the wrong time won't reset the text) and an un-bounded cache, but I think that both of those are low enough risk to merge this as-is and address if they actually are a problem. |
PR Summary
I updated the position_cursor function of Textbox so that it doesn't approximate the position of the cursor incorrectly anymore. Now, instead, to calculate the cursor's position, we accumulate the width of each character contained in the text and find the index where the distance between the click position and the accumulated sum is minimal.
I found that the approximation used in the position_cursor function before this PR was really buggy sometimes, running this code block and trying to set the cursor in different positions makes it apparent :
For example, to put the cursor at the start of the second block of "k"s you needed to click one letter forward, while with the change it works as one would expect.
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).