Skip to content

Let GeoJsonDocumentRasterOverlay be updated without recreating it - #1484

Draft
baruchInsert-tech wants to merge 3 commits into
CesiumGS:mainfrom
baruchInsert-tech:geojson-overlay-dynamic-styles
Draft

baruchInsert-tech wants to merge 3 commits into
CesiumGS:mainfrom
baruchInsert-tech:geojson-overlay-dynamic-styles

Conversation

@baruchInsert-tech

Copy link
Copy Markdown
Contributor

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

GeoJsonDocumentRasterOverlayTileProvider built its quadtree once and stored raw const VectorStyle* pointers into the caller's live GeoJsonDocument. As described in #1469, that meant:

  • Styles changed in place applied only to tiles rasterized afterward, leaving a patchwork.
  • Line-width padding and bounds were never recomputed.
  • Adding a style where there was none had no effect.
  • std::optional::reset() on a style left a dangling pointer.
  • Worker threads read the styles while the main thread wrote them, a data race.

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-VectorStyle problem class from #1468, since nothing points into the document's styles anymore.

New API on GeoJsonDocumentRasterOverlay:

void setDocument(const std::shared_ptr<const GeoJsonDocument>& pDocument);
void refreshStyles();
const VectorStyle& getDefaultStyle() const noexcept;
void setDefaultStyle(const VectorStyle& style);

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:

  • Style changes: for each geometry whose resolved style changed, the union of its old and new extent, padded by the larger of its old and new pixel line width or point size. Neighboring tiles that a wider line spills into are included.
  • New document: the old and the new contents.

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):

  • All methods must be called on the main thread.
  • Styles may be changed in place on the main thread and take effect after refreshStyles().
  • Geometry must not be changed while the overlay uses the document; pass a new document to setDocument instead.

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 createTileProvider call, so a second activation (another tileset, or the same one after removal) used an empty future. The overlay's state, including the document as a SharedFuture, 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 loadTileImage indexed nodes[rootId] anyway. It now returns a transparent tile.

Tests

Added to TestGeoJsonDocumentRasterOverlay.cpp:

  • refreshStyles invalidates only the tiles whose styles changed; changes made without refreshStyles are not picked up; refreshing with no changes invalidates nothing
  • a wider pixel-width line invalidates a neighboring tile it spills into, but not one farther away
  • setDefaultStyle re-rasterizes geometry that uses the default style, and not geometry with its own style
  • setDocument invalidates the old and new geometry but not unrelated tiles, and the new tiles render the new document
  • the overlay can be activated twice at once, and removed and added again (Access violation when re-adding or sharing GeoJsonDocumentRasterOverlay across multiple tilesets #1362); a change reaches both activations
  • a document with no geometry renders, and geometry added later with setDocument shows up

Testing

Built with MSVC (VS 2026, RelWithDebInfo) and ran the full cesium-native-tests suite locally with both commits: 652 test cases passed.

Merging

This conflicts with #1482 in one line: the value_or fix there is in code this PR replaces. The resolution is to keep this PR's version.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant