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

Normals got flipped #10

Closed
emackey opened this issue May 27, 2017 · 7 comments
Closed

Normals got flipped #10

emackey opened this issue May 27, 2017 · 7 comments

Comments

@emackey
Copy link
Contributor

emackey commented May 27, 2017

Hi guys,

An update to BabylonJS was recently pushed to this repo (in 6a74f09), and the direction of the Y-axis on the normal maps has flipped during that update. If I load the NormalTangentTest object with the latest version of this repo, I get mixed-up normal maps, like this:

babylonjs_bad

Prior to the update to BabylonJS, the same model produces correct normals:

babylonjs_good

As an experiment, I tried loading the so-called "DirectX normals" (as described in KhronosGroup/glTF#952), where the green channel intensity is inverted. These normals do produce the correct output in master on this repo, but, I believe they do not meet the glTF 2.0 spec as it stands.

@emackey
Copy link
Contributor Author

emackey commented May 27, 2017

/cc @sbtron @bghgary

@bghgary
Copy link
Collaborator

bghgary commented May 31, 2017

@emackey Thanks for catching this! I was making an optimization that inadvertently broke the generated tangents such that the handedness is wrong. I didn't notice because most of the sample models have tangents specified. I will make a fix.

@emackey
Copy link
Contributor Author

emackey commented May 31, 2017

@bghgary Great! Looking forward to merging a fix into my gltf-vscode extension.

On a related note, ThreeJS still isn't processing these normals correctly either. That bug is reported separately in donmccurdy/three-gltf-viewer#10.

@bghgary
Copy link
Collaborator

bghgary commented Jun 2, 2017

This should be fixed with f8f5db0

The PR for BabylonJS is here: BabylonJS/Babylon.js#2222

@emackey
Copy link
Contributor Author

emackey commented Jun 2, 2017

Well... I merged this into my fork, but it doesn't look like anything changed. Maybe the fix didn't get deployed to a release branch or something yet?

https://emackey.github.io/BabylonJS-glTFLoader/?model=NormalTangentTest

@bghgary
Copy link
Collaborator

bghgary commented Jun 2, 2017

Oops, sorry about that. I pushed the wrong version. Fixed now.

@emackey
Copy link
Contributor Author

emackey commented Jun 3, 2017

Awesome! Fix confirmed.

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

2 participants