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

Use binary search in SkiaParagraph.lineMetricsForOffset #934

Merged
merged 1 commit into from Dec 11, 2023

Conversation

m-sasha
Copy link

@m-sasha m-sasha commented Dec 11, 2023

Proposed Changes

Use binary search in SkiaParagraph.lineMetricsForOffset. Currently the search is linear.

Testing

Test: Ran the user-provided reproducer. Also tested manually that caret is placed correctly when clicking in a multi-line textfield.

Issues Fixed

Fixes: JetBrains/compose-multiplatform#4021

@m-sasha
Copy link
Author

m-sasha commented Dec 11, 2023

I changed List<T>.fastBinarySearch from commonMain to be internal (was private before), to avoid reimplementing binary search.

@m-sasha m-sasha force-pushed the m-sasha/fast-lineMetricsForOffset branch from 44a40dc to f9dade7 Compare December 11, 2023 11:56
Copy link
Member

@MatkovIvan MatkovIvan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to have more unit tests for this part (now I see only DesktopParagraphTest.getLineForOffset), but the fix itself lgtm

@m-sasha m-sasha merged commit 976f6fe into jb-main Dec 11, 2023
4 checks passed
@m-sasha m-sasha deleted the m-sasha/fast-lineMetricsForOffset branch December 11, 2023 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants