Property table custom shader

[mzschwartz5]

Description

**Note: depends on #13155**

This PR adds shader generation logic in the MetadataPipelineStage for binding and sampling property tables (represented as textures).

This is the last PR in a series to support property tables in custom shaders. There are several future follow-ups:

• Not all property types are supported in custom shaders, but many more useful ones are.
• Further, this support only extends to WebGL 2.
• Also out of scope for now - support for updating properties dynamically and having those updates be reflected in custom shaders.
• Finally, primitives with multiple feature ID sets that access the same property table are unsupported. This could possible be supported in the future (Cesium3DTileset constructor options support specifying a feature ID set).

Also, TODO: update documentation (here, and here). I think that may be a follow-up PR.

Issue number and link

#13124

Testing plan

Testing plan for #13155 still applies - as regression testing.

Additionally, this sandcastle tests actually using a property table in a custom shader to visualize boreholes and modulate their radii based on metadata. The example tileset used has three metadata properties accessible via the shader: drillingDiameter, fromDepth and toDepth (describing the start and end point of each borehole segment to which a drillingDiameter metadata sample applies).

Author checklist

• I have submitted a Contributor License Agreement
• I have added my name to CONTRIBUTORS.md
• I have updated CHANGES.md with a short summary of my change
• I have added or updated unit tests to ensure consistent code coverage
• I have updated the inline documentation, and included code examples where relevant
• I have performed a self-review of my code

[github-actions]

Thank you for the pull request, @mzschwartz5!

✅ We can confirm we have a CLA on file for you.

[mzschwartz5]

This isn't always the case... see this sample tileset that explicitly tests multiple feature ID sets for a single primitive.. (Which you can test via the sandcastle linked in the testing steps on the last PR)

However, if a single primitive has multiple feature ID sets, it's ambiguous which one we should use to sample the property table (indexed by (featureID, propertyID)) in the shader. Thus, we just use the first feature ID set we find and log a warning.

There may be a better way to do this in the future, like using an "active" feature ID set or a user-specified one. But this is also a really niche use-case that realistically we don't need to support.

[javagl]

Following up from the internal discussion: There is https://cesium.com/learn/cesiumjs/ref-doc/Cesium3DTileset.html#featureIdLabel , and one could make a strong case that this, which currently says

Label of the feature ID set to use for picking and styling.

should say

Label of the feature ID set to use for picking and styling and property table access

or something like that.

