-
Notifications
You must be signed in to change notification settings - Fork 231
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
Re-normalizes interpolated normals and deals with non-uniform scale #65
Conversation
…d meshes, both to get correct shading
…ents (slightly smoothed in Maya to reduce vertex splits), to get correct normal mapping and reflections
@@ -1,204 +1,235 @@ | |||
{ | |||
"accessors" : [ | |||
"asset": { | |||
"generator": "Maya2glTF", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DamagedHelmet model here comes from the sample model repository, and prior to that it was downloaded from SketchFab back before glTF 2.0 was even published. It does need fixing, and possibly it should be simply re-exported to glTF using SketchFab's current glTF exporter.
But in any case, I don't want this copy of DamagedHelmet to be different from the one on glTF-Sample-Models, regardless of where we get the fixes from.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, it seems I forgot to make a patch-request branch. This new helmet export was never intended to be part of this patch, mea culpa
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see, this is off your master
branch. You may want to close this whole PR and open a new one from a patch branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do so now.
I will created a dedicated PR branch for this issue. |
Fix sync issue inside each individual animation (GSVN-157)
When porting your shader to our Maya2glTF project, we noticed two problems that this pull request should address:
when no normal map is present, the shading is wrong. This is best noticed when choosing the
InterpolatedNormalsTest
from the models menu, and disabling IBL. Without this PR, when rotating the camera, you'll notice the specular highlight to unevenly brighten the edges of the triangles, as if the model is more or less flat shaded. The reason was that the normals are linearly interpolated by the GPU, and hence do not have unit length anymore. So the pixel-shader must re-normalize the normals. This was only correctly done when a normal map was present.when non-uniform scaling was applied, the normals were not computed correctly. Again this can be easily seen by choosing the
NonUniformScalingTest
from the models menu, and disabling IBL. Without this PR you'll notice a very weird reflection. I tried to document the math behind this, because this is so tricky and weird, I hope the comments can be of some educational value.Most likely I made some mistakes, I'll be happy to address them.
Thanks for any feedback,
Peter Verswyvelen