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

Handle the situation where the recorded component type does not match the required type for the actual number of stored points #447

Merged
merged 1 commit into from
Sep 2, 2023

Conversation

nyalldawson
Copy link
Contributor

This situation arises when decoding certain malformed files, most notably it's seen in glb tiles from Google Earth's 3d tileset.

It's a port of the workaround used by Cesium native here:

https://github.com/CesiumGS/cesium-native/blob/d9172461e2d53f19d050ebc39f24a4f4ef5b9224/CesiumGltfReader/src/decodeDraco.cpp#L101

@syoyo
Copy link
Owner

syoyo commented Aug 24, 2023

I think it'd be better to provide user callback functions for Draco decoding/encoding for flexibility, not implement workaround in the builtin Draco decoding function.

PR of callback functions for Draco decoding/encoding is much appreciated!

@nyalldawson
Copy link
Contributor Author

@syoyo would it be acceptable to you to follow the current approach but only use this logic when the permissive flag from #445 is set?

@syoyo
Copy link
Owner

syoyo commented Sep 1, 2023

@nyalldawson Yes, could you please modify this PR so that the situation is only considered when strictness flags is PERMISSIVE, like this?:

if (strictness == PERMISSIVE) {
  do PR 447
}

recorded component type does not match the required type
for the actual number of stored points

This situation arises when decoding certain malformed files, most
notably it's seen in glb tiles from Google Earth's 3d tileset.

It's a port of the workaround used by Cesium native here:

https://github.com/CesiumGS/cesium-native/blob/d9172461e2d53f19d050ebc39f24a4f4ef5b9224/CesiumGltfReader/src/decodeDraco.cpp#L101
@nyalldawson
Copy link
Contributor Author

@syoyo ok, that's done. I've also made it add a warning string when the component type is upgraded when in permissive mode.

@syoyo syoyo merged commit 51530ee into syoyo:release Sep 2, 2023
6 checks passed
@syoyo
Copy link
Owner

syoyo commented Sep 2, 2023

Awesome! Thanks! Merged!

TODO: This is still much appreciated 🥺 > PR of callback functions for Draco decoding/encoding is much appreciated!

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

Successfully merging this pull request may close these issues.

2 participants