Skip to content

ClippingPolygon immutable positions (performance) - #13665

Open
mzschwartz5 wants to merge 5 commits into
clipping-polygon-holesfrom
clipping-polygons-performance
Open

ClippingPolygon immutable positions (performance)#13665
mzschwartz5 wants to merge 5 commits into
clipping-polygon-holesfrom
clipping-polygons-performance

Conversation

@mzschwartz5

@mzschwartz5 mzschwartz5 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Description

Inspired by #13273 (which has drifted significantly in light of the new clipping polygon implementation this PR builds on). This PR makes the positions of ClippingPolgyons immutable (which is a breaking change!). The reason: if we can guarantee these positions do not change, we can get rid of dirty checks that occur per-vertex-per-frame. These checks are expensive for large collections (and at the high frequency of 60+ FPS). Instead, users can remove-and-readd (new) polygons. This isn't cheap (requires texture rebakes), but it's at least explicit / easier to detect (and respond to once rather than 60 times a second).

The original PR didn't quite land on a consensus of how to implement this. See my comment here on how I've decided to approach it.

Issue number and link

Part of #12258

Testing plan

This sandcastle (link is for this branch), is good for testing. It allows you to vary polygon count and polygon vertex count. Try it here and on main and compare the FPS -- it should be a pretty obvious difference.

Author checklist

  • I have submitted a Contributor License Agreement
  • I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change
  • I have added or updated unit tests to ensure consistent code coverage
  • I have updated the inline documentation, and included code examples where relevant
  • I have performed a self-review of my code

AI acknowledgment

  • I used AI to generate content in this PR
  • If yes, I have reviewed the AI-generated content before submitting

If yes, I used the following Tools(s) and/or Service(s):

If yes, I used the following Model(s):

PR Dependency Tree

This tree was auto-generated by Charcoal

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Thank you for the pull request, @mzschwartz5!

✅ We can confirm we have a CLA on file for you.

@mzschwartz5
mzschwartz5 marked this pull request as ready for review August 5, 2026 15:47
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygon-holes branch from e2f9d8f to a2cf88a Compare August 10, 2026 19:39
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygons-performance branch from 46d6579 to 82ea5e7 Compare August 10, 2026 19:39

@danielzhong danielzhong left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't forget to add CHANGES.md later!

@mzschwartz5
mzschwartz5 force-pushed the clipping-polygon-holes branch from a2cf88a to c54c525 Compare August 18, 2026 17:20
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygons-performance branch from 82ea5e7 to 8ef925c Compare August 18, 2026 17:20
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygon-holes branch from c54c525 to 15b8f39 Compare August 18, 2026 17:28
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygons-performance branch from 8ef925c to d34e4bd Compare August 18, 2026 17:28
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygon-holes branch from 15b8f39 to 0e904c0 Compare August 19, 2026 17:08
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygons-performance branch from d34e4bd to 398a04a Compare August 19, 2026 17:08
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygon-holes branch from 0e904c0 to 86de2b8 Compare August 19, 2026 19:38
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygons-performance branch from 398a04a to f96ec81 Compare August 19, 2026 19:38
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygon-holes branch from 86de2b8 to 589af27 Compare August 19, 2026 20:00
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygons-performance branch 2 times, most recently from 815b46e to 843b8ae Compare August 19, 2026 20:48
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygon-holes branch from 25d6e02 to 434e171 Compare August 20, 2026 12:50
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygons-performance branch from 843b8ae to e626f94 Compare August 20, 2026 12:50
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygon-holes branch from 434e171 to c50ae6f Compare August 20, 2026 14:17
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygons-performance branch from e626f94 to 4e05529 Compare August 20, 2026 14:17
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygon-holes branch from c50ae6f to 3c04c7d Compare August 20, 2026 14:33
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygons-performance branch 2 times, most recently from ef62eb2 to 9c78dc8 Compare August 20, 2026 15:11
result,
deprecationWarning(
"ClippingPolygon.computeRectangle",
"ClippingPolygon.computeRectangle is deprecated as of CesiumJS 1.144 and will be removed in 1.146. Use the ClippingPolygon.rectangle property instead.",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I noticed a small typo in the deprecation message. ClippingPolygon.computeRectangle should be deprecated as of version 1.145 instead of 1.144, since 1.144 is already released.

@mzschwartz5
mzschwartz5 force-pushed the clipping-polygon-holes branch from 3c04c7d to 1bb2726 Compare August 21, 2026 16:14
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygons-performance branch from 9c78dc8 to 5f65c7a Compare August 21, 2026 16:14
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygon-holes branch from 1bb2726 to 6ff03a6 Compare August 21, 2026 16:16
@mzschwartz5
mzschwartz5 force-pushed the clipping-polygons-performance branch from 5f65c7a to b9e7100 Compare August 21, 2026 16:16
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