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

Move batch table hierarchy to extension #6780

Merged
merged 6 commits into from
Jul 13, 2018
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -5,6 +5,7 @@ Change Log

#### Deprecated :hourglass_flowing_sand:
* Support for 3D Tiles `content.url` is deprecated to reflect updates to the [3D Tiles spec](https://github.com/AnalyticalGraphicsInc/3d-tiles/pull/301). Use `content.uri instead`. Support for `content.url` will remain for backwards compatibility. [#6744](https://github.com/AnalyticalGraphicsInc/cesium/pull/6744)
* Support for the 3D Tiles pre-version 1.0 Batch Table Hierarchy is deprecated to reflect updates to the [3D Tiles spec](https://github.com/AnalyticalGraphicsInc/3d-tiles/pull/301). Use the [`3DTILES_batch_table_hierarchy`](https://github.com/AnalyticalGraphicsInc/3d-tiles/tree/1.0/extensions/3DTILES_batch_table_hierarchy) extension instead. Support for the deprecated batch table hierarchy will remain for backwards compatibility. [#6780](https://github.com/AnalyticalGraphicsInc/cesium/pull/6780)

#### Fixes :wrench:
* Fixed bug causing billboards and labels to appear the wrong size when switching scene modes [#6745](https://github.com/AnalyticalGraphicsInc/cesium/issues/6745)
Expand Down
161 changes: 104 additions & 57 deletions Source/Scene/Cesium3DTileBatchTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ define([
'../Core/defaultValue',
'../Core/defined',
'../Core/defineProperties',
'../Core/deprecationWarning',
'../Core/destroyObject',
'../Core/DeveloperError',
'../Core/Math',
Expand Down Expand Up @@ -44,6 +45,7 @@ define([
defaultValue,
defined,
defineProperties,
deprecationWarning,
destroyObject,
DeveloperError,
CesiumMath,
Expand Down Expand Up @@ -80,33 +82,25 @@ define([
*/
this.featuresLength = featuresLength;

this._translucentFeaturesLength = 0; // Number of features in the tile that are translucent

/**
* @private
*/
this.batchTableJson = batchTableJson;

/**
* @private
*/
this.batchTableBinary = batchTableBinary;
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like in master as well as here this property isn't used, so it should be fine to remove.


var batchTableHierarchy;
var batchTableBinaryProperties;
this._translucentFeaturesLength = 0; // Number of features in the tile that are translucent

var extensions;
if (defined(batchTableJson)) {
// Extract the hierarchy and remove it from the batch table json
batchTableHierarchy = batchTableJson.HIERARCHY;
if (defined(batchTableHierarchy)) {
delete batchTableJson.HIERARCHY;
batchTableHierarchy = initializeHierarchy(batchTableHierarchy, batchTableBinary);
}
// Get the binary properties
batchTableBinaryProperties = Cesium3DTileBatchTable.getBinaryProperties(featuresLength, batchTableJson, batchTableBinary);
extensions = batchTableJson.extensions;
}
this._extensions = defaultValue(extensions, {});
this._extras = defined(batchTableJson) ? batchTableJson.extras : undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

extras can removed since it isn't used


var properties = initializeProperties(batchTableJson);
this._properties = properties;

this._batchTableHierarchy = batchTableHierarchy;
this._batchTableBinaryProperties = batchTableBinaryProperties;
this._batchTableHierarchy = initializeHierarchy(this, batchTableJson, batchTableBinary);
this._batchTableBinaryProperties = getBinaryProperties(featuresLength, properties, batchTableBinary);

// PERFORMANCE_IDEA: These parallel arrays probably generate cache misses in get/set color/show
// and use A LOT of memory. How can we use less memory?
Expand Down Expand Up @@ -146,6 +140,9 @@ define([
this._textureStep = textureStep;
}

// This can be overridden for testing purposes
Cesium3DTileBatchTable._deprecationWarning = deprecationWarning;

defineProperties(Cesium3DTileBatchTable.prototype, {
memorySizeInBytes : {
get : function() {
Expand All @@ -161,23 +158,63 @@ define([
}
});

function initializeHierarchy(json, binary) {
function initializeProperties(jsonHeader) {
var properties = {};

if (!defined(jsonHeader)) {
return properties;
}

for (var propertyName in jsonHeader) {
if (jsonHeader.hasOwnProperty(propertyName)
&& propertyName !== 'HIERARCHY' // Deprecated HIERARCHY property
&& propertyName !== 'extensions'
&& propertyName !== 'extras') {
properties[propertyName] = clone(jsonHeader[propertyName], true);
}
}

return properties;
}

function initializeHierarchy(batchTable, jsonHeader, binaryBody) {
if (!defined(jsonHeader)) {
return;
}

var hierarchy = batchTable.getExtension('3DTILES_batch_table_hierarchy');

var legacyHierarchy = jsonHeader.HIERARCHY;
if (defined(legacyHierarchy)) {
Cesium3DTileBatchTable._deprecationWarning('batchTableHierarchyExtension', 'The batch table HIERARCHY property has been moved to an extension. Use extension.3DTILES_batch_table_hierarchy instead.');
Copy link
Contributor

Choose a reason for hiding this comment

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

Change Use extension.3DTILES_batch_table_hierarchy to Use extensions.3DTILES_batch_table_hierarchy

batchTable._extensions['3DTILES_batch_table_hierarchy'] = legacyHierarchy;
hierarchy = legacyHierarchy;
}

if (!defined(hierarchy)) {
return;
}

return initializeHierarchyValues(hierarchy, binaryBody);
}

function initializeHierarchyValues(hierarchyJson, binaryBody) {
var i;
var classId;
var binaryAccessor;

var instancesLength = json.instancesLength;
var classes = json.classes;
var classIds = json.classIds;
var parentCounts = json.parentCounts;
var parentIds = json.parentIds;
var instancesLength = hierarchyJson.instancesLength;
var classes = hierarchyJson.classes;
var classIds = hierarchyJson.classIds;
var parentCounts = hierarchyJson.parentCounts;
var parentIds = hierarchyJson.parentIds;
var parentIdsLength = instancesLength;

if (defined(classIds.byteOffset)) {
classIds.componentType = defaultValue(classIds.componentType, ComponentDatatype.UNSIGNED_SHORT);
classIds.type = AttributeType.SCALAR;
binaryAccessor = getBinaryAccessor(classIds);
classIds = binaryAccessor.createArrayBufferView(binary.buffer, binary.byteOffset + classIds.byteOffset, instancesLength);
classIds = binaryAccessor.createArrayBufferView(binaryBody.buffer, binaryBody.byteOffset + classIds.byteOffset, instancesLength);
}

var parentIndexes;
Expand All @@ -186,7 +223,7 @@ define([
parentCounts.componentType = defaultValue(parentCounts.componentType, ComponentDatatype.UNSIGNED_SHORT);
parentCounts.type = AttributeType.SCALAR;
binaryAccessor = getBinaryAccessor(parentCounts);
parentCounts = binaryAccessor.createArrayBufferView(binary.buffer, binary.byteOffset + parentCounts.byteOffset, instancesLength);
parentCounts = binaryAccessor.createArrayBufferView(binaryBody.buffer, binaryBody.byteOffset + parentCounts.byteOffset, instancesLength);
}
parentIndexes = new Uint16Array(instancesLength);
parentIdsLength = 0;
Expand All @@ -200,14 +237,14 @@ define([
parentIds.componentType = defaultValue(parentIds.componentType, ComponentDatatype.UNSIGNED_SHORT);
parentIds.type = AttributeType.SCALAR;
binaryAccessor = getBinaryAccessor(parentIds);
parentIds = binaryAccessor.createArrayBufferView(binary.buffer, binary.byteOffset + parentIds.byteOffset, parentIdsLength);
parentIds = binaryAccessor.createArrayBufferView(binaryBody.buffer, binaryBody.byteOffset + parentIds.byteOffset, parentIdsLength);
}

var classesLength = classes.length;
for (i = 0; i < classesLength; ++i) {
var classInstancesLength = classes[i].length;
var properties = classes[i].instances;
var binaryProperties = Cesium3DTileBatchTable.getBinaryProperties(classInstancesLength, properties, binary);
var binaryProperties = getBinaryProperties(classInstancesLength, properties, binaryBody);
classes[i].instances = combine(binaryProperties, properties);
}

Expand Down Expand Up @@ -282,11 +319,15 @@ define([
}
//>>includeEnd('debug');

Cesium3DTileBatchTable.getBinaryProperties = function(featuresLength, json, binary) {
function getBinaryProperties(featuresLength, properties, binaryBody) {
if (!defined(properties)) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This check isn't needed. properties defaults to an empty object.


var binaryProperties;
for (var name in json) {
if (json.hasOwnProperty(name)) {
var property = json[name];
for (var name in properties) {
if (properties.hasOwnProperty(name)) {
var property = properties[name];
var byteOffset = property.byteOffset;
if (defined(byteOffset)) {
// This is a binary property
Expand All @@ -298,14 +339,14 @@ define([
if (!defined(type)) {
throw new RuntimeError('type is required.');
}
if (!defined(binary)) {
if (!defined(binaryBody)) {
throw new RuntimeError('Property ' + name + ' requires a batch table binary.');
}

var binaryAccessor = getBinaryAccessor(property);
var componentCount = binaryAccessor.componentsPerAttribute;
var classType = binaryAccessor.classType;
var typedArray = binaryAccessor.createArrayBufferView(binary.buffer, binary.byteOffset + byteOffset, featuresLength);
var typedArray = binaryAccessor.createArrayBufferView(binaryBody.buffer, binaryBody.byteOffset + byteOffset, featuresLength);

if (!defined(binaryProperties)) {
binaryProperties = {};
Expand All @@ -322,6 +363,10 @@ define([
}
}
return binaryProperties;
}

Cesium3DTileBatchTable.getBinaryProperties = function(featuresLength, batchTableJson, batchTableBinary) {
return getBinaryProperties(featuresLength, batchTableJson, batchTableBinary);
};

function getByteLength(batchTable) {
Expand Down Expand Up @@ -738,8 +783,7 @@ define([
Check.typeOf.string('name', name);
//>>includeEnd('debug');

var json = this.batchTableJson;
return (defined(json) && defined(json[name])) || (defined(this._batchTableHierarchy) && hasPropertyInHierarchy(this, batchId, name));
return (defined(this._properties[name])) || (defined(this._batchTableHierarchy) && hasPropertyInHierarchy(this, batchId, name));
};

Cesium3DTileBatchTable.prototype.getPropertyNames = function(batchId, results) {
Expand All @@ -750,12 +794,8 @@ define([
results = defined(results) ? results : [];
results.length = 0;

var json = this.batchTableJson;
for (var name in json) {
if (json.hasOwnProperty(name)) {
results.push(name);
}
}
var propertyNames = Object.keys(this._properties);
results.push.apply(results, propertyNames);
Copy link
Contributor

Choose a reason for hiding this comment

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

The reason for the more verbose form before was to avoid allocating an array (like Object.keys would) if results is supplied. In practice I'm not sure if it really matters for this specific function though, so the change is ok.


if (defined(this._batchTableHierarchy)) {
getPropertyNamesInHierarchy(this, batchId, results);
Expand All @@ -770,18 +810,14 @@ define([
Check.typeOf.string('name', name);
//>>includeEnd('debug');

if (!defined(this.batchTableJson)) {
return undefined;
}

if (defined(this._batchTableBinaryProperties)) {
var binaryProperty = this._batchTableBinaryProperties[name];
if (defined(binaryProperty)) {
return getBinaryProperty(binaryProperty, batchId);
}
}

var propertyValues = this.batchTableJson[name];
var propertyValues = this._properties[name];
if (defined(propertyValues)) {
return clone(propertyValues[batchId], true);
}
Expand Down Expand Up @@ -817,17 +853,11 @@ define([
}
}

if (!defined(this.batchTableJson)) {
// Tile payload did not have a batch table. Create one for new user-defined properties.
this.batchTableJson = {};
}

var propertyValues = this.batchTableJson[name];

var propertyValues = this._properties[name];
if (!defined(propertyValues)) {
// Property does not exist. Create it.
this.batchTableJson[name] = new Array(featuresLength);
propertyValues = this.batchTableJson[name];
this._properties[name] = new Array(featuresLength);
propertyValues = this._properties[name];
}

propertyValues[batchId] = clone(value, true);
Expand Down Expand Up @@ -1466,6 +1496,23 @@ define([
}
};

/**
* Returns the contents of the batch table extension, or <code>undefined</code>
* if the extension is not present.
* @param {String} extensionName The name of the extension.
*
* @returns {Object|undefined} The contents of the batch table extension, or <code>undefined</code>
* if the extension is not present.
*/
Cesium3DTileBatchTable.prototype.getExtension = function(extensionName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function isn't used outside this class. Should it be a local function, or are there plans to use it externally?

var extensions = this._extensions;
if (!defined(extensions)) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This check isn't required since _extensions defaults to an empty object.


return extensions[extensionName];
};

Cesium3DTileBatchTable.prototype.isDestroyed = function() {
return false;
};
Expand Down
16 changes: 16 additions & 0 deletions Source/Scene/Cesium3DTileset.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ define([
this._asset = undefined; // Metadata for the entire tileset
this._properties = undefined; // Metadata for per-model/point/etc properties
this._geometricError = undefined; // Geometric error when the tree is not rendered at all
this._extensionsUsed = undefined;
this._gltfUpAxis = undefined;
this._processingQueue = [];
this._selectedTiles = [];
Expand Down Expand Up @@ -709,6 +710,7 @@ define([
that._asset = tilesetJson.asset;
that._properties = tilesetJson.properties;
that._geometricError = tilesetJson.geometricError;
that._extensionsUsed = tilesetJson.extensionsUsed;
that._gltfUpAxis = gltfUpAxis;
that._readyPromise.resolve(that);
}).otherwise(function(error) {
Expand Down Expand Up @@ -1915,6 +1917,20 @@ define([
}
};

/**
* <code>true</code> if this the tileset JSON file lists the extension in extensionsUsed; otherwise, <code>false</code>.
Copy link
Contributor

Choose a reason for hiding this comment

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

if this the tileset JSON file - small typo. Same below.

* @param {String} extensionName The name of the extension to check.
*
* @returns {Boolean} <code>true</code> if this the tileset JSON file lists the extension in extensionsUsed; otherwise, <code>false</code>.
*/
Cesium3DTileset.prototype.hasExtension = function(extensionName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Write a test for this function.

if (!defined(this._extensionsUsed)) {
return false;
}

return (this._extensionsUsed.indexOf(extensionName) > -1);
};

/**
* Returns true if this object was destroyed; otherwise, false.
* <br /><br />
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"asset": {
"version": "1.0"
"version": "0.0"
},
"geometricError": 70,
"root": {
Expand Down Expand Up @@ -41,7 +41,13 @@
},
"geometricError": 0,
"content": {
"url": "tile.b3dm"
"uri": "tile.b3dm"
}
}
},
"extensionsUsed": [
"3DTILES_batch_table_hierarchy"
],
"extensionsRequired": [
"3DTILES_batch_table_hierarchy"
]
}
Binary file not shown.
Loading