Skip to content

Conversation

@JaffaKetchup
Copy link
Member

Both types of culling are still performed after simplification.

It is expected that this will have the most effect on longer polylines that are not always within the viewport - for example, the user is zoomed into see part of one specific long polyline in a map with many. Whilst this does add more work, it avoids many iterations of a loop that would be useless. To minimize the expense of constructing the bounding box for the polylines, they are cached on the Polyline (the same as we do for Polygons).
Note that the bbox culling still works in geographic space - this is what we do for polygons, and to avoid this, a new class would be needed to maintain efficiency, mixing Bounds and DoublePoint.

In the example stress test, with simplification set to 0.02 to increase stress on the culling, with the entire polyline off screen, it seems to reduce the load on the UI thread by about ~1-2ms.

Before:
Screenshot 2024-11-24 164159

After:
Screenshot 2024-11-24 164354

Copy link
Contributor

@mootw mootw left a comment

Choose a reason for hiding this comment

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

is the bbox re-used for post-simplification culling or is that based on the simplified polygon?

@JaffaKetchup JaffaKetchup requested review from a team and removed request for TesteurManiak, ibrierley and josxha November 24, 2024 18:43
@JaffaKetchup
Copy link
Member Author

JaffaKetchup commented Nov 24, 2024

@mootw That's a good point. It is using the original shape not the simplified shape for the bounding box. However, it looks as though the polygon culling is also doing this. Do you think this needs fixing? Theoretically not fixing could increase the chance of pop-in, but I'm not sure how big of an issue it is. Incorporating caching for the bounding box may also be a little more complex (but necessary, because constructing the bbox is quite expensive).

@JaffaKetchup JaffaKetchup merged commit 4dc3bca into master Dec 5, 2024
8 checks passed
@JaffaKetchup JaffaKetchup deleted the perf-test branch December 5, 2024 09:58
@JaffaKetchup JaffaKetchup restored the perf-test branch December 5, 2024 18:58
@JaffaKetchup JaffaKetchup deleted the perf-test branch December 5, 2024 18:58
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.

3 participants