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 treats +X as glTF forward vector, 2.0 spec says it's +Z. #6591

Closed
emackey opened this issue May 15, 2018 · 11 comments
Closed

Cesium treats +X as glTF forward vector, 2.0 spec says it's +Z. #6591

emackey opened this issue May 15, 2018 · 11 comments

Comments

@emackey
Copy link
Contributor

emackey commented May 15, 2018

Not sure how I didn't notice this before, but VelocityOrientationProperty apparently places the +X vector as the forward vector, while glTF 2.0 settled some time ago on the +Z vector being forward.

This means that all glTF 2.0 models used with VelocityOrientationProperty will fly or drive sideways.

@emackey
Copy link
Contributor Author

emackey commented May 15, 2018

Also this repo contains an out-dated GroundVehiclePBR, that I am in the process of updating, that was using the old +X convention.

@emackey
Copy link
Contributor Author

emackey commented May 15, 2018

New vehicle added in #6592. Note that if you use the corrected model with VelocityOrientationProperty, you will now see truck driving sideways.

@emackey
Copy link
Contributor Author

emackey commented May 22, 2018

Expanding the scope: It's not just VelocityOrientationProperty, it's everything that deals with orientation. For example, headings are all 90 degrees off for glTF 2.0 files.

One option here could be to add a frontAxis setting similar to the existing upAxis setting. The default would have to auto-detect glTF 1.0 (+X front) vs glTF 2.0 (+Z front) and assign itself accordingly.

@emackey emackey changed the title VelocityOrientationProperty doesn't agree with glTF 2.0 Cesium treats +X as glTF forward vector, 2.0 spec says it's +Z. May 22, 2018
@mramato
Copy link
Contributor

mramato commented May 22, 2018

As you already noted, the scope here goes far beyond models. I think this is a massive breaking change for Cesium if we were to make it. I'm not sure changing all of Cesium because of glTF 2.0 is the way to go. It might be better for the Model primitive to have special logic for handling gltf models so that glTF "just works" with Cesium's existing convention.

@emackey
Copy link
Contributor Author

emackey commented May 22, 2018

It might be better for the Model primitive to have special logic

Yes, and I suspect the existing upAxis setting is an example of such logic. Perhaps all that's needed is a new forwardAxis setting in Model.js. Would need to be careful that this didn't mess up 3D tiles somehow.

@hpinkos
Copy link
Contributor

hpinkos commented May 22, 2018

Is that all related to this? #3256

The East/North/Up transform we default to everywhere makes heading: 0 east instead of north.

@emackey
Copy link
Contributor Author

emackey commented May 22, 2018

@hpinkos I was noticing that in my testing of headings just now. heading: 0 appears to place glTF's +X axis to the East, and +Z to the South. glTF 2.0 models drive South with heading 0.

@hpinkos
Copy link
Contributor

hpinkos commented May 22, 2018

@emackey yeah, I've been trying to argue for years that heading: 0 should be north because that seems to be a more common standard. But I lost that argument and we decided just to document it and add the fixed frame generator to Transforms.

@emackey
Copy link
Contributor Author

emackey commented May 22, 2018

@hpinkos OK. I think that's an unrelated issue. The proposed fix here would just make glTF 1.0 and 2.0 agree on East.

@hpinkos
Copy link
Contributor

hpinkos commented May 22, 2018

Gotcha 👍

@lilleyse
Copy link
Contributor

Fixed in #6632

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

No branches or pull requests

4 participants