Skip to content

core: Fix double caret rendering in justified text #17066

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

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

kjarosh
Copy link
Member

@kjarosh kjarosh commented Jul 9, 2024

Progresses #1891.

Before After

@kjarosh kjarosh added text Issues relating to text rendering/input waiting-on-review Waiting on review from a Ruffle team member labels Jul 9, 2024
@torokati44
Copy link
Member

torokati44 commented Jul 10, 2024

My finger is hovering over the merge button, but the fact that I don't understand the change (or even the code it changes) completely, keeps me from pressing it.
Would you mind explaining in a sentence or two how the logic produced two (and not more) carets to be rendered, and how the change results in at most one being rendered, @kjarosh? 🤔

The condition

  visible_selection.start() >= *start && visible_selection.end() <= *end

was inaccurate, because the end of the selection is exclusive.
That caused the condition to be true for two adjacent boxes.
For instance:

  box 1: from 0 to 6,  "hello "
  box 2: from 6 to 11, "world"

The caret was rendered for both boxes when it was at position 6.

When applying a correct condition (i.e. treating the end as exclusive)
there is a problem with rendering the caret at the very end of the text,
because the condition will not be triggered for any box
(position 11 in the example above).

That is why a condition specific to this case is added, i.e.

  *end == text_len

When the box is the last box in the text, we are forcing
the caret to be rendered.
@kjarosh
Copy link
Member Author

kjarosh commented Jul 10, 2024

Added a description to the commit message which describes the fix.

The condition

visible_selection.start() >= *start && visible_selection.end() <= *end

was inaccurate, because the end of the selection is exclusive. That caused the condition to be true for two adjacent boxes. For instance:

  box 1: from 0 to 6,  "hello "
  box 2: from 6 to 11, "world"

The caret was rendered for both boxes when it was at position 6.

When applying a correct condition (i.e. treating the end as exclusive) there is a problem with rendering the caret at the very end of the text, because the condition will not be triggered for any box (position 11 in the example above).

That is why a condition specific to this case is added, i.e.

*end == text_len

When the box is the last box in the text, we are forcing the caret to be rendered.

@torokati44
Copy link
Member

Oookay, it all makes sense now, thanks!

@torokati44 torokati44 merged commit ac9b39a into ruffle-rs:master Jul 10, 2024
17 checks passed
@kjarosh kjarosh deleted the double-caret branch July 11, 2024 14:25
@kjarosh kjarosh removed the waiting-on-review Waiting on review from a Ruffle team member label Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
text Issues relating to text rendering/input
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants