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

refacto: added saveLayers parameter #1219

Merged
merged 2 commits into from Apr 19, 2022
Merged

refacto: added saveLayers parameter #1219

merged 2 commits into from Apr 19, 2022

Conversation

TesteurManiak
Copy link
Collaborator

Solve #1217

  • Added saveLayers parameter to PolylineLayerOptions & PolylinePainter to enable or disable calls to canvas.saveLayer & canvas.restore (by default at false as advised in ibrierley's comment)
  • Replaced calls to path.lineTo by path.addPolygon in PolylinePainter._paintLine (recommended here: Tracking: Performance Issues #1165 (comment))

- Added saveLayers parameter to PolylineLayerOptions & PolylinePainter to enable or disable calls to canvas.saveLayer & canvas.restore
- Replaced calls to path.lineTo by path.addPolygon in PolylinePainter._paintLine
@ibrierley
Copy link
Collaborator

Just thinking, I note the polygon layer also does a saveLayer...worth us doing the same there for consistency ?

@TesteurManiak
Copy link
Collaborator Author

It depends, in my opinion you won't have the same performance issues with polygons as, most of the time, you will draw less polygons than polylines and you might need more often to manage opacity or gradients on polygons.

Copy link
Collaborator

@ibrierley ibrierley left a comment

Choose a reason for hiding this comment

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

Tests fine for me...only thing I think would be an extra useful bit, is a comment to explain why someone would turn it on...eg turn this on if you get unwanted darker lines where they overlap, but there will be a performance hit.

@TesteurManiak
Copy link
Collaborator Author

Yeah this is a good idea, I'm adding it.

@JaffaKetchup JaffaKetchup linked an issue Apr 19, 2022 that may be closed by this pull request
8 tasks
@JaffaKetchup
Copy link
Member

I'll see what I can do to add this to the new documentation :)

@ibrierley
Copy link
Collaborator

All good for me, fine for me to merge @JaffaKetchup or wanted to do some further testing ?

@JaffaKetchup
Copy link
Member

Its ok for you to merge if you're happy

@ibrierley ibrierley merged commit 998f615 into fleaflet:master Apr 19, 2022
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.

[BUG] Polylines lagging after upgrading from Flutter 2.5 to 2.10
3 participants