Gate screen-space error per view's visibility

[byjtew]

Description

The fix is straightforward imo: if a tile is not visible by the narrow camera but only by the wide one, then don't use the narrow computed-quality on the tiles only visible from the wide camera.

Issue number or link

CesiumGS/cesium-unreal#1823

https://community.cesium.com/t/multiple-registred-cameras-performance-issue/45482/11

Author checklist

• I have submitted a Contributor License Agreement (only needed once).
• I have done a full self-review of my code.
• I have updated CHANGES.md with a short summary of my change (for user-facing changes).
• I have added or updated unit tests to ensure consistent code coverage as necessary.
• I have updated the documentation as necessary.

Remaining Tasks

I have found this to be extremely effective for this weird special context with multiple cameras, but it may break other things. There may be a better approach, or maybe not ?

Testing plan

Call Tileset::updateView with two ViewStates sharing the same position and target, one wide ~40deg FoV and one narrow, then sweep the narrow view's FoV down to ~1deg.

Before this change, getViewUpdateResult().tilesVisited climbs ~34x (e.g. ~1600 to ~52000) as the narrow FoV shrinks while tilesToRenderThisFrame stays flat, since tiles only the wide view sees get refined to the narrow view's SSE. After the change, tilesVisited stays bounded (~2300) at every FoV, with no change to either view's rendered tiles.

Reviewer checklist

Thank you for taking the time to review this PR. By approving a PR you are taking as much responsibility for these changes as the author.

As you review, please go through the checklist below:

• Review and run all parts of the test plan on this branch and verify it matches expectations.
  • If the issue is a bug please make sure you can reproduce the bug in the main branch and then checkout this branch to make sure it actually solved the issue.
• Review the code and make sure you do not have any remaining questions or concerns. You should understand the code change and the chosen approach. If you are not confident or have doubts about the code, please do not hesitate to ask questions.
  • Code should follow the C++ Style Guide
• Review the unit tests and make sure there are no missing tests or edge cases.
• Review documentation changes and updates to CHANGES.md to make sure they accurately cover the work in this PR.
• Verify that the Contributor License Agreement has been submitted, if needed.

[j9liu]

Thanks @byjtew for the contribution! This looks like a straightforward and logical fix -- feel free to add this to CHANGES.md. We can try to sneak it into the upcoming release but I know that it's cutting it close, that's on us for not reviewing sooner.

Before this change, getViewUpdateResult().tilesVisited climbs ~34x (e.g. ~1600 to ~52000) as the narrow FoV shrinks while tilesToRenderThisFrame stays flat, since tiles only the wide view sees get refined to the narrow view's SSE. After the change, tilesVisited stays bounded (~2300) at every FoV, with no change to either view's rendered tiles.

That's a major improvement! Were you testing this with Unreal Engine? If so, were you using the Tracing Insights to measure the time difference?

Also, are you able to add a unit test for this in TestTilesetSelection.cpp? Checking .tileScreenSpaceErrorThisFrame would probably be sufficient.

[j9liu]

One concern I thought of is making sure that isVisibleFromCamera is true for at least one frustum in the vector.

We know SSE isn't computed for any tiles that we shouldn't visit because of the early return for culled tiles:

cesium-native/Cesium3DTilesSelection/src/TilesetSelection.cpp

Lines 1086 to 1121 in 1f4e85d

	if (!cullResult.shouldVisit) {
	addCurrentTileAndDescendantsToTilesFadingOutIfPreviouslyRendered(
	frameState.viewGroup,
	tile,
	result);
	frameState.viewGroup.getTraversalState().currentState() =
	TileSelectionState(TileSelectionState::Result::Culled);
	++result.tilesCulled;
	TraversalDetails traversalDetails{};
	if (context.options.forbidHoles &&
	tile.getRefine() == TileRefine::Replace) {
	// In order to prevent holes, we need to load this tile and also not
	// render any siblings until it is ready. We don't actually need to
	// render it, though.
	addTileToLoadQueue(
	context,
	frameState,
	tile,
	TileLoadPriorityGroup::Normal,
	tilePriority);
	traversalDetails = createTraversalDetailsForSingleTile(frameState, tile);
	} else if (context.options.preloadSiblings) {
	addTileToLoadQueue(
	context,
	frameState,
	tile,
	TileLoadPriorityGroup::Preload,
	tilePriority);
	}
	traversalState.finishNode(&tile);
	return traversalDetails;
	}

So the tile is definitely selected, but I do worry about the logic in isVisibleFromCamera possibly not resolving in the case that a frustum-culled tile should still be visited.

cesium-native/Cesium3DTilesSelection/src/TilesetSelection.cpp

Lines 209 to 232 in 1f4e85d

	bool isVisibleFromCamera(
	const ViewState& viewState,
	const BoundingVolume& boundingVolume,
	const Ellipsoid& ellipsoid,
	bool forceRenderTilesUnderCamera) {
	if (viewState.isBoundingVolumeVisible(boundingVolume)) {
	return true;
	}
	if (!forceRenderTilesUnderCamera) {
	return false;
	}
	const std::optional<CesiumGeospatial::Cartographic>& position =
	viewState.getPositionCartographic();
	// TODO: it would be better to test a line pointing down (and up?) from the
	// camera against the bounding volume itself, rather than transforming the
	// bounding volume to a region.
	std::optional<GlobeRectangle> maybeRectangle =
	estimateGlobeRectangle(boundingVolume, ellipsoid);
	if (position && maybeRectangle) {
	return maybeRectangle->contains(position.value());
	}
	return false;
	}

