Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support KHR_texture_transform for feature ID textures #1360

Merged
merged 11 commits into from
Feb 26, 2024

Conversation

j9liu
Copy link
Contributor

@j9liu j9liu commented Feb 21, 2024

Depends on #1355 (which I still have to get working, sorry 😭)

This adds support for KHR_texture_transform for feature ID textures, both with picking and styling. To make sure this actually worked, I used the sample model that CesiumJS uses for its specs: https://github.com/CesiumGS/cesium/tree/main/Specs/Data/Models/glTF-2.0/FeatureIdTextureWithTextureTransform/glTF.

HOWEVER, be sure to modify the glTF so that the feature ID set has "featureCount": 64,. This is required by the spec, so it's an error on the model's part.

To test picking:

  • Open the 06_CesiumMetadata sample level
  • Load a tileset with the aforementioned glTF in it.
  • Modify the Level Blueprint to print the feature ID like so:
    image
  • Press play. If correct, the very top-left pixel should return the value "54". The bottom-right pixel will return 153.

To test styling:

  • Add a CesiumFeaturesMetadata component to the tileset
  • Click Auto Fill. You should see a feature ID texture with the "Has Khr Texture Transform" flag = true
  • Click Generate Material. There should be nodes for the scale / offset / rotation from KHR_texture_transform:
    image
  • The base color texture is also used for feature IDs, so if the texture transform works, the square should appear exactly the same. Configure the material graph to look like so.
    image
  • Create a new material instance with this as the layer (and remember to name it "FeaturesMetadata"!). Then assign it to the tileset. It should look exactly the same. You can test this by clicking the "revert" button next to the material and checking if the appearance changes.
    image

Bonus Changes:

  • Reorganized the way Material Functions were passed in CesiumFeaturesMetadataComponent because I initially was using other material functions. But I've removed them and am just keeping the new organization because it seems cleaner.
  • Vector parameters were hooked incorrectly for Feature ID textures, and I re-wrote the code to connect them properly. I will sweep through the other parameters and check for correctness in a following PR.

@kring kring changed the base branch from update-for-zlib-ng to main February 23, 2024 08:41
@kring
Copy link
Member

kring commented Feb 26, 2024

Thanks @j9liu!

@kring kring merged commit 405b236 into main Feb 26, 2024
23 checks passed
@kring kring deleted the feature-id-texture-transform branch February 26, 2024 00:32
@javagl
Copy link
Contributor

javagl commented Feb 27, 2024

@j9liu

HOWEVER, be sure to modify the glTF so that the feature ID set has "featureCount": 64,. This is required by the spec, so it's an error on the model's part.

Just drop me a note when you notice something like this - I added this file without validating, so that's my fault.

Fix is in CesiumGS/cesium#11863

@j9liu
Copy link
Contributor Author

j9liu commented Feb 27, 2024

Thanks for noticing @javagl -- I intended to create a fix, but got completely distracted. 😓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants