3D Tiles Styling - treat colors as vec4 #4849
Conversation
| @@ -212,7 +202,7 @@ define([ | |||
| Expression.prototype.evaluateColor = function(frameState, feature, result) { | |||
| ScratchStorage.reset(); | |||
| var color = this._runtimeAst.evaluate(frameState, feature); | |||
| return Color.clone(color, result); | |||
| return Color.fromCartesian4(color, result); | |||
There was a problem hiding this comment.
I figured evaluateColor should return a Color object. evaluate will just return a Cartesian4.
There was a problem hiding this comment.
Yes, agreed. It is fine for the Cesium implementation to map vec4 to Cesium's Color when we know it is the color property. This is an implementation detail and does not affect the spec.
| } else if (member === 'w') { | ||
| return property.alpha; | ||
| if ((property instanceof Cartesian2) || (property instanceof Cartesian3) || (property instanceof Cartesian4)) { | ||
| // Vector components may be accessed with .red, .green, .blue, .alpha and implicitly with .x, .y, .z, .w |
There was a problem hiding this comment.
Like how glsl can access components of a vector with .r, .g, .b, .a, I kept in the .red, .green, .blue, .alpha. Should they be shortened to .r, etc instead, or should this be removed?
There was a problem hiding this comment.
Let's keep it, but go with r, g, b, a. I went with the full words to start because I was copying the Cesium Color type, but we should stay consistent with GLSL so the styling language isn't too much of a hybrid.
Also, given that styles are often very concise and look like equations, this will fit well.
@pmconne this will be a small breaking change for your styles.
|
Code looks good. Don't forget to update the Sandcastle examples when you make the property renames. |
|
Updated. I couldn't find any uses in the examples. |
Based on discussion here: CesiumGS/3d-tiles#166 (comment).