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

Flip H and Flip V normalmap problem on sprite with nvidia flicker workaround #40905

Closed
RPicster opened this issue Jul 31, 2020 · 5 comments
Closed

Comments

@RPicster
Copy link
Contributor

Godot version:
v3.2.2.stable.official

OS/device including version:
OpenGL ES 3.0
Renderer: GeForce RTX 2080/PCIe/SSE2
Driver Version: 446.14

Issue description:
When using the option 2d/use_nvidia_rect_flicker_workaround in the "Quality" project settings, the normal maps of a sprite will not render correctly when using flip_h or flip_v - the normal map is flipped with the sprite.
Here is a short gif illustrating the issue

Steps to reproduce:

  • Add a Sprite with a texture and a normal map
  • Add a Light2D
  • Set Flip H or Flip V under Offset in the Sprite options
  • Move the Light

Minimal reproduction project:
nvidia-flicker-normalmap-flip_h-problem.zip

@lawnjelly
Copy link
Member

lawnjelly commented Aug 1, 2020

Related to #39851.

I'll have to see if I can work out why it even works at all with the uniform draw path in GLES3. If you flip the normal map, although the texels get flipped to match the color, the normal directions don't get flipped. So the right side of a sphere when flipped to the left will still point to the right (which is the 'erroneous' behaviour).

It needs an intervention to also flip the normal orientations, which is a more tricky problem:

  • We might have to include extra information somehow to let the shader know to flip normals
  • Flipping normals in the fragment shader could be bad performance wise on mobile

Edit: Yes it looks like the shader is setting local_rot in an attempt to deal with this problem. This is relying on the uniform draw method, I'll see if there's a way around this for non uniform draw.

@lawnjelly lawnjelly self-assigned this Aug 1, 2020
@lawnjelly
Copy link
Member

lawnjelly commented Aug 1, 2020

It turns out this is a pretty pervasive problem. It will affect the nvidia workaround on GLES2 as well, as well as having implications for normal mapping with batching.

The local_rot is dependent on the local transform and extra matrix in the shader, which of course will not be set in batching. In addition, the information as to flipping Horz or Vert is only available in TEXTURE_RECT mode (i.e. uniform draw method).

Given that we are moving over towards batching as primary method and the flipping is incompatible with nvidia_workarounds, I'm now tending to think we should add an extra 2 Vector2s into the vertex format in cases of normal mapping in order to transfer the normal map rotation necessary for lighting.

Actually we may be able to do this in a compact form. Strictly speaking we only need an angle for an UP vector (which can derived by sin and cos) and a polarity for the horizontal flipping (which could be the sign of this angle, except in the case of zero, but that is workaroundable). I'll have to discuss this with @clayjohn.

EDIT - I now have a tentative solution to this adding a new LIGHT_ANGLE vertex attribute, a bit more work before I make a WIP PR. I'll maybe fix this first for GLES3 then GLES2 nvidia workaround and the batching. 👍

@akien-mga
Copy link
Member

This is probably not so relevant anymore now that 2D batching is implemented?

@lawnjelly
Copy link
Member

Yes the normal mapping uses LIGHT_ANGLES to work correctly with batching. The problem probably still exists with the legacy renderer with nvidia workaround, but not many people are using that now I expect.

@akien-mga
Copy link
Member

I think we can close as no longer an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants