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

Add support to check children bounds for visibility before selecting a tile #4978

Merged
merged 20 commits into from
Feb 22, 2017
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
11 changes: 11 additions & 0 deletions Source/Scene/Cesium3DTile.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ define([
'../Core/Cartesian3',
'../Core/Color',
'../Core/ColorGeometryInstanceAttribute',
'../Core/clone',
'../Core/defaultValue',
'../Core/defined',
'../Core/defineProperties',
Expand Down Expand Up @@ -38,6 +39,7 @@ define([
Cartesian3,
Color,
ColorGeometryInstanceAttribute,
clone,
defaultValue,
defined,
defineProperties,
Expand Down Expand Up @@ -305,6 +307,13 @@ define([
* @private
*/
this.visibilityPlaneMask = true;

/**
* Flag to mark children visibility
*
* @private
*/
this.childrenVisibility = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of assigning to 1, ChildrenVisibilityFlags should be in its own file (it can be @private) and declared like other Cesium enums. For example, see Cesium3DTileContentState.js.


/**
* The last frame number the tile was selected in.
Expand All @@ -329,6 +338,8 @@ define([
this._debugViewerRequestVolume = undefined;
this._debugColor = new Color.fromRandom({ alpha : 1.0 });
this._debugColorizeTiles = false;

this._optimizations = clone(tileset._optimizations);
}

defineProperties(Cesium3DTile.prototype, {
Expand Down
125 changes: 125 additions & 0 deletions Source/Scene/Cesium3DTileOptimizations.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*global define*/
define([
'../Core/Cartesian3',
'../Core/defaultValue',
'../Core/freezeObject',
'./TileOrientedBoundingBox',
'./TileBoundingRegion'
Copy link
Contributor

Choose a reason for hiding this comment

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

Flip these two (alphabetical ordering)

], function(
Cartesian3,
defaultValue,
freezeObject,
TileOrientedBoundingBox,
TileBoundingRegion) {
'use strict';

/**
* @alias Cesium3DTileOptimizationFlag
*
* Flag denoting support for a 3D Tiles optimization
*/
var Cesium3DTileOptimizationFlag = {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also be @private in its own file.

Also, we generally don't suffix enums with Flag or Flags. Perhaps there are better names for these.

UNSUPPORTED: 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps rename these to:

NOT_COMPUTED
USE_OPTIMIZATION
SKIP_OPTIMIZATION

In Cesium, "supported" usually refers to WebGL features.

SUPPORTED: 1,
INDETERMINATE: -1
};

/**
* Defines optional optimization flags for a {@link Cesium3DTileset}.
* Optimizations marked as Cesium3DTileOptimizations.Flags.SUPPORTED or Cesium3DTileOptimizations.Flags.UNSUPPORTED
Copy link
Contributor

Choose a reason for hiding this comment

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

Use {@link } when referring to enums.

For more, see the doc guide.

* will not be evaluated at runtime
*
* @alias Cesium3DTileOptimizations
* @constructor
*
* @example
* var tileset = new Cesium.Cesium3DTileset({
* url: ...,
* optimizations: new Cesium.Cesium3DTileOptimizations({
* childrenWithinParent: Cesium.Cesium3DTileOptimizations.Flags.SUPPORTED
* })
* });
*/
function Cesium3DTileOptimizations(options) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In the doc, this should be @private.

Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid some indirection at runtime (and likely cache misses), consider replacing with with a boolean property on the tile. So file would just contain a single function for checkChildrenWithinParent.

In time, the boolean would probably become a bitmask with many booleans for different optimizations and tiles would more cohesively fit into cache lines (I'm not saying that they do today, but this extra indirection does not help the case).

options = defaultValue(options, defaultValue.EMPTY_OBJECT);

/**
* Denotes support for if the childrenWithinParent optimization is supported. This is used to more tightly cull tilesets if children bounds are
* fully contained within the parent.
*
* @type {Cesium3DTileOptimizationFlag}
*/
this.childrenWithinParent = defaultValue(options.childrenWithinParent, Cesium3DTileOptimizations.Flags.INDETERMINATE);
}

var scratchAxis = new Cartesian3();

/**
* Checks and optinonally evaluates support for the childrenWithinParent optimization. This is used to more tightly cull tilesets if children bounds are
* fully contained within the parent.
*
* @param {Cesium3DTile} tile The tile to check.
* @param {Boolean} [evaluate=false] Whether to evaluate support if support for the childrenWithinParent optimization is Cesium3DTileOptimizations.INDETERMINATE
* @param {Boolean} [force=false] Whether to always evaluate support for the childrenWithinParent optimization
* @returns {Boolean} Whether the childrenWithinParent optimization is supported.
*/
Cesium3DTileOptimizations.prototype.checkChildrenWithinParent = function(tile, evaluate, force) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Depending on your tests, we may end up removing this, e.g., if the new CPU overhead is trivial and/or it is a win for most datasets.

If either of the above is true, we may just do it all the time and even avoid include hints in the tileset.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can probably just check this every time. In practice, we only need to evaluate it once per tile (unless tiles move relative to their parent?). So really, this is just a small one-time cost on load.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not about load time performance, it's about client-side complexity.

My point is that writing a general test for this taking into account all the different bounding volume types (basically it is CSG union) could be a lot of work to write, test, etc. Maybe there are some heuristics to simplify this by putting boxes around regions and using a cost function dependent on the number of children.

I dunno, could be complicated.

So we might just skip this completely on the client if the tileset contains a hint to do it or not, or if always checking the children is fast/useful enough.

evaluate = defaultValue(evaluate, false);
force = defaultValue(force, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Throw DeveloperError if tile is undefined.


if ((this.childrenWithinParent === Cesium3DTileOptimizations.Flags.INDETERMINATE && evaluate) || force) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a reminder to comment this function as we discussed offline.

var children = tile.children;
var length = children.length;
var boundingVolume = tile._boundingVolume;
if (boundingVolume instanceof TileOrientedBoundingBox || boundingVolume instanceof TileBoundingRegion) {
var orientedBoundingBox = boundingVolume._orientedBoundingBox;
this.childrenWithinParent = Cesium3DTileOptimizations.Flags.SUPPORTED;
for (var i = 0; i < length; ++i) {
var child = children[i];
var childBoundingVolume = child._boundingVolume;
if (childBoundingVolume instanceof TileOrientedBoundingBox || childBoundingVolume instanceof TileBoundingRegion) {
var childOrientedBoundingBox = childBoundingVolume._orientedBoundingBox;
var axis = Cartesian3.subtract(childOrientedBoundingBox.center, orientedBoundingBox.center, scratchAxis);
var axisLength = Cartesian3.magnitude(axis);
Cartesian3.divideByScalar(axis, axisLength, axis);

var proj1 = Math.abs(orientedBoundingBox.halfAxes[0] * axis.x) +
Math.abs(orientedBoundingBox.halfAxes[1] * axis.y) +
Math.abs(orientedBoundingBox.halfAxes[2] * axis.z) +
Math.abs(orientedBoundingBox.halfAxes[3] * axis.x) +
Math.abs(orientedBoundingBox.halfAxes[4] * axis.y) +
Math.abs(orientedBoundingBox.halfAxes[5] * axis.z) +
Math.abs(orientedBoundingBox.halfAxes[6] * axis.x) +
Math.abs(orientedBoundingBox.halfAxes[7] * axis.y) +
Math.abs(orientedBoundingBox.halfAxes[8] * axis.z);

var proj2 = Math.abs(childOrientedBoundingBox.halfAxes[0] * axis.x) +
Math.abs(childOrientedBoundingBox.halfAxes[1] * axis.y) +
Math.abs(childOrientedBoundingBox.halfAxes[2] * axis.z) +
Math.abs(childOrientedBoundingBox.halfAxes[3] * axis.x) +
Math.abs(childOrientedBoundingBox.halfAxes[4] * axis.y) +
Math.abs(childOrientedBoundingBox.halfAxes[5] * axis.z) +
Math.abs(childOrientedBoundingBox.halfAxes[6] * axis.x) +
Math.abs(childOrientedBoundingBox.halfAxes[7] * axis.y) +
Math.abs(childOrientedBoundingBox.halfAxes[8] * axis.z);

if (proj1 <= proj2 + axisLength) {
this.childrenWithinParent = Cesium3DTileOptimizations.Flags.UNSUPPORTED;
break;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this check general enough to be used as a function in OrientedBoundingBox? If so this could be useful to have there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps, but it's not a totally accurate OrientedBoundingBox.contains(OrientedBoundingBox) check. It becomes less useful if the parent and child don't have similar axes. In this case it's not a problem for two reasons. Most of the time the chidlren do have the same axes as their parent. Secondly, if it is wrong, we will just skip the optimization and not produce visual artifacts..


} else {
this.childrenWithinParent = Cesium3DTileOptimizations.Flags.UNSUPPORTED;
break;
}
}
}
}

return this.childrenWithinParent === Cesium3DTileOptimizations.Flags.SUPPORTED ? true : false;
}

Cesium3DTileOptimizations.Flags = freezeObject(Cesium3DTileOptimizationFlag)

return Cesium3DTileOptimizations;
});
122 changes: 92 additions & 30 deletions Source/Scene/Cesium3DTileset.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ define([
'../Core/Cartesian3',
'../Core/Cartographic',
'../Core/Color',
'../Core/clone',
'../Core/defaultValue',
'../Core/defined',
'../Core/defineProperties',
Expand All @@ -26,6 +27,7 @@ define([
'../ThirdParty/when',
'./Cesium3DTile',
'./Cesium3DTileColorBlendMode',
'./Cesium3DTileOptimizations',
'./Cesium3DTileRefine',
'./Cesium3DTileStyleEngine',
'./CullingVolume',
Expand All @@ -39,6 +41,7 @@ define([
Cartesian3,
Cartographic,
Color,
clone,
defaultValue,
defined,
defineProperties,
Expand All @@ -62,6 +65,7 @@ define([
when,
Cesium3DTile,
Cesium3DTileColorBlendMode,
Cesium3DTileOptimizations,
Cesium3DTileRefine,
Cesium3DTileStyleEngine,
CullingVolume,
Expand Down Expand Up @@ -270,7 +274,8 @@ define([
lastColor : new Cesium3DTilesetStatistics(),
lastPick : new Cesium3DTilesetStatistics()
};


Copy link
Contributor

Choose a reason for hiding this comment

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

Extra whitespace.

this._optimizations = defaultValue(options.optimizations, new Cesium3DTileOptimizations());
Copy link
Contributor

Choose a reason for hiding this comment

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

Just remove this for now until we decide on CesiumGS/3d-tiles#176

this._tilesLoaded = false;

/**
Expand Down Expand Up @@ -822,6 +827,12 @@ define([
get : function() {
return this._statistics;
}
},

optimizations : {
get : function() {
return this._optimizations;
}
}
});

Expand Down Expand Up @@ -1154,6 +1165,46 @@ define([
var scratchStack = [];
var scratchRefiningTiles = [];

var ChildrenVisibilityFlags = {
NONE: 0x0,
VISIBLE: 0x1,
IN_REQUEST_VOLUME: 0x2,
VISIBLE_IN_REQUEST_VOLUME: 0x4
};

function computeChildrenVisibility(tile, frameState, checkViewerRequestVolume) {
var flag = ChildrenVisibilityFlags.NONE;
var children = tile.children;
var childrenLength = children.length;
var visibilityPlaneMask = tile.visibilityPlaneMask;
for (var k = 0; k < childrenLength; ++k) {
var child = children[k];

var visibilityMask = child.visibility(frameState, visibilityPlaneMask);

if (isVisible(visibilityMask)) {
flag |= ChildrenVisibilityFlags.VISIBLE;
}

if (checkViewerRequestVolume) {
if (!child.insideViewerRequestVolume(frameState)) {
visibilityMask = CullingVolume.MASK_OUTSIDE;
} else {
flag |= ChildrenVisibilityFlags.IN_REQUEST_VOLUME;
if (isVisible(visibilityMask)) {
flag |= ChildrenVisibilityFlags.VISIBLE_IN_REQUEST_VOLUME;
}
}
}

child.visibilityPlaneMask = visibilityMask
}

tile.childrenVisibility = flag;

return flag;
}

function selectTiles(tileset, frameState, outOfCore) {
if (tileset.debugFreezeFrame) {
return;
Expand Down Expand Up @@ -1285,11 +1336,21 @@ define([
// With replacement refinement, if the tile's SSE
// is not sufficient, its children (or ancestors) are
// rendered instead
var useChildrenBoundUnion = t._optimizations.checkChildrenWithinParent(t, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

This workflow is a bit awkward. Can we just compute this on load and then just check a boolean property here?

Longer-term, this is the type of processing that would be in a web worker when a tile is downloaded and before it is ready to render.


var childrenVisibility;

if ((sse <= maximumScreenSpaceError) || (childrenLength === 0)) {
// This tile meets the SSE so add its commands.
// Select tile if it's a leaf (childrenLength === 0)
selectTile(tileset, t, fullyVisible, frameState);
if (useChildrenBoundUnion) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure this is the right place to add this code? I was expecting a place that use to have

checkThisTilesVolume

to be replaced with

checkTheUnionOfTheTilesChildsVolumes

childrenVisibility = computeChildrenVisibility(t, frameState, false);
if ((childrenVisibility & ChildrenVisibilityFlags.VISIBLE) || childrenLength === 0) {
selectTile(tileset, t, fullyVisible, frameState);
}
} else {
selectTile(tileset, t, fullyVisible, frameState);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Given these updates, it may be cleaner to separate out the childrenLength === 0 and put it before the sse <= maximumScreenSpaceError check.

} else if (!tileset._refineToVisible) {
// Tile does not meet SSE.
// Refine when all children (visible or not) are loaded.
Expand All @@ -1311,50 +1372,49 @@ define([
}

if (!allChildrenLoaded) {
// Tile does not meet SSE. Add its commands since it is the best we have and request its children.
selectTile(tileset, t, fullyVisible, frameState);
if (useChildrenBoundUnion) {
childrenVisibility = computeChildrenVisibility(t, frameState, false);
}
if (!useChildrenBoundUnion || (childrenVisibility & ChildrenVisibilityFlags.VISIBLE) || childrenLength === 0) {
// Tile does not meet SSE. Add its commands since it is the best we have and request its children.
selectTile(tileset, t, fullyVisible, frameState);

if (outOfCore) {
for (k = 0; (k < childrenLength) && t.canRequestContent(); ++k) {
child = children[k];
// PERFORMANCE_IDEA: we could spin a bit less CPU here by keeping separate lists for unloaded/ready children.
if (child.contentUnloaded) {
requestContent(tileset, child, outOfCore);
} else {
// Touch loaded child even though it is not selected this frame since
// we want to keep it in the cache for when all children are loaded
// and this tile can refine to them.
touch(tileset, child, outOfCore);
if (outOfCore) {
for (k = 0; (k < childrenLength) && t.canRequestContent(); ++k) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This area was probably missed in the merge conflict resolution, but make sure to fix it with the changes from here: https:/AnalyticalGraphicsInc/cesium/pull/5008/files

child = children[k];
// PERFORMANCE_IDEA: we could spin a bit less CPU here by keeping separate lists for unloaded/ready children.
if (child.contentUnloaded) {
requestContent(tileset, child, outOfCore);
} else {
// Touch loaded child even though it is not selected this frame since
// we want to keep it in the cache for when all children are loaded
// and this tile can refine to them.
touch(tileset, child, outOfCore);
}
}
}
}
} else {
// Tile does not meet SSE and its children are loaded. Refine to them in front-to-back order.
var anyChildrenVisible = false;
childrenVisibility = computeChildrenVisibility(t, frameState, true);
for (k = 0; k < childrenLength; ++k) {
child = children[k];
if (child.insideViewerRequestVolume(frameState)) {
child.visibilityPlaneMask = child.visibility(frameState, visibilityPlaneMask);
} else {
child.visibilityPlaneMask = CullingVolume.MASK_OUTSIDE;
}

if (isVisible(child.visibilityPlaneMask)) {
stack.push(child);
anyChildrenVisible = true;
} else {
// Touch the child tile even if it is not visible. Since replacement refinement
// requires all child tiles to be loaded to refine to them, we want to keep it in the cache.
touch(tileset, child, outOfCore);
}
}

if (anyChildrenVisible) {
if (childrenVisibility & ChildrenVisibilityFlags.VISIBLE_IN_REQUEST_VOLUME) {
t.replaced = true;
if (defined(t.descendantsWithContent)) {
scratchRefiningTiles.push(t);
}
} else {
} else if (!useChildrenBoundUnion || (childrenVisibility & ChildrenVisibilityFlags.VISIBLE) || childrenLength === 0) {
// Even though the children are all loaded they may not be visible if the camera
// is not inside their request volumes.
selectTile(tileset, t, fullyVisible, frameState);
Expand All @@ -1366,16 +1426,17 @@ define([

// Get visibility for all children. Check if any visible children are not loaded.
// PERFORMANCE_IDEA: exploit temporal coherence to avoid checking visibility every frame
updateTransforms(children, t.computedTransform);
childrenVisibility = computeChildrenVisibility(t, frameState, true);

if (useChildrenBoundUnion && childrenVisibility === ChildrenVisibilityFlags.NONE && childrenLength !== 0) {
continue;
}

var allVisibleChildrenLoaded = true;
var someVisibleChildrenLoaded = false;
for (k = 0; k < childrenLength; ++k) {
child = children[k];
child.updateTransform(t.computedTransform);
if (child.insideViewerRequestVolume(frameState)) {
child.visibilityPlaneMask = child.visibility(frameState, visibilityPlaneMask);
} else {
child.visibilityPlaneMask = CullingVolume.MASK_OUTSIDE;
}
if (isVisible(child.visibilityPlaneMask)) {
if (child.contentReady) {
someVisibleChildrenLoaded = true;
Expand Down Expand Up @@ -1461,6 +1522,7 @@ define([
for (j = 0; j < descendantsLength; ++j) {
descendant = refiningTile.descendantsWithContent[j];
if (!descendant.selected && !descendant.replaced &&
((descendant.childrenVisibility & ChildrenVisibilityFlags.VISIBLE) || descendant.children.length === 0) &&
(frameState.cullingVolume.computeVisibility(descendant.contentBoundingVolume) !== Intersect.OUTSIDE)) {
refinable = false;
break;
Expand Down