But... having no clue about how complicated this is (I haven't read the whole changes in all related PRs), this is just a hint, and may be tracked as a possible follow-up for the future.

[mzschwartz5]

Yes I did see that. I would need to look into how that setting would plumb down to the metadata pipeline stage, and how it should be made available for use in a shader (uniform?).

It might be very easy. It's just that, given the scope of these changes already, I think it might be best as a follow-up. I'll add it to the follow-ups listed in the description.

[mzschwartz5]

A lot of the changes in this file are just moving code from PropertyTextureProperty.js to this, lower-level class, so that it can be reused by property table properties, which are now also backed by textures.

There are some changes to the moved functions as well, though. May be handy to paste the functions into vscode and run a diff or something.

[mzschwartz5]

This test became flaky after these changes. Not entirely sure what broke the camel's back, so to speak, but this should be more robust now - waiting for tiles to load before asserting anything.

[mzschwartz5]

Again, the changes here are mainly just moving tests to go along with the code that was moved.

[lilleyse]

I confirmed that this is working for instanced and non-instanced models 👍

Sandcastle code

[lilleyse]

I'm testing with a 3D Tiles 1.0 asset (using b3dm and batch table) and the custom shader doesn't seem to pick up the properties. This might be due to it using batch table JSON instead of batch table binary.

Sandcastle code

[javagl]

@lilleyse

The same error appears on the currently depolyed sandcastle: https://sandcastle.cesium.com/index.html#c=jVLda9swEP9XDj85EORkfevcMEgZFFb6kGxPhlWVzsmt+giS7Kwp/d8nW0qarn0oGJ/ufh9nnU9Y4wP0hHt0cAUG97BET51mv8Za2RRizJfWBE4GXVNMvjamMWJUBlLoMUQp33MKR3EKF9frBLPWWf3TqejGWBWfFdc7hdc88OoNtcqC06HKDdgfb82xdfpc5gUaZDtHmgL16BmXssz8gZhpB2v12p4DjTm6is4Hq1dbLv+//vIMKZ8bA9A6vtFoQqqt8W+4hPsBAOgtyRN+G8dUfs/Jjdl1AVo/ximQsTEVB/1bW4nqlgd0xBXofJgkv+cUYk9leYAt0mY7zDj7MI2Byzg8lpDxSklALZSZXsOXGZtNXs3g1IZJatvOY7TsUVyUczabwiy/hgkl+gugiqRPGIza+XuDdBjD/bQxL/F+xbSofXhSuBiq30jvrAvQxeWIexEw7kXs4auHTjwOP8j7wa6ujpJaUg8krz7YSxCKex+RtlNqRQdsikVdRf4bWZyoJLO569Ep/jRQtvPFj1RkjNVVTN+rgrXqgbszx38

And I checked: This is not caused by the fact that the property is called Height (and not height) 🙂

[lilleyse]

The same error appears on the currently depolyed sandcastle: https://sandcastle.cesium.com/index.html#c=jVLda9swEP9XDj85EORkfevcMEgZFFb6kGxPhlWVzsmt+giS7Kwp/d8nW0qarn0oGJ/ufh9nnU9Y4wP0hHt0cAUG97BET51mv8Za2RRizJfWBE4GXVNMvjamMWJUBlLoMUQp33MKR3EKF9frBLPWWf3TqejGWBWfFdc7hdc88OoNtcqC06HKDdgfb82xdfpc5gUaZDtHmgL16BmXssz8gZhpB2v12p4DjTm6is4Hq1dbLv+//vIMKZ8bA9A6vtFoQqqt8W+4hPsBAOgtyRN+G8dUfs/Jjdl1AVo/ximQsTEVB/1bW4nqlgd0xBXofJgkv+cUYk9leYAt0mY7zDj7MI2Byzg8lpDxSklALZSZXsOXGZtNXs3g1IZJatvOY7TsUVyUczabwiy/hgkl+gugiqRPGIza+XuDdBjD/bQxL/F+xbSofXhSuBiq30jvrAvQxeWIexEw7kXs4auHTjwOP8j7wa6ujpJaUg8krz7YSxCKex+RtlNqRQdsikVdRf4bWZyoJLO569Ep/jRQtvPFj1RkjNVVTN+rgrXqgbszx38

I think that's expected until this PR is merged.

And I checked: This is not caused by the fact that the property is called Height (and not height) 🙂

I checked both as well because I wasn't sure about the property id sanitization rules. But it seems like it should be Height instead of height. I updated the sandcastle.

Property ID Sanitization

GLSL only supports alphanumeric identifiers, i.e. identifiers that do not
start with a number. Additionally, identifiers with consecutive
underscores (__), as well as identifiers with the gl_ prefix, are
reserved in GLSL. To circumvent these limitations, the property IDs are
modified as follows:

1. Replace all sequences of non-alphanumeric characters with a single _.
2. Remove the reserved gl_ prefix if present.
3. If the identifier begins with a digit ([0-9]), prefix with an _

[javagl]

I think that's expected until this PR is merged.

I wasn't entirely sure about that - yesterday, I looked at #10480 and wondered whehther there is anything explicit about the compatibility of "old/legacy" things (like batch tables) with "new" things (like custom shaders). Specifically: Batch tables should work for styling, but I wasn't sure about custom shaders. (I know that there are some translations between them under the hood, but don't know all details)

But it seems like it should be Height instead of height.

Right now, in the shader compliation error message, one can see that the metadata structs only contain _empty, so it's not an issue of case sensitivity (but I assume that when they are not empty, it will have to be Height).

[mzschwartz5]

I should add that it's not my intention to add any new features for the legacy metadata workflows. So if they didn't work with custom shaders before, they probably won't now. I only want to make sure I'm not actively breaking any workflows we still support.