Vector Textures for Polygon Clipping - #13635
Conversation
|
Thank you for the pull request, @mzschwartz5! ✅ We can confirm we have a CLA on file for you. |
6d8274e to
8ff468e
Compare
06fb8bd to
a8d6359
Compare
a24fd41 to
01f8ceb
Compare
a8d6359 to
387fd3a
Compare
01f8ceb to
90400af
Compare
387fd3a to
9a9a1c5
Compare
bdc7874 to
f1ffa66
Compare
b7e15a0 to
6e0f908
Compare
f1ffa66 to
1e862fa
Compare
1e862fa to
a2a430d
Compare
2e08e5d to
ee4fdcb
Compare
6c783e3 to
a862d38
Compare
a862d38 to
1f19e4a
Compare
| this._vectorCollectionData = VectorPipeline.packPolygonCollectionData( | ||
| this._bufferPolygonCollection, | ||
| this.ellipsoid, | ||
| ); |
There was a problem hiding this comment.
I think there's a staleness issue with refreshing the vector data here. polygonAdded / polygonRemoved fire synchronously inside add() / remove(), but this repack doesn't happen until the next update(). Since the doc on requestRectangleData tells consumers to re request in response to those events, they could end up reading a snapshot that doesn't include the new polygon yet.
Also, because update() early returns when totalPositions hasn't changed, removing one polygon and adding another with the same vertex count never reaches this line at all, the vector data stays stale forever. That's a known trade off of the SDF path, but it silently carries over into the new API.
One more thing: we never pass the third result arg (or call markClean()), so the version short-circuit in packPolygonCollectionData can never kick in, every call is a full repack with fresh allocations.
What do you think about dropping this hook (and the eager pack in the constructor) and refreshing lazily inside requestRectangleData instead? Something like:
this._vectorCollectionData = VectorPipeline.packPolygonCollectionData(
this._bufferPolygonCollection,
this.ellipsoid,
this._vectorCollectionData,
);
this._bufferPolygonCollection.markClean();
There was a problem hiding this comment.
I think there's a staleness issue with refreshing the vector data here.
Good call out. I think this could be reframed as more of a documentation issue, though. The docs should tell readers to listen for add()/remove(), but that the collection data refreshes on the next update(). In up-stack PRs, you can see how consumers mark a flag as dirty when add/remove are called, and then re-request rectangle data on the next update.
(One could alternatively change the code, so that the data repacks lazily in requestRectangleData, and is thus never stale. However, Globe and Models actually call ClippingPolygonCollection.update() manually, so they're already in control of the lifecycle of this vector data resource, if that makes sense. In fact, unlike other entities in Cesium, ClippingPolygonCollection.update is only ever called by its owner, despite what the method's docs say).
Also, because update() early returns when totalPositions hasn't changed, removing one polygon and adding another with the same vertex count never reaches this line at all, the vector data stays stale forever.
This is also true, but it's addressed in a PR later in the stack. Eventually, we remove that check entirely and rely on a plain dirty flag.
One more thing: we never pass the third result arg (or call markClean()), so the version short-circuit in packPolygonCollectionData can never kick in, every call is a full repack with fresh allocations.
I think that's okay. The purpose of the third arg is as a caching optimization -- one that only works if using the version number on the buffer collection. I designed these ClippingPolygonCollection changes to use push-based notifications (the add()/remove() events) instead of pull-based (version numbers), so any caller to requestRectangleData already knows their data is dirty and needs a refresh.
Think of ClippingPolygonCollection as an alternative to VectorProvider. While consumers of VectorProvider call updateTileData each frame and let VectorProvider track caches and dirtiness, consumers of ClippingPolygonCollection subscribe to its dirty events, and use those as signals to rerequest data.
Description
This is the second step in reimplementing Clipping Polygons on top of the vector data pipeline. In step one, I created a
BufferPolygonCollectionas a mirrored backing data structure for theClippingPolygonCollectionclass. In this PR, I expose a new (internal) API onClippingPolygonCollection--requestRectangleData-- analogous to the similarly named versions provided byVectorProvider. This API accepts a rectangular region of an ellipsoid, and processes the clipping polygons within that rectangle to create textures used for vector-style clipping (by calling theVectorPipelineAPIs).There is also a parallel
releaseRectangleDataAPI. The intention is for various consumers (terrain, 3d tiles, standalone models) to call these APIs with their respective footprint rectangles.Issue number and link
https://github.com/iTwin/cesiumjs-web3d-internal/issues/32
Testing plan
Again, there are no outward facing changes in this PR. At this point, clipping still relies on the SDF path. The machinery is just being put into place for vector clipping. As such, more regression testing suffices (sandcastle)
Author checklist
CONTRIBUTORS.mdCHANGES.mdwith a short summary of my changeAI acknowledgment
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