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

Fix normal map rendering issue on Adreno GPUs #15850 #17158

Merged
merged 1 commit into from
Aug 8, 2019

Conversation

JordyvanDortmont
Copy link
Contributor

This fixes #15850 by checking if the material is double sided, if so we use an alternative for computing the possible normal inversion with gl_FrontFacing.

The same issue was identified in #10331 and @bhouston also commented the fix I implemented on the blog post that was already being referenced in the shader code.

@mrdoob mrdoob added this to the r108 milestone Aug 8, 2019
@mrdoob mrdoob merged commit eaa4c2b into mrdoob:dev Aug 8, 2019
@mrdoob
Copy link
Owner

mrdoob commented Aug 8, 2019

Thanks!

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 23, 2019

@JordyvanDortmont Please notice that this change was reverted via #17958.

@JordyvanDortmont
Copy link
Contributor Author

@Mugen87 Thanks for notifying me. We'll fork and patch the shader.

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 23, 2019

We'll fork and patch the shader.

It should be no problem to replace the shader chunk with Material.onBeforeCompile() if necessary. Also notice the discussion from here #17804 (comment). Using BufferGeometryUtils.computeTangents() and setting Material.vertexTangents to true actually solved the rendering glitch with the respective wheels model.

@JordyvanDortmont
Copy link
Contributor Author

It should be no problem to replace the shader chunk with Material.onBeforeCompile() if necessary.

Yes, I intended to patch it this way as well, instead of forking.

Also notice the discussion from here #17804 (comment). Using BufferGeometryUtils.computeTangents() and setting Material.vertexTangents to true actually solved the rendering glitch with the respective wheels model.

So just to be sure, if BufferGeometryUtils.computeTangents() is called on the geometry with the material that's double-sided and set Material.vertexTangents to true, rendering on an Adreno GPU should work?

For both cases we can check if there's an Adreno GPU and apply a fix.

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 24, 2019

So just to be sure, if BufferGeometryUtils.computeTangents() is called on the geometry with the material that's double-sided and set Material.vertexTangents to true, rendering on an Adreno GPU should work?

Yes. However, BufferGeometryUtils.computeTangents() currently requires an indexed-geometry but this can be fixed.

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 24, 2019

@WestLangley What do you think about generating an index in BufferGeometryUtils.computeTangents() if not present, use it for the tangent computation and then remove it at the end of the method?

@WestLangley
Copy link
Collaborator

@Mugen87 If it doesn't require and index to produce reasonable results -- and I do not think it does -- then I'd just operate on the triangle soup directly.

I'll file a PR and add VertexTangentsHelper.

@mrdoob
Copy link
Owner

mrdoob commented Feb 4, 2021

In case anyone lands here, the Adreno bug is only when gl_FrontFacing is inside a function: #21205

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.

Rendering issue with normals and DoubleSided materials on some Adreno GPU series
4 participants