Add support for Cesium RTC #1114
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tension is used by OpenDroneMap.
Merged
|
Just to add flavor: we think this is a feature of general use to draco users outside OpenDroneMap, and prefer to upstream features when we believe this to be the case. Thanks so much for opening this pull request @NathanMOlson. |
|
@gemini-cli /review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Review Summary
This pull request adds support for the CESIUM_RTC glTF extension to both the encoder and decoder. The changes are well-structured and include corresponding tests to ensure correctness.
🔍 General Feedback
- The implementation correctly handles the
CESIUM_RTCextension, reading and writing thecenterproperty. - New unit tests have been added to cover both the decoding and encoding paths, which is great.
- I've provided a few suggestions to enhance performance and code clarity. Specifically, I've pointed out an opportunity to optimize the extension checking logic in the decoder and to avoid an unnecessary copy by returning a
constreference in theSceneclass.
Overall, this is a solid contribution that improves Draco's support for glTF extensions.
Merged
|
@FrankGalligan Any thoughts on this PR? Your bot likes it. 😊 |
|
@FrankGalligan Any thoughts on this PR? |
ondys
approved these changes
Sep 4, 2025
|
looks good. Thanks for the PR |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add support for Cesium RTC extension in GLTF encoder/decoder. This extension is used by OpenDroneMap.