CesiumGS / cesium

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

`Model` - support `noData` and `required` from `EXT_mesh_features` #9887

Closed ptrgags closed 2 years ago

ptrgags commented 2 years ago

One of the schema changes from EXT_mesh_features is to remove default and optional from the schema and replace them with noData and required. The CesiumJS implementation still needs to be updated for this.

This was out of scope for #9880 since it has wider reaching implications. I want to wait on this until getting metadata working in Custom Shaders

ptrgags commented 2 years ago

Some more details:

lilleyse commented 2 years ago

should the noData value be passed in to the shader and the user is responsible for interpreting it?

Yeah I think that's a good path to start

lilleyse commented 2 years ago

@ptrgags could you post a new summary of the changes required here? Some things have changed in the spec and implementation since - notably defaultValue was added back to the spec and property textures and property attributes were added to custom shaders (which means we can actually start implementing this).

Afterwards we can get noData working for voxels: https://github.com/CesiumGS/cesium/pull/10253

ptrgags commented 2 years ago

Looks like a few details have already been accounted for:

Remaining details:

Future:

lilleyse commented 2 years ago

not sure if ternary operators lead to divergence

Might depend on the compiler and complexity of the expressions. czm_branchFreeTernary could be used instead.

lilleyse commented 2 years ago

Hm this brings up bigger questions about the custom shaders API. We also need to consider min, max, and statistics.

What about sub-structs for these special properties? Only problem is it could collide with actual property names.

metadata.noData.propertyName
metadata.default.propertyName
metadata.min.propertyName
metadata.max.propertyName
metadata.statistics.propertyName.min
metadata.statistics.propertyName.max
metadata.statistics.propertyName.occurrences

... other statistics properties including user-defined properties...

Related areas of the spec:

The voxels branch already does it this way for statistics.

lilleyse commented 2 years ago

It seems like this issue could be split into two PRs:

jjhembd commented 2 years ago

What about sub-structs for these special properties? Only problem is it could collide with actual property names.

Two options to avoid the naming conflict:

1. Move the property value to metadata.propertyName.value

metadata.propertyName.value
metadata.propertyName.noData
metadata.propertyName.default
  ...
metadata.propertyName.statistics.mean
  ...

This would be a breaking change. Current applications get the property value directly as metadata.propertyName

2. Add a new top-level property to the Vertex/FragmentInput struct

struct VertexInput {
    // Processed attribute values.
    Attributes attributes;
    // Feature IDs/Batch IDs.
    FeatureIds featureIds;
    // Metadata property values.
    Metadata metadata;
    // Information about metadata properties.
    MetadataClassInfo metadataClassInfo;
};

The per-vertex or per-fragment property values would still be accessed as metadata.propertyName. Constant values defined in the schema would be accessed from metadataClassInfo as follows:

metadataClassInfo.propertyName.noData
metadataClassInfo.propertyName.default
  ...
metadataClassInfo.propertyName.statistics.median
  ...
ptrgags commented 2 years ago

@jjhembd I feel a bit undecided here, but leaning towards the second option since it keeps the basic value access simple (vsInput.metadata.propertyName) rather than nesting it deeper in .value. Though it perhaps is a bit weird to keep one value separate from the rest of the struct.

