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

Fix incorrect metadata property values being sent to material #1524

Merged
merged 2 commits into from
Sep 25, 2024

Conversation

j9liu
Copy link
Contributor

@j9liu j9liu commented Sep 20, 2024

In our metadata handling code, I found some unfortunate typos that happened as a result of overzealous copy-pasting. For some reason the encodedProperty.offset value was being passed for the scale, noData, and default value properties, which is obviously wrong. 🙃 Plus, I had typoed a + sign as an = sign, so one parameter was never being set.

This PR does two things:

  1. Fixes the aforementioned bugs
  2. Moves the Set____ParameterValues functions from CesiumGltfComponent.cpp to CesiumEncodedFeaturesMetadata.h. This was not strictly necessary, but I think this organization is helpful for decreasing the complexity of CesiumGltfComponent, and for putting the code closer to the rest of the CesiumEncodedFeaturesMetadata functionality.

If you'd like, you can verify that these values are passed correctly using the CesiumJS sample dataset in Specs\Data\Cesium3DTiles\Metadata\StructuralMetadata. Notably the employeeCount, year, and temperatureCelsius properties will be affected.

For temperatureCelsius you can reproduce this result with these material nodes:

Result Nodes
image image

You can also get the same result by dividing the Raw Value by 255, and lerp-ing that.

For year you can reproduce this result with these nodes, though it simply tests that the default value is passed in.

Result Nodes
image image

For employeeCount you can reproduce this result with these nodes:

Result Nodes
image image

@kring
Copy link
Member

kring commented Sep 25, 2024

Looks good, thanks @j9liu!

@kring kring merged commit 8b0f050 into main Sep 25, 2024
23 checks passed
@kring kring deleted the metadata-material-fixes branch September 25, 2024 06:48
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.

2 participants