Feature reactive Cesium3DTileStyle.style object

[bkuster]

This closes #7567.

Recap

The .style property was set on creation of Cesium3DTileStyle and no longer evaluated.

New Implementation

The .style property now reacts to property changes on the Cesium3DTileStyle. This allows cloning or serializing of a style, even after it has been updated.

[cesium-concierge]

Thanks for the pull request @bkuster!

• ✔️ Signed CLA found.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

[lilleyse]

Thanks @bkuster, I'll look at this soon.

[cesium-concierge]

Thanks again for your contribution @bkuster!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[hpinkos]

@lilleyse will you have time to review this soon?

[lilleyse]

Maybe it would be better to separate getting the expression from setting the json. One idea:

function getJsonFromExpression(expression) {
    if (defined(value.expression)) {
        return value.expression;
    } else if (defined(value.conditionsExpression)) {
        return clone(value.conditionsExpression, true);
    }
    return expression;
}

set : function(value) {
    this._show = getExpression(this, value);
    this._style.show = getJsonFromExpression(this._show);
}

A couple things to handle are:

• this._style would need to be initialized to an empty object
• What happens if the expression is a custom expression? Should that be added to the json as is? Or should getJsonFromExpression return undefined in that case.

[bkuster]

Thanks for the feedback @lilleyse. True, abusing getExpression in this fashion is not very intuitive, I like your proposal and will implement something along those lines.

1. this._style initialization in the constructor makes sense to me, but this would change a non-experimental API, as the default value for .style would not longer be undefined but Object, or would you still return undefined in the getter, if the Object is empty?
2. Haven't thought of custom expressions tbh. Both approaches (undefined or the expression instance) have their uses. You cant serialize the expression, but you could use it to clone the style at runtime. I would lean toward an implementation that is closest to the current behavior, I'll have to checkout what that is first, but would assume it to be undefined?

[bkuster]

Changed my mind on 2.. Behavior as is now adds the custom expression passed in the options to style, this should remain unchanged. Added some specs to outline the behavior.

[bkuster]

This is ready for a second review.

[lilleyse]

Thanks @bkuster, this all looks good to me. My simple test checks out:

var viewer = new Cesium.Viewer('cesiumContainer');

var tileset = new Cesium.Cesium3DTileset({
    url: '../../SampleData/Cesium3DTiles/PointCloud/PointCloudWithPerPointProperties/tileset.json'
});
viewer.scene.primitives.add(tileset);
viewer.zoomTo(tileset, new Cesium.HeadingPitchRange(0.0, -1.0, 50.0));

var style = new Cesium.Cesium3DTileStyle({
    color : "color('#ff0000')"
});

tileset.style = style;

console.log(style.style);

style.color = "color('#ffff00')";

console.log(style.style);

And the breaking change is minimal enough to not worry about deprecating first.