Side note, overall this input struct feels like it is getting cumbersome to work with given how long the names become. vsInput.metadataClassInfo.propertyName.statistics.median... If we ever qualified property names (see https://github.com/CesiumGS/cesium/issues/10085) it would be 7 levels deep in the worst case: vsInput.metadataClassInfo.schemaName.className.propertyName.statistics.median... not ergonomic for writing shaders.

ptrgags commented 2 years ago

Just jotting down one note from a conversation with @jjhembd, instead of nesting the statistics inside the MetadataClassInfo struct, maybe there could be a separate MetadataStatistics struct so it would be fsInput.metadataStatistics.propertyName.min? this would save a level of nesting.

Less nesting is better not just for ergonomics for the user, but also makes the GLSL code generation easier (fewer dynamic structs to generate)

jjhembd commented 2 years ago

10520 brought properties from the metadata Class Property into the shader.

The next step is to bring in Property Statistics. In some ways, this is conceptually similar to what we did in #10520. But there are 3 additional complexities:

1. Property types are more complicated The properties min, max, median, and sum can be assumed to be of the same type as the property value. But properties mean, standardDeviation, and variance are inherently float values. We therefore need a type conversion function, to set up float mean, float standardDeviation, float variance for int property values, and vec2 mean for ivec2 property values, etc.

2. Values of statistics are harder to retrieve Class Properties were available by simply exposing the private members from Scene/PropertyAttributeProperty and Scene/PropertyTextureProperty. I have not yet been able to trace back to where Property Statistics can be retrieved.

3. enum types require special handling enum metadata will not have min, max, mean, median, sum, standardDeviation, or variance properties in the statistics. Instead, this type of metadata has an occurrences property. In the shader, the occurrences type will be an array of integers, with values corresponding to the number of times the value at that index in the enum occurs in the dataset. I will need a little more time to understand some nuances in the spec.

These factors will affect the time estimate for the next PR. I will follow up with a couple questions.

jjhembd commented 2 years ago

@ptrgags, can you point me to where I can find answers to these 2 questions?

  1. Where can I retrieve values for Property Statistics?
  2. What does this phrase mean, in the spec for occurrences?
    For fixed-length arrays, this is an array of component-wise occurrences.

    Do we have enums with array values?

ptrgags commented 2 years ago
  1. Note that statistics only exist in 3DTILES_metadata (the 3D Tiles extension), not the glTF extension. You can access it through tileset.metadataExtension.statistics, that just gives you an object that matches the statistics schema
  2. This is the number of occurrences for enum values, see here: https://github.com/CesiumGS/3d-tiles/blob/f9ad4e1a4c753b91262c63121b3124f38ffe49ff/extensions/3DTILES_metadata/schema/statistics.class.property.schema.json#L69-L87

Do we have enums with array values?

No, but we have arrays of enum values:

propertyEnumArray: {
  type: "ENUM",
  array: true,
  count: 3
  enumType: "myEnum",
},

// so the statistics should be an array with occurrences for index 0, index 1, index 2...
lilleyse commented 2 years ago

Note that statistics only exist in 3DTILES_metadata (the 3D Tiles extension), not the glTF extension. You can access it through tileset.metadataExtension.statistics, that just gives you an object that matches the statistics schema

Statistics also exist for 3D Tiles 1.1. @ptrgags is the API the same for both?

ptrgags commented 2 years ago

Ah yes, sorry for the confusion. Yes, metadataExtension has the same API for both 3DTILES_metadata and 3D Tiles 1.1 (both are parsed in processMetadataExtension() in Cesium3DTileset)

jjhembd commented 2 years ago

Thanks @ptrgags! So if I understand you correctly, the glTF files we are currently using in MetadataPipelineStage spec will not have statistics. I will need to:

  1. Add a test loading a tileset in MetadataPipelineStageSpec
  2. Make metadataStatistics struct construction contingent on the presence of statistics in the input
jjhembd commented 2 years ago

I'm still trying to trace back the link between renderResources.model in MetadataPipelineStage and the tileset.metadataExtension.

ptrgags commented 2 years ago

@jjhembd There are some parent pointers that you can use to locate the tileset:

These pointers skip over a few levels of abstraction. To recap the full hierarchy here (from root to leaves):

ptrgags commented 2 years ago

Also one thing to consider when implementing statistics (and this goes for anything else that comes from tileset/tile/content metadata as well), is that this information is coarser-grained, so as far as the ModelExperimental is concerned, this information is constant. So it could be passed in as either a uniform, or even hard-coded when generating the shader code. It might make sense to do the hard-coding (where reasonable) to avoid taking up more uniform location (plus in WebGL 1 each uniform update has to be done separately = more WebGL API calls)

jjhembd commented 2 years ago

As discussed with @ptrgags: we need to load a 3DTileset in MetadataPipelineStageSpec to test statistics. This took me a little while to understand and get working. Some key steps included:

  1. Set scene.camera to look at the tileset data

  2. Load the tileset using Cesium3DTilesTester, with ModelExperimental enabled:

    const tilesetOptions = { enableModelExperimental: true };
    return Cesium3DTilesTester.loadTileset(
    scene,
    tilesetWithMetadata,
    tilesetOptions
    ).then(tileset => {  // ...
  3. Retrieve model from tileset.root.content._model, and use it to mock renderResources

  4. Retrieve a sample primitive from model.sceneGraph.components.nodes[0].primitives[0] for the call to MetadataPipelineStage.process

I couldn't find a sample tileset with statistics. So I spent some time understanding the structure of this tileset:

Specs/Data/Cesium3DTiles/Metadata/AllMetadataTypes/tileset_1.1.json

and am now adding a statistics field with fake values for the test. (Let me know if there's a quick way to get actual values for min, max, mean, etc.)

jjhembd commented 2 years ago

One upcoming challenge for MetadataPipelineStage: the types as listed in the tileset.json are not always what we need in the shader. For example, a property with "componentType": "UINT16" and "normalized": true will actually be loaded as a float or vecN type. Two possible ways to handle this:

lilleyse commented 2 years ago

we need to load a 3DTileset in MetadataPipelineStageSpec to test statistics.

Can we mock the tileset like in PickingPipelineStageSpec? Generally I'm not a fan of mocking objects but it might drastically simplify the test.

Or we should step back and think about whether statistics should be passed along to the model so that it doesn't have to reach back to the tileset.

lilleyse commented 2 years ago

Use the MetadataStatistics property name to look up the type used by the corresponding PropertyTextureProperty or PropertyAttributeProperty

This option seems cleaner to me. It can leverage some of the existing code.

jjhembd commented 2 years ago

A significant part of what this issue describes has been implemented in #10520 and #10683.

I opened two new issues, #10799 and #10800, to better define the remaining features that have not yet been implemented.