Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

External tileset traversal fix #7035

Merged
merged 4 commits into from
Sep 14, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Change Log
* Added `cartographicLimitRectangle` to `Globe`. Use this to limit terrain and imagery to a specific `Rectangle` area. [#6987](https://github.com/AnalyticalGraphicsInc/cesium/pull/6987)

##### Fixes :wrench:
* Fixed an issue in the 3D Tiles traversal where external tilesets would not always traverse to their root tile. [#7035](https://github.com/AnalyticalGraphicsInc/cesium/pull/7035)
* Fixed an issue in the 3D Tiles traversal where empty tiles would be selected instead of their nearest loaded ancestors. [#7011](https://github.com/AnalyticalGraphicsInc/cesium/pull/7011)

### 1.49 - 2018-09-04
Expand Down
32 changes: 17 additions & 15 deletions Source/Scene/Cesium3DTilesetTraversal.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ define([
return tile._distanceToCamera;
}
var parent = tile.parent;
var useParentScreenSpaceError = defined(parent) && (!skipLevelOfDetail(tileset) || (tile._screenSpaceError === 0.0));
var useParentScreenSpaceError = defined(parent) && (!skipLevelOfDetail(tileset) || (tile._screenSpaceError === 0.0) || parent.hasTilesetContent);
var screenSpaceError = useParentScreenSpaceError ? parent._screenSpaceError : tile._screenSpaceError;
var rootScreenSpaceError = tileset.root._screenSpaceError;
return rootScreenSpaceError - screenSpaceError; // Map higher SSE to lower values (e.g. root tile is highest priority)
Expand Down Expand Up @@ -307,7 +307,7 @@ define([

// Use parent's geometric error with child's box to see if the tile already meet the SSE
var parent = tile.parent;
if (defined(parent) && (parent.refine === Cesium3DTileRefine.ADD) && getScreenSpaceError(tileset, parent.geometricError, tile, frameState) <= tileset._maximumScreenSpaceError) {
if (defined(parent) && !parent.hasTilesetContent && (parent.refine === Cesium3DTileRefine.ADD) && getScreenSpaceError(tileset, parent.geometricError, tile, frameState) <= tileset._maximumScreenSpaceError) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is getting a bit unwieldy. Can we account for this in updateVisibility? Or if not just pull out

getScreenSpaceError(tileset, parent.geometricError, tile, frameState)
or
getScreenSpaceError(tileset, parent.geometricError, tile, frameState) <= tileset._maximumScreenSpaceError

into its own var?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to its own function.

Unfortunately it can't be handled in updateVisibility because it shouldn't get run when checking children visibility for the cull-with-children-bounds optimization.

tile._visible = false;
return;
}
Expand Down Expand Up @@ -438,6 +438,18 @@ define([
return tile._screenSpaceError > baseScreenSpaceError;
}

function canTraverse(tileset, tile) {
if (tile.children.length === 0) {
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the consensus from #7007 is not to throw a warning, correct?

It seems like it would be fairly trivial to include here with a one-time warning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think we should wait and see if it comes up again. But agreed it should be easy to add if we decide to.

}
if (tile.hasTilesetContent) {
// Traverse external tileset to visit its root tile
// Don't traverse if the subtree is expired because it will be destroyed
return !tile.contentExpired;
}
return tile._screenSpaceError > tileset._maximumScreenSpaceError;
}

function executeTraversal(tileset, root, baseScreenSpaceError, maximumScreenSpaceError, frameState) {
// Depth-first traversal that traverses all visible tiles and marks tiles for selection.
// If skipLevelOfDetail is off then a tile does not refine until all children are loaded.
Expand All @@ -455,19 +467,11 @@ define([
var baseTraversal = inBaseTraversal(tileset, tile, baseScreenSpaceError);
var add = tile.refine === Cesium3DTileRefine.ADD;
var replace = tile.refine === Cesium3DTileRefine.REPLACE;
var children = tile.children;
var childrenLength = children.length;
var parent = tile.parent;
var parentRefines = !defined(parent) || parent._refines;
var traverse = (childrenLength > 0) && (tile._screenSpaceError > maximumScreenSpaceError);
var refines = false;

if (tile.hasTilesetContent && tile.contentExpired) {
// Don't traverse expired subtree because it will be destroyed
traverse = false;
}

if (traverse) {
if (canTraverse(tileset, tile)) {
refines = updateAndPushChildren(tileset, tile, stack, frameState) && parentRefines;
}

Expand Down Expand Up @@ -514,7 +518,6 @@ define([
function executeEmptyTraversal(tileset, root, frameState) {
// Depth-first traversal that checks if all nearest descendants with content are loaded. Ignores visibility.
var allDescendantsLoaded = true;
var maximumScreenSpaceError = tileset._maximumScreenSpaceError;
var stack = emptyTraversal.stack;
stack.push(root);

Expand All @@ -526,7 +529,7 @@ define([
var childrenLength = children.length;

// Only traverse if the tile is empty - traversal stop at descendants with content
var traverse = hasEmptyContent(tile) && (childrenLength > 0) && (tile._screenSpaceError > maximumScreenSpaceError);
var traverse = hasEmptyContent(tile) && canTraverse(tileset, tile);

// Traversal stops but the tile does not have content yet.
// There will be holes if the parent tries to refine to its children, so don't refine.
Expand Down Expand Up @@ -573,7 +576,6 @@ define([
* selected tiles must be no deeper than 15. This is still very unlikely.
*/
function traverseAndSelect(tileset, root, frameState) {
var maximumScreenSpaceError = tileset._maximumScreenSpaceError;
var stack = selectionTraversal.stack;
var ancestorStack = selectionTraversal.ancestorStack;
var lastAncestor;
Expand Down Expand Up @@ -606,7 +608,7 @@ define([
var shouldSelect = tile._shouldSelect;
var children = tile.children;
var childrenLength = children.length;
var traverse = (childrenLength > 0) && (tile._screenSpaceError > maximumScreenSpaceError);
var traverse = canTraverse(tileset, tile);

if (shouldSelect) {
if (add) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
88
]
},
"geometricError": 240,
"geometricError": 70,
"refine": "ADD",
"content": {
"uri": "tileset2.json"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
20
]
},
"geometricError": 70,
"geometricError": 0,
"content": {
"uri": "tileset3/tileset3.json"
}
Expand Down
12 changes: 10 additions & 2 deletions Specs/Scene/Cesium3DTilesetSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1079,8 +1079,7 @@ defineSuite([
//
return Cesium3DTilesTester.loadTileset(scene, tilesetReplacement1Url).then(function(tileset) {
tileset.root.geometricError = 90;
viewRootOnly();
scene.camera.zoomIn(20);
setZoom(80);
scene.renderForSpecs();

var statistics = tileset._statistics;
Expand Down Expand Up @@ -1364,6 +1363,15 @@ defineSuite([
});
});

it('always visits external tileset root', function() {
viewRootOnly();
return Cesium3DTilesTester.loadTileset(scene, tilesetOfTilesetsUrl).then(function(tileset) {
var statistics = tileset._statistics;
expect(statistics.visited).toEqual(2); // Visits external tileset tile, and external tileset root
expect(statistics.numberOfCommands).toEqual(1); // Renders external tileset root
});
});

it('set tile color', function() {
return Cesium3DTilesTester.loadTileset(scene, noBatchIdsUrl).then(function(tileset) {
// Get initial color
Expand Down