I'm going to dig into this later to confirm whether or not this is a concern, but wanted to mention it early in case.

[j9liu]

So the tile is definitely selected, but I do worry about the logic in isVisibleFromCamera possibly not resolving in the case that a culled tile should still be visited (because we do have those situations)...

This case can happen when enableFrustumCulling is false. However, I believe checking for context.options.enableFrustumCulling before doing the isVisibleFromCamera check should be enough to avoid returning an SSE of 0.0.

[byjtew]

I don't manage to repro in the cesium samples project with enableFrustumCulling set to true. I think we can call it a false alarm.
Thanks @j9liu

[j9liu]

Hi @byjtew, can you clarify what you didn't manage to repro and what is a false alarm?

[byjtew]

Got busy sorry about that, but I didn't want to come back with a random fix not backed up by tests.

Honest disclaimer: I got help from LLMs for the new fixes and commits that are on that PR. I know the CesiumForUnreal code well, not the cesium-native as well at all. Feel free to question who/what/how as much as your policy requires.

---

Correction to my earlier comment: not a false alarm. Two paths reach it.

1. enableFrustumCulling = false. frustumCull sets culled = true and leaves shouldVisit alone, so the tile is visited with nothing seeing it. The gated error is 0.0, which meets every threshold, so culledScreenSpaceError is never consulted. Same for forbidHoles and an unconditionally refined root.
2. cullWithChildrenBounds, independent of any option. frustumCull passes the tile when a child is visible, while gating on the tile's own bounds sees nothing. Default config, Replace refinement.

Checking enableFrustumCulling does not cover 2. Re-testing visibility inside computeSse does not either, and costs an isVisibleFromCamera per frustum per tile.

Current approach: frustumCull records which frustums saw the tile in CullResult, and computeSse folds only over those. When no frustum recorded visibility it folds over all of them, which covers 1. Everything is in TilesetSelection.cpp, no header changes.

Notes:

• frustumCull no longer short-circuits, so all frustums are tested. One view is unchanged. N views cost N visibility tests, against N error computations that computeSse now skips.
• The record is a uint64_t, so frustums past 64 are treated as visible and are not gated, which is the behavior before this change. A scratch buffer would remove the cap, at the cost of a TileSelectionContext member.

Tests are committed before the fix so they can be run against the old behavior.

• 40 and 1 degree views from the same position. For each rendered tile the narrow view cannot see, directly or through a child, the recorded error should be the wide view's. Before the fix it is 76.9 against 1.8. The ratio is 41.7, which is tan(20 deg) / tan(0.5 deg).
• Culled tiles keep the ungated error. This one passes on main. It fails for a gate with no fallback.

tilesVisited on the first fixture: 43,643 before, 167 after. The fixture creates children on demand with no I/O, so it refines until the error threshold stops it; the ratio is an upper bound, not a figure from real data. maxDepthVisited is 17 in both runs, so the difference is breadth. Rendered tiles go from 68 to 100.

[byjtew]

After rebase on main update:

Two cases: culling disabled, where the tile gets visited with nothing seeing it; and cullWithChildrenBounds, which happens on default options.
frustumCull now records which frustums saw the tile, computeSse only folds over those. Falls back to all of them when none did.
4 tests, one commit each, before the fix. Adding a 1 degree camera pointed at the sky took tilesVisited from 55 to 43,643.
One thing to flag: this narrows the rule in the "highest SSE decides refinement" subcase to views that can actually see the tile.

Each test in one sentence:

• per-view gating: two co-located cameras at 40° and 1° both looking at the globe: every tile's recorded error must belong to a camera that can see it, and each camera must drive at least one tile.
• per-view distance: two cameras differing in both position and field of view, so pairing a camera with the other camera's distance would yield a value neither could have reported.
• culled tiles: with frustum culling off and both cameras facing away, tiles no camera sees must keep the maximum error across all cameras rather than 0.
• blind camera: adding a camera that faces away from the globe must leave tilesVisited, tilesCulled, the rendered count, and every recorded error exactly unchanged.

[j9liu]

Thanks for the changes @byjtew! I left some comments about the overall approach as well as some cleanup on the unit tests. Let me know if you have any questions!

[j9liu]

Instead of iterating over all of the frustums in frustumCull and saving a bitfield, you could save the index of the first visible frustum found by frustumCull like so:

 std::optional<size_t> firstVisibleFrustumIndex;

If it's std::nullopt, then computeSse can exit early with an SSE of 0.0. Otherwise, you can use that as the starting index in the for loop.

This would eliminate any size restraints from using a uint64_t bitfield. It's also less data than a std::vector<bool>. And because frustumCull returns early, we're only iterating over the N frustums once.

