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

Cesium z-up extension #1338

Closed
wants to merge 4 commits into from
Closed

Conversation

ggetz
Copy link
Contributor

@ggetz ggetz commented May 9, 2018

Fixes #1324

@lilleyse Please let me know if you have any initial feedback.

@lexaknyazev
Copy link
Member

Couple optional comments.

  • This seems to be the first extension without an extension object. Maybe, it's worth to mention in the readme that it doesn't exist just in case.

  • an implementation must treat the mesh geometry as z-up

    I assume that everything else (node and IBM matrices) should be treated as usual, right? Maybe, the readme could also emphasize that z-up applies to all mesh geometry.

@emackey
Copy link
Member

emackey commented May 9, 2018

I have to ask, is an extension really needed here? Could one just place a parent node with a 90-deg rotation at the top of the node tree, rather than applying this extension? Are there performance implications for adding such a parent node?

What's really the difference between a model that specifies this extension, vs a model that has a root node with a rotation added to it?

@lilleyse
Copy link
Contributor

lilleyse commented May 9, 2018

Conceptually there isn't any difference, but the purpose of this PR is to avoid that extra node.

For someone who has WGS84 referenced data in their glTF it is a lot easier to tell them to include this extension than describe the changes they need to make to the node hierarchy.

It also allows glTF and 3D Tiles to share the same coordinate system. Although this may not be important broadly, it is really convenient in 3D Tiles pipelines.

@emackey
Copy link
Member

emackey commented May 10, 2018

/cc CesiumGS/3d-tiles#307

@ggetz
Copy link
Contributor Author

ggetz commented May 11, 2018

Closing in favor of solution in CesiumGS/3d-tiles#307

@ggetz ggetz closed this May 11, 2018
@ggetz ggetz deleted the cesium-z-up-ext branch May 11, 2018 15:07
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.

4 participants