Point cloud styling fixes

CHANGES.md

@@ -10,6 +10,7 @@
 
 - Fixed a bug that could cause rendering of a glTF model to become corrupt when switching from a Uint16 to a Uint32 index buffer to accomodate new vertices added for edge outlining. [#8820](https:/CesiumGS/cesium/pull/8820)
 - This fixes a bug where a removed billboard can prevent changing of the terrainProvider [#8766](https:/CesiumGS/cesium/pull/8766)
+- Fixed an issue with 3D Tiles point cloud styling where `${feature.propertyName}` and `${feature["propertyName"]}` syntax would cause a crash. Also fixed an issue where property names with non-alphanumeric characters would crash. [#8785](https:/CesiumGS/cesium/pull/8785)
 
 ### 1.69.0 - 2020-05-01
 

Source/Scene/Cesium3DTileStyle.js

@@ -1638,7 +1638,7 @@ Object.defineProperties(Cesium3DTileStyle.prototype, {
  * Gets the color shader function for this style.
  *
  * @param {String} functionName Name to give to the generated function.
- * @param {String} attributePrefix Prefix that is added to any variable names to access vertex attributes.
+ * @param {String} propertyNameMap Maps property variable names to shader attribute names.
  * @param {Object} shaderState Stores information about the generated shader function, including whether it is translucent.
  *
  * @returns {String} The shader function.
@@ -1647,7 +1647,7 @@ Object.defineProperties(Cesium3DTileStyle.prototype, {
  */
 Cesium3DTileStyle.prototype.getColorShaderFunction = function (
  functionName,
- attributePrefix,
+ propertyNameMap,
  shaderState
 ) {
  if (this._colorShaderFunctionReady) {
@@ -1660,7 +1660,7 @@ Cesium3DTileStyle.prototype.getColorShaderFunction = function (
  this._colorShaderFunction = defined(this.color)
  ? this.color.getShaderFunction(
  functionName,
- attributePrefix,
+ propertyNameMap,
  shaderState,
  "vec4"
  )
@@ -1673,7 +1673,7 @@ Cesium3DTileStyle.prototype.getColorShaderFunction = function (
  * Gets the show shader function for this style.
  *
  * @param {String} functionName Name to give to the generated function.
- * @param {String} attributePrefix Prefix that is added to any variable names to access vertex attributes.
+ * @param {String} propertyNameMap Maps property variable names to shader attribute names.
  * @param {Object} shaderState Stores information about the generated shader function, including whether it is translucent.
  *
  * @returns {String} The shader function.
@@ -1682,7 +1682,7 @@ Cesium3DTileStyle.prototype.getColorShaderFunction = function (
  */
 Cesium3DTileStyle.prototype.getShowShaderFunction = function (
  functionName,
- attributePrefix,
+ propertyNameMap,
  shaderState
 ) {
  if (this._showShaderFunctionReady) {
@@ -1694,7 +1694,7 @@ Cesium3DTileStyle.prototype.getShowShaderFunction = function (
  this._showShaderFunction = defined(this.show)
  ? this.show.getShaderFunction(
  functionName,
- attributePrefix,
+ propertyNameMap,
  shaderState,
  "bool"
  )
@@ -1706,7 +1706,7 @@ Cesium3DTileStyle.prototype.getShowShaderFunction = function (
  * Gets the pointSize shader function for this style.
  *
  * @param {String} functionName Name to give to the generated function.
- * @param {String} attributePrefix Prefix that is added to any variable names to access vertex attributes.
+ * @param {String} propertyNameMap Maps property variable names to shader attribute names.
  * @param {Object} shaderState Stores information about the generated shader function, including whether it is translucent.
  *
  * @returns {String} The shader function.
@@ -1715,7 +1715,7 @@ Cesium3DTileStyle.prototype.getShowShaderFunction = function (
  */
 Cesium3DTileStyle.prototype.getPointSizeShaderFunction = function (
  functionName,
- attributePrefix,
+ propertyNameMap,
  shaderState
 ) {
  if (this._pointSizeShaderFunctionReady) {
@@ -1727,7 +1727,7 @@ Cesium3DTileStyle.prototype.getPointSizeShaderFunction = function (
  this._pointSizeShaderFunction = defined(this.pointSize)
  ? this.pointSize.getShaderFunction(
  functionName,
- attributePrefix,
+ propertyNameMap,
  shaderState,
  "float"
  )

Source/Scene/ConditionsExpression.js

@@ -136,7 +136,7 @@ ConditionsExpression.prototype.evaluateColor = function (feature, result) {
  * Returns undefined if the shader function can't be generated from this expression.
  *
  * @param {String} functionName Name to give to the generated function.
- * @param {String} attributePrefix Prefix that is added to any variable names to access vertex attributes.
+ * @param {String} propertyNameMap Maps property variable names to shader attribute names.
  * @param {Object} shaderState Stores information about the generated shader function, including whether it is translucent.
  * @param {String} returnType The return type of the generated function.
  *
@@ -146,7 +146,7 @@ ConditionsExpression.prototype.evaluateColor = function (feature, result) {
  */
 ConditionsExpression.prototype.getShaderFunction = function (
  functionName,
- attributePrefix,
+ propertyNameMap,
  shaderState,
  returnType
 ) {
@@ -161,11 +161,11 @@ ConditionsExpression.prototype.getShaderFunction = function (
  var statement = conditions[i];
 
  var condition = statement.condition.getShaderExpression(
- attributePrefix,
+ propertyNameMap,
  shaderState
  );
  var expression = statement.expression.getShaderExpression(
- attributePrefix,
+ propertyNameMap,
  shaderState
  );
 

Source/Scene/Expression.js

@@ -171,7 +171,7 @@ Expression.prototype.evaluateColor = function (feature, result) {
  * Returns undefined if the shader function can't be generated from this expression.
  *
  * @param {String} functionName Name to give to the generated function.
- * @param {String} attributePrefix Prefix that is added to any variable names to access vertex attributes.
+ * @param {String} propertyNameMap Maps property variable names to shader attribute names.
  * @param {Object} shaderState Stores information about the generated shader function, including whether it is translucent.
  * @param {String} returnType The return type of the generated function.
  *
@@ -181,11 +181,11 @@ Expression.prototype.evaluateColor = function (feature, result) {
  */
 Expression.prototype.getShaderFunction = function (
  functionName,
- attributePrefix,
+ propertyNameMap,
  shaderState,
  returnType
 ) {
- var shaderExpression = this.getShaderExpression(attributePrefix, shaderState);
+ var shaderExpression = this.getShaderExpression(propertyNameMap, shaderState);
 
  shaderExpression =
  returnType +
@@ -205,18 +205,18 @@ Expression.prototype.getShaderFunction = function (
  * Gets the shader expression for this expression.
  * Returns undefined if the shader expression can't be generated from this expression.
  *
- * @param {String} attributePrefix Prefix that is added to any variable names to access vertex attributes.
+ * @param {String} propertyNameMap Maps property variable names to shader attribute names.
  * @param {Object} shaderState Stores information about the generated shader function, including whether it is translucent.
  *
  * @returns {String} The shader expression.
  *
  * @private
  */
 Expression.prototype.getShaderExpression = function (
- attributePrefix,
+ propertyNameMap,
  shaderState
 ) {
- return this._runtimeAst.getShaderExpression(attributePrefix, shaderState);
+ return this._runtimeAst.getShaderExpression(propertyNameMap, shaderState);
 };
 
 var unaryOperators = ["!", "-", "+"];
@@ -1967,23 +1967,35 @@ function colorToVec4(color) {
  return "vec4(" + r + ", " + g + ", " + b + ", " + a + ")";
 }
 
-function getExpressionArray(array, attributePrefix, shaderState, parent) {
+function getExpressionArray(array, propertyNameMap, shaderState, parent) {
  var length = array.length;
  var expressions = new Array(length);
  for (var i = 0; i < length; ++i) {
  expressions[i] = array[i].getShaderExpression(
- attributePrefix,
+ propertyNameMap,
  shaderState,
  parent
  );
  }
  return expressions;
 }
 
+function getVariableName(variableName, propertyNameMap) {
+ if (!defined(propertyNameMap[variableName])) {
+ throw new RuntimeError(
+ 'Style references a property "' +
+ variableName +
+ '" that does not exist or is not styleable.'
+ );
+ }
+
+ return propertyNameMap[variableName];
+}
+
 var nullSentinel = "czm_infinity"; // null just needs to be some sentinel value that will cause "[expression] === null" to be false in nearly all cases. GLSL doesn't have a NaN constant so use czm_infinity.
 
 Node.prototype.getShaderExpression = function (
- attributePrefix,
+ propertyNameMap,
  shaderState,
  parent
 ) {
@@ -1998,28 +2010,31 @@ Node.prototype.getShaderExpression = function (
  if (defined(this._left)) {
  if (Array.isArray(this._left)) {
  // Left can be an array if the type is LITERAL_COLOR or LITERAL_VECTOR
- left = getExpressionArray(this._left, attributePrefix, shaderState, this);
+ left = getExpressionArray(this._left, propertyNameMap, shaderState, this);
  } else {
- left = this._left.getShaderExpression(attributePrefix, shaderState, this);
+ left = this._left.getShaderExpression(propertyNameMap, shaderState, this);
  }
  }
 
  if (defined(this._right)) {
- right = this._right.getShaderExpression(attributePrefix, shaderState, this);
+ right = this._right.getShaderExpression(propertyNameMap, shaderState, this);
  }
 
  if (defined(this._test)) {
- test = this._test.getShaderExpression(attributePrefix, shaderState, this);
+ test = this._test.getShaderExpression(propertyNameMap, shaderState, this);
  }
 
  if (Array.isArray(this._value)) {
  // For ARRAY type
- value = getExpressionArray(this._value, attributePrefix, shaderState, this);
+ value = getExpressionArray(this._value, propertyNameMap, shaderState, this);
  }
 
  switch (type) {
  case ExpressionNodeType.VARIABLE:
- return attributePrefix + value;
+ if (checkFeature(this)) {
+ return undefined;
+ }
+ return getVariableName(value, propertyNameMap);
  case ExpressionNodeType.UNARY:
  // Supported types: +, -, !, Boolean, Number
  if (value === "Boolean") {
@@ -2071,6 +2086,9 @@ Node.prototype.getShaderExpression = function (
  case ExpressionNodeType.CONDITIONAL:
  return "(" + test + " ? " + left + " : " + right + ")";
  case ExpressionNodeType.MEMBER:
+ if (checkFeature(this._left)) {
+ return getVariableName(right, propertyNameMap);
+ }
  // This is intended for accessing the components of vector properties. String members aren't supported.
  // Check for 0.0 rather than 0 because all numbers are previously converted to decimals.
  if (right === "r" || right === "x" || right === "0.0") {
@@ -2132,7 +2150,8 @@ Node.prototype.getShaderExpression = function (
  value === "x" ||
  value === "y" ||
  value === "z" ||
- value === "w"
+ value === "w" ||
+ checkFeature(parent._left)
  ) {
  return value;
  }