Parallax mapping - fundamental bug?

@survivor said: @nehon: There are only two solutions:
  1. Change handedness in TBG as it was in RC2.
  • no shader has to change
  • models with saved tangents stay compatible
  • you have to dig into TBG, which sounds like pain for you :slight_smile:
  1. Change handedness in shaders
    pro/con: inverse of 1.

I’d change it, so that models are compatible to ogre or other formats. Don’t know if there is a quasi standard how the tangent buffer should look like.


1 is reverting back to more issues with normal mapping. The TBG is some kind of math hell (or paradise depending of your freakishness level) and last time I kind of managed my way out of it without my eyes bleeding and a fixed issue…I’d rather not revert this change :stuck_out_tongue:
I basically translated from C++ changes made in Ogre’s TBG to handle mirrored uvs and distorted tangents. So the base was solid.

2 seems like the way to go … We have a normal.y = -normal.y line commented in and out since a long time fluctuating with the changes to the TBG. Iast time we removed it because that’s one more operation per pixel that can be avoided by reverting the green channel in the normal map…but…if you have a large number of asset…going through all normal maps and inverting the green channel can be very painful…
So I guess the -normal.y thing is the safest route…
I’d like to hear what @Momoko_Fan, and @pspeed think of this.

So this has nothing to do with parallax mapping and only to do with normal mapping?

I liked the change that fixed the flip problem. It seemed weird to have to do an extra step to every normal map… so I didn’t mind regenerating a few of them.

@survivor said: Ok, the problem is the "handedness" of the tangent which should normally come in "inTangent.w".

Changing line 174 of “Lighting.vert” from
[java]
mat3 tbnMat = mat3(wvTangent, wvBinormal * -inTangent.w,wvNormal);
[/java]
to
[java]
mat3 tbnMat = mat3(wvTangent, wvBinormal * inTangent.w, wvNormal);
[/java]
fixes the problem.

The question @devs, @nehon is: Did the handedness change accidentally or by purpose? (TangentBinormalGenerator.java)

Actually, if this is the patch we say is working then this seems like the right one as the -inTangent.w thing always seemed like a hack to me. Like, “Well, TBG seems to be giving us the opposite of what we need so we’ll invert it.”

…I think I removed that minus from my own shaders a long time ago.

@pspeed said: Actually, if this is the patch we say is working then this seems like the right one as the -inTangent.w thing always seemed like a hack to me. Like, "Well, TBG seems to be giving us the opposite of what we need so we'll invert it."

…I think I removed that minus from my own shaders a long time ago.


Yeah that too :p.
The real fix is to apply survivor change AND to invert the normal y value. So no operation will be needed on normal maps, but…we’ll have that additional operation once per fragment. That’s no big deal IMO…but still.

@nehon said: Yeah that too :p. The real fix is to apply survivor change AND to invert the normal y value. So no operation will be needed on normal maps, but....we'll have that additional operation once per fragment. That's no big deal IMO...but still.

Compared to all of the other stuff… I doubt that little multiply will make any difference.

Yeah, also I found a way to make it free
replacing

vec3 normal = normalize((normalHeight.xyz * vec3(2.0) - vec3(1.0)));

with

vec3 normal = normalize((normalHeight.xyz * vec3(2.0,-2.0,2.0) - vec3(1.0,-1.0,1.0)));

I’ll just drop a big comment so that we don’t go “WTF is that??” in one year :stuck_out_tongue:

3 Likes

Ok I committed the fix.
@Perjin and @survivor thanks a lot for your help on this issue.

4 Likes

N1ce! I’ll adjust my stuff accordingly.

@nehon said: Ok I committed the fix. @Perjin and @survivor thanks a lot for your help on this issue.

Does my ugly testModel look ok with this fix? I mean Seamms on mirrored UVs. Or i need to test it?

@mifth said: Does my ugly testModel look ok with this fix? I mean Seamms on mirrored UVs. Or i need to test it?
yes it does, as scary as usual, but no seam ;)
@nehon said: yes it does, as scary as usual, but no seam ;)

Cool! :slight_smile: