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

GLTFExporter: fix znear and zfar range for cameras #13396

Merged
merged 1 commit into from
Feb 22, 2018

Conversation

@mrdoob mrdoob added this to the r91 milestone Feb 22, 2018
@mrdoob mrdoob merged commit d8c7b76 into mrdoob:dev Feb 22, 2018
@mrdoob
Copy link
Owner

mrdoob commented Feb 22, 2018

Thanks!

@takahirox
Copy link
Collaborator

takahirox commented Feb 22, 2018

In Three.js, near and far should be

PerspectiveCamera: Infinity > far > near > 0
OrthographicCamera Camera: Infinity > far > near >= 0

https://threejs.org/docs/#api/cameras/OrthographicCamera.near
https://threejs.org/docs/#api/cameras/OrthographicCamera.far
https://threejs.org/docs/#api/cameras/PerspectiveCamera.near
https://threejs.org/docs/#api/cameras/PerspectiveCamera.far

So ideally we don't need to check in exporter. I think this check should be done in loader, rather than in exporter.

@fernandojsg How did you make/load the model which didn't pass the validation? I suppose there's another root issue.

@takahirox
Copy link
Collaborator

/ping @fernandojsg

@fernandojsg
Copy link
Collaborator Author

@takahirox even if the valid ranges are as you stated, three.js doesn't have any type of validation when you set these values so it's perfectly fine to add a <0 near plane and things will works as expected. In fact if you look at the examples where the ortho camera is being used, you could find many examples where it has a negative value.
So I'd just keep it to ensure we export valid things even if the original code could be potencially wrong.

@takahirox
Copy link
Collaborator

takahirox commented Feb 24, 2018

I discussed with @mrdoob on Twitter.

https://twitter.com/superhoge/status/966966096141193216

We concluded it's worth to ensure the valid values in GLTFExporter although we don't do neither parameter validation nor type check in general. Because

  • exporter isn't hot code (code that runs every frame), so no performance impact
  • hard to fix the invalid gltf files that get spread around
  • for Facebook 3D Posts (This has updated our mind!)

I wanna add note about it in GLTFExporter.

From these reasons, feeling like we should more aggressively ensure the valid values in the exporter.

@takahirox
Copy link
Collaborator

takahirox commented Feb 24, 2018

BTW

In fact if you look at the examples where the ortho camera is being used, you could find many examples where it has a negative value.

I think we should fix the examples (or update the documents).

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.

3 participants