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

Only issue a warning for channel mismatches #293

Merged
merged 2 commits into from
Jan 2, 2024
Merged

Conversation

javagl
Copy link
Contributor

@javagl javagl commented Dec 26, 2023

Until now, the validator made the assumption that image channels have 8 bits. This means that when someone tried to represent, for example, a 16-bit value in a property texture with a single channel, then the validator generated an ERROR with type TEXTURE_CHANNELS_OUT_OF_RANGE.

There still are some open questions about the handling of non-8-bit channels in images. Some thoughts and discussion are tracked in CesiumGS/3d-tiles#748 . But for now, the validator should not claim that assets that contain images with 16-bit channels are "invalid". So this PR changes the former ERROR in these cases into a WARNING with the type TEXTURE_CHANNELS_SIZE_MISMATCH.

The issue will therefore be reported as

        {
          "type": "TEXTURE_CHANNELS_SIZE_MISMATCH",
          "path": "example.glb/propertyTextures/0/properties/exampleProperty",
          "message": "The property 'exampleProperty' has the component type UINT16, with a size of 2 bytes, and the type SCALAR with 1 components, resulting in 2 bytes per element, but the number of channels in the property texture property was 1",
          "severity": "WARNING"
        }

but the asset will still be considered to be valid.

@lilleyse
Copy link
Contributor

Are there any changes required for EXT_mesh_features validation?

@javagl
Copy link
Contributor Author

javagl commented Dec 28, 2023

The main aspect of this change is specific for the EXT_structural_metadata property textures. Namely: It checked whether the number of bytes of the data type of the property matched the number of channels.

For the EXT_mesh_features, there is no such constraint. It does only validate the basic structure of the channels (e.g. here). However, there are some open questions that are tracked in the issue at CesiumGS/3d-tiles#748 . For example, the validator currently makes the assumption that the channels have 8 bits, and we still have to sort out how to handle cases where this assumption is wrong...

@lilleyse lilleyse merged commit 05c710b into main Jan 2, 2024
2 checks passed
@lilleyse lilleyse deleted the channel-mismatch-warning branch January 2, 2024 14:00
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