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

Correct provoking vertex for lighting when flat shading #11577

Merged
merged 3 commits into from
Nov 23, 2018

Conversation

unknownbrackets
Copy link
Collaborator

This fixes the Blade Dancer lighting problem in #4140.

I'm not sure when this was fixed for OpenGL, but it turns out that it was already fixed there. This fixes separate bugs in software and hardware rendering that had the same basic results but different causes (realizing that was key to fixing...)

  • Software would clip using interpolation, always, screwing up flat shading.
  • Software handled culling by reversing verts, which caused the provoking vertex to incorrectly swap.
  • Hardware used the wrong normal to calculate lighting, ultimately causing a similar bug when using all the same vertex color.

This doesn't solve two areas I have not yet tested, although they're edge cases:

  • It's not clear how skinning/weights interact with flat shading, it's possibly also wrong. May be wrong already.
  • I'm not sure which normal should be used for texture matrix mapping.

-[Unknown]

It needs to use the provoking color, regardless of culling or clipping.

Fixes Blade Dancer lighting (see hrydgard#4140.)
This might interact strangely with skinning of texture matrix mapping, but
they're not likely to be used together.
@unknownbrackets unknownbrackets added this to the v1.8.0 milestone Nov 23, 2018
@@ -354,6 +351,8 @@ void SoftwareTransform(

case GE_TEXMAP_TEXTURE_MATRIX:
{
// TODO: What's the correct behavior with flat shading? Provoked normal or real normal?
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess would be real normal, flat shading is not really related to the texture pipe at all I think..

@hrydgard
Copy link
Owner

In the software renderer, I wonder if it wouldn't be more elegant to add a flag to reverse the backface culling, instead of keep the reordering of the verts and adding that argument. But I'm fine with it as is.

@hrydgard hrydgard merged commit c1e1076 into hrydgard:master Nov 23, 2018
@unknownbrackets unknownbrackets deleted the lighting branch November 23, 2018 14:04
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.

2 participants