Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1041: Wrap long lines based on a settable maximum line width #1355

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hwoongkang
Copy link
Contributor

Previously, rendering glyphs in the scene depended solely on glyphLines property, which originates from text entry of side panel.

And I recognized that to handle alignments, advance width of glyphs and total line lengths were already being tracked.

By using stack and pushing the "remaining line" on the top of the stack when the line length exceeds certain threshold, it was possible to implement the line wrapping algorithm.

--

But the problem is that currently "selection API" depends on "indices", and various codes accessing the current selected glyphs use these line and glyph indices to get glyph info out of glyphLines property.

This leads to a bug:

  • the text I used is:
abcdeabcdeabcdeabcdeabcdeabcde
cdefg
  • it gets wrapped as:
abcdeabcdeabcdeabcde
abcdeabcde
cdefg

when I select b from the "wrapped second line", and edit it, d gets edited because it is the second glyph of the second line, with respect to glyphLines

Therefore I handled this case by changing related codes to use positionedLines instead of glyphLines. Since type signatures of two data differs, some transforming is done. This is what second commit is about.

But it feels too much "ad hoc" for me, so any suggestions or reviews will be very helpful

@justvanrossum
Copy link
Collaborator

Thank you for your proposal. I see the problem with the indices, but I will have to think more how to resolve it.

@justvanrossum
Copy link
Collaborator

Sorry for letting this sit for so long.

I think the right solution requires more work: the selection should not be a lineIndex/glyphIndex duo, but a single index into the full input string (where a newline is simply a character). We then need an efficient way to find the selected positionedGlyph in the positionedLines, and also the reverse, taking dynamic line breaking into account.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants