Repository navigation
Let GeoJsonDocumentRasterOverlay be updated without recreating it - #1484
Draft
baruchInsert-tech wants to merge 3 commits into
Draft
baruchInsert-tech wants to merge 3 commits into
baruchInsert-tech wants to merge 3 commits into
Conversation
Add `invalidate()` and `invalidate(region, paddingPixels)` to `RasterOverlayTileProvider` and `ActivatedRasterOverlay`. The provider keeps a small log of invalidated regions, and each `RasterOverlayTile` records the provider's invalidation generation when it starts loading, so `ActivatedRasterOverlay::isTileInvalidated` can tell whether a tile's image is out of date. During the tileset's per-frame overlay update, `RasterMappedTo3DTile::reloadIfInvalidated` requests a replacement for an out-of-date tile. The current image stays attached until the replacement has loaded, so the overlay doesn't flicker. A failed reload keeps the current image and isn't retried until the next invalidation.
The tile provider now resolves and copies every style it needs on the main thread, instead of keeping pointers into the document. Changing a document's styles in place no longer races with rasterization on worker threads or leaves dangling pointers. Add `setDocument`, `refreshStyles`, `getDefaultStyle` and `setDefaultStyle`. Each tile provider picks up changes in `tick()`, rebuilds its quadtree on a worker thread, and invalidates only the regions that render differently: for style changes, the old and new extent (plus pixel padding) of each geometry whose style changed; for a new document, the old and new contents. The overlay's state, including the document future, is now shared by every tile provider created from it. This fixes a crash when the same overlay was added to more than one tileset or re-added after removal. Also handle documents with no geometry. Fixes CesiumGS#1469 Fixes CesiumGS#1362
Clang rejects comparing a raw RasterOverlayTile pointer with an IntrusivePointer inside doctest's CHECK as ambiguous, so compare raw pointers in the raster overlay reload test. clang-tidy's bugprone-unchecked-optional-access can't see the check of an optional accessed through an iterator in RasterOverlayTileProvider::isInvalidatedSince, so read it into a local reference first.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1469. Fixes #1362.
Depends on #1483, which adds raster overlay invalidation. This PR includes that commit until it's merged; only the last commit, "Let GeoJsonDocumentRasterOverlay be updated without recreating it", is new here.
Problem
GeoJsonDocumentRasterOverlayTileProviderbuilt its quadtree once and stored rawconst VectorStyle*pointers into the caller's liveGeoJsonDocument. As described in #1469, that meant:std::optional::reset()on a style left a dangling pointer.Changing geometry required recreating the overlay, and re-adding or sharing an overlay instance crashed (#1362).
Changes
Styles are copied. When the quadtree is built, the provider resolves each geometry's style (geometry → feature → collection → default) on the main thread and copies it into the tree. Identical styles are stored once. Rasterization on worker threads reads only those copies. This also removes the dangling-
VectorStyleproblem class from #1468, since nothing points into the document's styles anymore.New API on
GeoJsonDocumentRasterOverlay:Each tile provider picks up changes in
tick(). It copies the styles on the main thread, rebuilds the quadtree on a worker thread, swaps it in, and then invalidates only what renders differently:Through #1483, tiles that are already shown keep their image until the replacement is ready, so nothing flickers.
Compared with the proposal in #1469,
refreshStyles()takes no region argument. The provider compares the old and new styles itself, so callers don't have to track what changed.Threading contract (documented on the class):
refreshStyles().setDocumentinstead.Behavior change: changing a style in place without calling
refreshStyles()now has no effect. Before, it applied partially and unsafely.#1362: the overlay's document future was moved out by the first
createTileProvidercall, so a second activation (another tileset, or the same one after removal) used an empty future. The overlay's state, including the document as aSharedFuture, is now shared by every tile provider created from it, and each one receives updates.Empty documents: a document with no geometry produced no quadtree nodes, and
loadTileImageindexednodes[rootId]anyway. It now returns a transparent tile.Tests
Added to
TestGeoJsonDocumentRasterOverlay.cpp:refreshStylesinvalidates only the tiles whose styles changed; changes made withoutrefreshStylesare not picked up; refreshing with no changes invalidates nothingsetDefaultStylere-rasterizes geometry that uses the default style, and not geometry with its own stylesetDocumentinvalidates the old and new geometry but not unrelated tiles, and the new tiles render the new documentGeoJsonDocumentRasterOverlayacross multiple tilesets #1362); a change reaches both activationssetDocumentshows upTesting
Built with MSVC (VS 2026, RelWithDebInfo) and ran the full
cesium-native-testssuite locally with both commits: 652 test cases passed.Merging
This conflicts with #1482 in one line: the
value_orfix there is in code this PR replaces. The resolution is to keep this PR's version.