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 faster check for line whether it's inside drawing rect #4269

Merged
merged 1 commit into from Jan 23, 2020

Conversation

danielgindi
Copy link
Collaborator

This obviously does not consider slope that's outside,
But these cases will be clipping by CoreGraphics anyway.
Doing a full line collision test here is an overkill.

Issue Link πŸ”—

Fixes a performance degradation introduced here: #4100
It was a big chunk of code that was introduced to fix a subtle bug, but most of the fix was actually the code calling it - figuring out the first/last line coordinate for stepped lined.

Goals ⚽

Revert to almost-the-previous-implementation, keeping the actual fix in place.

Implementation Details 🚧

When testing simply that lastCoordinate.x > left && firstCoordinate.x < right, it's enough to ensure the line is horizontally somewhere in the content rect.
The equivalent vertical check was inverted, causing rare issues.
But the main issue was actually with stepped lines, where a line was more than 2 coordinates, and only the first 2 were validated.

Testing Details πŸ”

Zooming in manually in all scenarios until all coordinates are off screen - horizontally and vertically.

This obviously does not consider slope that's outside,
But these cases will be clipping by CoreGraphics anyway.
Doing a full line collision test here is an overkill.
@danielgindi danielgindi merged commit 6c3d72c into master Jan 23, 2020
@danielgindi danielgindi deleted the bugfix/line_intersection_perf branch January 24, 2020 07:44
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.

None yet

1 participant