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!: support of solid, dotted, dashed styles for polygons, with optimized rendering #1865

Merged
merged 8 commits into from
Apr 10, 2024

Conversation

monsieurtanuki
Copy link
Contributor

What

  • Now we support dashed style for polygons.
  • We use the same algorithm as in polylines, with
    • DottedPixelHiker that returns the visible dots (and only them)
    • DashedPixelHiker that returns the visible segments (and only them)
  • The algorithm has been improved in order to take into account side-effects fixed in fix: prevent crash when zooming far into Polygons #1854

Screenshot

Screenshot_1712493274

Files

New files:

  • pixel_hiker.dart: Pixel hikers that list the visible items on the way. Code used to be in polyline_layer/painter.dart, but was heavily refactored with fix: prevent crash when zooming far into Polygons #1854 in mind
  • visible_segment.dart: Cohen-Sutherland algorithm to clip segments as visible into a canvas. Code used to be in polygon_layer/painter.dart, and was lightly refactored.

Impacted files:

  • polygon_layer/painter.dart: now using new file pixel_hiker.dart for optimized rendering; moved "clip code" to new file visible_segment.dart; minor refactoring about parameter order consistency
  • polyline_layer/painter.dart: now using new file pixel_hiker.dart for optimized rendering; moved "pixel hiker" to new file pixel_hiker.dart
  • pages/polygon.dart: replaced bool isDotted with PolylinePattern pattern and in one case replaced it with "dashed"
  • polygon_layer/polygon.dart: BREAKING - replaced bool isDotted with PolylinePattern pattern
  • polygon_layer/polygon_layer.dart: minor refactoring
  • polyline_layer/polyline_layer.dart: minor refactoring

…imized rendering

New files:
* `pixel_hiker.dart`: Pixel hikers that list the visible items on the way. Code used to be in `polyline_layer/painter.dart`, but was heavily refactored with fleaflet#1854 in mind
* `visible_segment.dart`: Cohen-Sutherland algorithm to clip segments as visible into a canvas. Code used to be in `polygon_layer/painter.dart`, and was lightly refactored.

Impacted files:
* `polygon_layer/painter.dart`: now using new file `pixel_hiker.dart` for optimized rendering; moved "clip code" to new file `visible_segment.dart`; minor refactoring about parameter order consistency
* `polyline_layer/painter.dart`: now using new file `pixel_hiker.dart` for optimized rendering; moved "pixel hiker" to new file `pixel_hiker.dart`
* `pages/polygon.dart`: replaced `bool isDotted` with `PolylinePattern pattern` and in one case replaced it with "dashed"
* `polygon_layer/polygon.dart`: BREAKING - replaced `bool isDotted` with `PolylinePattern pattern`
* `polygon_layer/polygon_layer.dart`: minor refactoring
* `polyline_layer/polyline_layer.dart`: minor refactoring
@JaffaKetchup JaffaKetchup self-requested a review April 9, 2024 14:06
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.

Untested, going to make some minor file organisation changes, but just just this one comment to consider for now.
This is looking amazing though, and the new painter code is so much simpler!

lib/src/layer/polygon_layer/polygon.dart Show resolved Hide resolved
Re-organised file structure
@monsieurtanuki
Copy link
Contributor Author

Untested, going to make some minor file organisation changes, but just just this one comment to consider for now. This is looking amazing though, and the new painter code is so much simpler!

@JaffaKetchup Thank you! The complexity of the dots or dash patterns multiplied by the "visible?" question asked for those changes.

Copy link
Contributor Author

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

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

@JaffaKetchup I'm fine with all your code organization improvements.

Co-authored-by: monsieurtanuki <[email protected]>
Copy link
Contributor

@josxha josxha left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request @monsieurtanuki. I did a quick test on web with chrome and didn't experience any problems, nice work!

The directory /src/layer/general makes sense to me, but could we have it's name be in line with /src/misc by renaming one of these folders please?

lib/src/layer/general/line_patterns/stroke_pattern.dart Outdated Show resolved Hide resolved
lib/src/layer/general/line_patterns/pixel_hiker.dart Outdated Show resolved Hide resolved
lib/src/layer/general/line_patterns/stroke_pattern.dart Outdated Show resolved Hide resolved
@monsieurtanuki
Copy link
Contributor Author

The directory /src/layer/general makes sense to me, but could we have it's name be in line with /src/misc by renaming one of these folders please?

@JaffaKetchup I have no strong opinion on the subject and I'm notoriously neither regarding nor imaginative when it comes to folder and file names, so feel free to move files to a more appropriate folder.

Copy link
Contributor

@josxha josxha left a comment

Choose a reason for hiding this comment

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

Looks good to me. @JaffaKetchup I assume you want to test the changes yourself before we merge?

@JaffaKetchup
Copy link
Member

Bit short on time, so if you're happy, that sounds good to me!
Thanks once again @monsieurtanuki :)

@JaffaKetchup JaffaKetchup merged commit 930e8b6 into fleaflet:master Apr 10, 2024
7 checks passed
@josxha josxha added this to the v7.0 milestone Apr 10, 2024
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