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

feat: optimization of "solid" polygon/polyline display #1876

Merged
merged 6 commits into from
May 23, 2024

Conversation

monsieurtanuki
Copy link
Contributor

What

Optimized the algorithm for "solid" polygon/polyline display.
The trick is to use addPolygon instead of numerous moveTo/lineTo. It looked like removing redundant moveTos was quite effective, too.

Following are some perf tests. We're getting closer to the old solid algorithm (that we had to dismiss because of bugs in 25+ zooms).

x old solid algo new solid algo current PR
Raster (ms) 206.2 578.9 208.6
UI (ms) 42.1 53.2 49.6

Impacted files

  • polygon_layer/painter.dart: minor refactoring
  • polyline_layer/painter.dart: minor refactoring
  • pixel_hiker.dart: optimized the "solid pixel hiker" relying on addPolygon instead of numerous moveTo/lineTo

Screenshots

Screenshot_20240507_160145

monsieurtanuki and others added 3 commits May 7, 2024 16:19
Impacted files:
* `polygon_layer/painter.dart`: minor refactoring
* `polyline_layer/painter.dart`: minor refactoring
* `pixel_hiker.dart`: optimized the "solid pixel hiker" relying on `addPolygon` instead of numerous `moveTo`/`lineTo`
Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

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

Hey @monsieurtanuki,

Thanks for continuing to work on this! I've been testing this PR and have noticed that there's an issue on the master branch and on this PR, that could be related. Not sure whether it's worth fixing in a different PR, or this PR, but needs to be fixed for sure.

image

It looks like the culling algortihm no longer accounts for the radius of the line, so it's possible to see the radius of the curve at the edges of the viewport.

@monsieurtanuki
Copy link
Contributor Author

Hi @JaffaKetchup!

Probably not related to culling, but to drawing segments clipped to screen rectangle instead of a continuous polygon.
Could be fixed by pretending that the screen is slightly bigger: an extra half radius on each side of the clipping screen rectangle.

@monsieurtanuki
Copy link
Contributor Author

Hi @JaffaKetchup!
I've just fixed the issue, that was more obvious for solid lines but impacted dashed and dotted lines too.
As expected, the idea was just to take into account the stroke width when we clip segments into the screen.
Instead of clipping into
[0, 0, canvas.width, canvas.height]
we have to clip into
[-strokeWidth/2, -strokeWidth/2, canvas.width+strokeWidth/2, canvas.height/strokeWidth/2].
Et voilà!

x before after
polyline Screenshot_1716189708 Screenshot_1716189734
polygon Screenshot_1716189913 Screenshot_1716189938

@JaffaKetchup
Copy link
Member

I'll try to review this tomorrow :)

Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

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

LGTM I think! Thanks once again :)

@JaffaKetchup JaffaKetchup merged commit 7c99ae3 into fleaflet:master May 23, 2024
7 checks passed
@monsieurtanuki
Copy link
Contributor Author

Thank you @JaffaKetchup for your reviews and feedbacks!

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

Successfully merging this pull request may close these issues.

3 participants