cesium icon indicating copy to clipboard operation
cesium copied to clipboard

[PoC] Shared Context

Open pmconne opened this issue 9 months ago • 5 comments

Proof-of-concept only, submitted for feedback. Not intended to merge.

Description

Currently every Scene has its own WebGL context. Two scenes cannot share primitives, shader programs, textures, etc. This can cause an app that displays multiple scenes to use more memory than it should need to, especially if it's displaying large primitives like 3D tilesets.

This PR is a quick and dirty attempt to enable multiple scenes to share a single WebGL context. This feature is opt-in; the existing default behavior and APIs are undisturbed.

The PR is submitted to solicit feedback on the approach.

Reference-counting in PrimitiveCollection

The destroyPrimitives option supplied to the constructor can now be specified as "reference-counted". Adding a Primitive to any reference-counted PrimitiveCollection increments its reference count. Removing one decrements the count and - if destroyPrimitives is true (because it can be changed after construction) and the reference count reaches zero - destroys the primitive.

SharedContext

Apps can create a SharedContext from optional ContextOptions. It wraps a Context. It can be supplied in place of ContextOptions to the constructors of Scene, CesiumWidget, and Viewer. In that case, its primitives will be reference-counted and its _context will be a proxy that mostly delegates to the shared context.

By default, a SharedContext is destroyed automatically after every Scene that uses it is destroyed.

Issue number and link

Testing plan

Added a "Shared Context" sandcastle that displays a single tileset and cylinder primitive in two viewers sharing a WebGL context. Commenting out the line that enables context sharing illustrates how this previously did not work (the graphics would only display in the first viewer).

Added unit tests.

Open questions

  • [ ] Many bits of code rely on FrameState.frameNumber when updating. This breaks when multiple contexts are using the same object. See for example the change I made to ImageBasedLighting.
  • [ ] Some additional Context fields probably should be unique per-Scene rather than forwarded to the shared context (e.g., _id, _nextPickColor?)
  • [ ] Some primitives like Cesium3DTileset store state from previous traversals, assuming only a single Scene is using them. (This also precludes traversal for purposes other than display).
  • [ ] There doesn't appear to be a way to draw the same tileset with two different styles in two different scenes.
  • [ ] The higher-level EntityCollection/DataSourceDisplay APIs generate primitives under the hood, making it tricky to share WebGL resources between entities even if the underlying scenes share the same context.
  • [ ] Scene.initializeFrame does some housekeeping on the context's shader and texture caches every 12- frames - that's going to thrash a bit when multiple scenes share the context.
  • [ ] Map/globe do not appear to share WebGL resources between contexts.
  • [ ] A tile's content and the styling applied to it via Cesium3DTilesetStyle are tightly coupled, making it tricky to apply different styling to the same tileset in different views.

Really the most fundamental issue is that primitives and their WebGL resources do not really live independently from a Context that wants to display them. WebGL resources instead get created as a by-product of trying to render a primitive, within the context in which they are being rendered.

Author checklist

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

pmconne avatar May 26 '25 18:05 pmconne

Thank you for the pull request, @pmconne!

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

github-actions[bot] avatar May 26 '25 18:05 github-actions[bot]

Here are a few slides describing the approach, potential next steps, and proposed benefits.

I'm looking for feedback re: feasibility from CesiumJS experts familiar with all the nuances I've undoubtedly overlooked.

pmconne avatar May 29 '25 14:05 pmconne

I cannot provide a thorough review (or at least, would have to allocate more time for that).

A high-level comment is that I'm in strong favor of whatever helps to clean up the resource handling. There is a tight coupling between 'Something' and 'Something, but rendered'. I think that trying to use something in two contexts will eventually lead to a state where the separation between "an object" and "what is required for rendering that object" becomes clearer.

(It is not unusual that generalizations - like ~"having two scenes", forces cleaner approaches. Although it's usually not "two". Musings about "two"......).

Similarly, when you mention that "Some primitives like Cesium3DTileset store state from previous traversals...", one could argue that anything that is related to the traversal is not part of the inherent state of a tileset, but rather of that of a 'TilesetTraverser', but "traversal" and "rendering" are similarly coupled (and be it only via the FrameState)

An overly specific question is whether https://github.com/CesiumGS/cesium/issues/1567#issuecomment-1871297825 👴 might be resolved by the reference counting. I might try to allocate some time to try it out...

javagl avatar May 31 '25 13:05 javagl

Thanks for the proposal @pmconne!

Overall, I'm very pleased with how simple the underlying approach is here. I can't think of any particular reason not to take this route. I expect the brunt of the effort will be testing and updating the existing feature set to take advantage of a shared context, and refactoring where existing assumptions are not sufficient.

As @javagl mentioned, this is probably a chance to update some older parts of the codebase for the better.

@lilleyse Could you please take a high-level pass on the approach here?

Assuming @lilleyse does not identify any show-stoppers, we should discuss a plan for how we should go about iterating on this for additional features.

ggetz avatar Jun 03 '25 13:06 ggetz

CC https://github.com/CesiumGS/cesium/issues/5214

ggetz avatar Jun 03 '25 13:06 ggetz

  • Does this approach work with different scene modes, e.g. one scene using 2D and another using 3D? I think it should work since most primitives generate separate 2D and 3D geometry / commands rather than rebuilding when the mode changes.

Any other scene state that potentially affects geometry / shaders?

lilleyse avatar Jun 27 '25 16:06 lilleyse

Mark the new APIs @private for now.

pmconne avatar Jun 27 '25 17:06 pmconne

Following up from chatting with @pmconne offline:

  • We know the 3D Tiles selection for multiple cameras is going to be critical for making this feature broadly useful, but it's likely a sizable chunk of work. We should coordinate with the Cesium native team who have implemented this and see what we can bring back to CesiumJS.
  • In the meantime, to keep the scope of this PR small, we can mark the opt-in APIs as private for now to get this merged into main, then incrementally merge in improvements like 3D Tiles selection or others we identify.
  • When we're ready to broadly launch the feature, we can mark the opt-in APIs public (and make sure we have polished docs and examples to support that).

ggetz avatar Jun 27 '25 17:06 ggetz

  • [ ] I have updated CHANGES.md with a short summary of my change

@ggetz I presume that new @private APIs don't bear mentioning in this change log?

pmconne avatar Jul 07 '25 21:07 pmconne

@pmconne

I presume that new @Private APIs don't bear mentioning in this change log?

Correct. The change log should include only public API changes.

ggetz avatar Jul 07 '25 21:07 ggetz

Thanks @pmconne and @markschlosseratbentley! This looks ready to go. We'll follow up in https://github.com/CesiumGS/cesium/issues/5214 for additional updates.

ggetz avatar Jul 28 '25 13:07 ggetz