CesiumGS / cesium

An open-source JavaScript library for world-class 3D globes and maps :earth_americas:
https://cesium.com/cesiumjs/
Apache License 2.0
13.04k stars 3.51k forks source link

property check fails in style expression #8413

Open cr8tpro opened 5 years ago

cr8tpro commented 5 years ago

I am going to show points on Point Cloud according to Classification. If "Classification" property exists, style expression works fine below ['${Classification} === ' + id, color]

But if "Classification" property does not exist, I get this error message. RuntimeError: Style references a property "Classification" that does not exist or is not styleable.

So I tried to like this in expression. "defines" : { "Classification" : "${Classification} !== Undefined ? ${Classification} : -1" },

But still get error with this message DeveloperError: The style is not loaded. Use Cesium3DTileStyle.readyPromise or wait for Cesium3DTileStyle.ready to be true.

I saw Undefined is avilable to use and @lilleyse mentioned at #4637 that it is available to use {MyProperty} !== undefined. Also I checked it is available to use Undefined at https://github.com/AnalyticalGraphicsInc/3d-tiles/blob/master/specification/Styling/README.md#reference-point-cloud-style.

If Cesium doesn't support this, I think there should be built-in function to check property in PointCloud.

lilleyse commented 5 years ago

The GLSL styling backend for point clouds has some limitations compared to the JS backend that's used for b3dm/i3dm. One of those is lack of support for undefined.

Implementation Note: Point cloud styling engines may often use a shader (GLSL) implementation, however some features of the expression language are not possible in pure a GLSL implementation. Some of these features include:

  • Evaluation of isNan and isFinite (GLSL 2.0+ supports isnan and isinf for these functions respectively)
  • The types null and undefined
  • Strings, including accessing object properties (color()['r']) and batch table values
  • Regular expressions
  • Arrays of lengths other than 2, 3, or 4
  • Mismatched type comparisons (e.g. 1.0 === false)
  • Array index out of bounds

From https://github.com/AnalyticalGraphicsInc/3d-tiles/blob/master/specification/Styling/README.md#point-cloud

This is something we'd like to fix eventually. Thanks for submitting an issue.

As a side note - we want to make the list of properties more discoverable in tileset.json so that an app doesn't have to check for the existence of the property in the shader itself. This would include fixes to the tilers in ion and improvements to the spec.

OmarShehata commented 4 years ago

@lilleyse this has now been fixed after adding support for isNaN, undefined etc to the GLSL backend, right? In https://github.com/CesiumGS/cesium/pull/8621

lilleyse commented 4 years ago

@OmarShehata should be. I haven't checked the style above but it ought to work now.

OmarShehata commented 4 years ago

Ok, so it is possible to guard against a crash when using the JS backend for styling, by inserting a top-level check for IsNaN/undefined etc as shown here https://github.com/CesiumGS/cesium/pull/8621#issuecomment-588006150.

This does not solve the case in this issue, where you have a point cloud that doesn't have per-point classification, and you want to know at runtime if it has classification in order to apply the style. This also came up here: https://community.cesium.com/t/testing-for-3dtileset-properties/10930.

I think what we'd need to do here is either (1) Expose a way to list what per-point properties are available in a given tile, which I think is how Cesium Stories knows whether it's safe to apply styles for point clouds (by checking the root tile and assuming its properties are representative), or (2) standardize in the 3D Tiles spec that the tileset.json should contain this info.

anne-gropler commented 2 years ago

So the way undefined is implemented in #8621 for point cloud styles means that we can check if a number is undefined, but we cannot check if a vector is undefined. That's because undefined boils down to czm_infinity (a number) and in case the comparing vector is defined, we get GLSL error Vertex shader compile log: ERROR: 0:11: '==' : dimension mismatch ERROR: 0:11: '==' : wrong operand types - no operation '==' exists that takes a left-hand operand of type 'in highp 3-component vector of float' and a right operand of type 'const highp float' (or there is no acceptable conversion).

It's very unintuitive to define undefined using a high enough number, imho.

However, instead of checking if the vector equals undefined, using isNaN() actually does the trick. Maybe this helps someone.