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 text editing for layouts which contain inline boxes #299

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

Conversation

valadaptive
Copy link
Contributor

@valadaptive valadaptive commented Mar 15, 2025

This PR includes #296, since they both touch the selection-drawing code.

There were a couple issues preventing text cursor positioning and selection from working on lines with inline boxes:

  • Cluster::from_point and Cluster::from_byte_index were setting the run_index in their cluster paths to the index of the nth line item that is a run. It should be set to the nth line item, regardless of whether the previous items are inline boxes.
  • Cluster::from_point and Selection::geometry_with were ignoring inline boxes when accumulating the lines' advance widths.

It's not perfect yet. Cursor affinity should take inline boxes into account, but since we could have multiple inline boxes in a row, or inline boxes next to soft line breaks, the cursor affinity will need to be an index and not just an upstream/downstream state (how do other APIs handle this?) That's a much bigger undertaking for a later time.

There was also a discrepancy between the documentation for inline boxes and the implementation--InlineBox::index is documented as the byte index to insert the box at, whereas the shaping code treated it as a character index. I've changed the shaping code to treat it as a byte index, but I could change the documentation instead.

@valadaptive valadaptive force-pushed the inline-box-editing branch 2 times, most recently from 6f8a94a to 3759c15 Compare March 15, 2025 07:04
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.

1 participant