[byjtew]

#1427 (comment)

[j9liu]

This test is a bit confusing because of the multiple frustums. It would be stronger if it only used blind to select tiles, observing that no tiles were rendered in the frame.

[byjtew]

#1427 (comment)

[byjtew]

Done: merged main and moved the changelog entry (the clean merge had quietly dropped it into the released 0.64.0 section), folded the helper back into the tests, both renames.

On the frustum index, I don't think it works as described, two things.

• returning 0.0 for nullopt brings back the culled tile case you flagged earlier, it bypasses culledScreenSpaceError entirely.
• using the first visible index as the loop start still folds in the frustums after it, so wide at index 0 with narrow at index 1 stays broken.

Your point about the cap was right though, so I dropped the bitfield anyway. computeDistances already runs before frustumCull, so frustumCull now just accumulates the max SSE over the frustums that can see the tile, one std::optional in CullResult. No bitfield, no 64 limit, still a single pass, and computeSse goes back to exactly what it was.

About the blind test: I checked and blind alone renders 0 tiles both with and without the fix, so that version would pass on unfixed code.
Kept the two frustums, but happy to add the blind case test next to it if you want it covered too ?

[kring]

Janine asked me to take a look at this. The logic in this part of the code (even before this PR) is surprisingly tricky. The direction in this PR looks great (thanks for the PR @byjtew!), but I don't think it addresses all the corner cases yet.

By the way, I believe this PR fixes CesiumGS/cesium-unreal#1823.

culledScreenSpaceError

The single visibleScreenSpaceError might be inadequate. I think we need at least two SSEs. The first one (visibleScreenSpaceError) tracks the max SSE for any view in which the tile is inside the view's frustum. The second (let's call it culledScreenSpaceError) is the max SSE for any ViewState in which the tile is outside the frustum. The reason they need to be separate is because we have two different Max SSE values in TilesetOptions for these two scenarios: maximumScreenSpaceError and culledScreenSpaceError. (The second really should be named culledMaximumScreenSpaceError, but that's unrelated to this PR...)

Imagine we have two views. One (A) is close to the surface and looking toward a given tile. The other (B) is farther away and looking away from the tile, so the tile is outside its frustum. enableFrustumCulling=false so we need to consider both tiles when deciding whether or not to refine this tile. maximumScreenSpaceError=16, so we need to refine this tile if the SSE observed by view A is greater than 16. culledScreenSpaceError=64, so we need to refine this tile if the SSE observed by view B is greater than 64. If A sees 12 and B sees 61, there's no need to refine.

tileSse

visitTileIfNeeded passes the tileSse through to visitTile. It looks like it's only used when the tile is selected for rendering, to create the parallel tileScreenSpaceErrorThisFrame vector. Which is used for voxel rendering I think? We need to think through what to pass here; perhaps we need two SSEs now. If the highest SSE is from a view where the tile is inside its frustum, of course we should use that one. If the tile isn't inside any view's frustum, then I guess we would use the culledScreenSpaceError. But what if the highest SSE comes from a view where the tile is outside its frustum? Or, more confusingly, what about a case like this:

• In View A, tile is outside the frustum, and has an SSE of 60 (<64, so no need to refine).
• In View B, the tile is inside the frustum, and has an SSE of 20 (>16 so we need to refine, but View B's SSE is lower than View A's).

[byjtew]

@kring
Pushed a failing (on master too) new test for the culledScreenSpaceError case.

One addition: the culled SSE should only count when enableFrustumCulling is false. With culling on, a view that can't see a tile wants it gone, so it shouldn't drive its refinement. Otherwise the 1 degree view keeps refining tiles it can't see and the fix stops working.

Idea:

struct CullResult {
  bool shouldVisit = true;
  bool culled = false;
  // Largest error among the frustums that can see this tile.
  std::optional<double> visibleScreenSpaceError;
  // Largest among those that can't. Only filled when frustum culling is off,
  // since otherwise such a view culls the tile away entirely.
  std::optional<double> culledViewScreenSpaceError;
};

bool meetsSse = true;
if (cullResult.visibleScreenSpaceError) {
  meetsSse = *cullResult.visibleScreenSpaceError <
             context.options.maximumScreenSpaceError;
}
if (meetsSse && cullResult.culledViewScreenSpaceError) {
  meetsSse = !context.options.enforceCulledScreenSpaceError ||
             *cullResult.culledViewScreenSpaceError <
                 context.options.culledScreenSpaceError;
}

Existing meetsSseThreshold(computeSse(...), cullResult.culled) stays for when neither is filled: fog culled, excluder culled, forbidHoles, unconditionally refined root.

On tileSse: I'd pass the visible error when any view sees the tile, else the culled one. Your A=60 outside / B=20 inside case reports 20, the view actually rendering it. You probably know what consumes tileScreenSpaceErrorThisFrame though, so tell me if that's wrong.

Unrelated to your comments: fogCull only marks a tile culled when every view fog culls it, so the same two budget question exists for fog. Happy to leave it out of this PR for you guys.

Side-note: I'm okay if your teams wants to take over this PR, I didn't think it would extend that far tbh.