As you can see, the reflections are all messed up. Changing the invert options doesn’t seem to do anything.
The issue doesn’t happen when using NodeMaterial where tangent isn’t connected, to enforce auto generated tangents even when the mesh has them: https://playground.babylonjs.com/#VL1FPT#1
Issue has been found and not related to the texture flag, I will try to push the fix tomorrow which will probably be a new input on the perturbNormal block as well as a new block to generate the correct TBN matrix.
Nice. Hopefully the new system still seamlessly triggers the auto generated normals if the attribute is missing. Can’t have different node trees for different mesh attribute combinations.
Looks like there is at least one more related issue in the latest babylonjs 5.5.5 version:
MorphTargetsBlock tangent output is Vector3 so it isn’t possible to connect it to the new tangent input in TBNBlock, and probably wouldn’t work correctly even if I could connect it. :s
EDIT: And looks like if I connect the tangents to the input directly just to temporarily bypass the morph target block, I seem to get a shader error again. Not sure what causes it: https://playground.babylonjs.com/#VL1FPT#3
I now confirmed with a local version of MorphTargetsBlock that it starts working after changing tangentOutput to Vec4 and updating the related generated shader lines:
However, even after that there seems to also be yet another an issue that causes backface reflections to render incorrectly, that didn’t happen previously. I’ll try to setup a playground example for that.
This seems to happen in both left handed and right handed coordinate modes.
EDIT: Actually. I just realized that the TBN normal input was connected to the non flipped normal in the above example, so that likely is the problem. Unfortunately connecting a flipped normal doesn’t work either: https://playground.babylonjs.com/#VL1FPT#6
You get FrontFacingBlock must only be used in a fragment shader error and I don’t see a way to get rid of it.
I am looking into it ASAP. The last one is not supported in PBR either. I will at least fix the morph tangents and try to think about a trick for the last one.
Not sure what you mean about the last one not supported in PBR? The model does look fine even on back side if you throw the NormalTangentMirrorTest.glb into the sandbox.
I was actually just now playing around trying to figure out the last issue. I locally was able to make the TBNBlock work also in fragment only mode by:
UPDATED WITH SOME TWEAKS:
Changing target getter setter to:
public get target() {
return this._target;
}
public set target(value: NodeMaterialBlockTargets) {
if (
value === NodeMaterialBlockTargets.VertexAndFragment ||
value === NodeMaterialBlockTargets.Fragment
) {
this._target = value;
}
}
So that Fragment target is also allowed.
Adding TBN as excluded variable name in initialize():
state._excludeVariableName('TBN');
And also changing _buildBlock() code to:
const normal = this.normal;
const tangent = this.tangent;
const world = this.world;
const TBN = this.TBN;
const tbnTarget =
this.target === NodeMaterialBlockTargets.Fragment
? NodeMaterialBlockTargets.Fragment
: NodeMaterialBlockTargets.Vertex;
// Vertex
if (state.target === NodeMaterialBlockTargets.Vertex) {
state._emitVaryingFromString(TBN.associatedVariableName, 'mat3');
}
if (state.target === tbnTarget) {
// Declare output type for fragment only block as no varying is used in that case
const tbnType =
tbnTarget === NodeMaterialBlockTargets.Fragment ? 'mat3 ' : '';
state.compilationString += `
// ${this.name}
vec3 tbnNormal = normalize(${normal.associatedVariableName}).xyz;
vec3 tbnTangent = normalize(${tangent.associatedVariableName}.xyz);
vec3 tbnBitangent = cross(tbnNormal, tbnTangent) * ${tangent.associatedVariableName}.w;
${tbnType}${TBN.associatedVariableName} = mat3(${world.associatedVariableName}) * mat3(tbnTangent, tbnBitangent, tbnNormal);
`;
}
// Fragment
if (state.target === NodeMaterialBlockTargets.Fragment) {
state.sharedData.blocksWithDefines.push(this);
}
With that in place I was able to pass a FrontFacingBlock determined normal to it and the backside is no longer black and front side was correct too.
However the reflections on the back were still messed up like in the original issue.
I also tried to flip the tangent w component for backfaces to see what would happen then. That caused backside reflections to appear otherwise normal, except they were right side up, even though they should be upside down on the concave backside. So I guess flipping the w coponent isn’t the solution there.
Yup double sided lighting is not supported with tangents as we can not interpolate the tangents and their orientation easily so they have to be coming from the vertex shader.
Unfortunately double sided requires as well the face orientation to flip the space for the triangle winding order which can only be done in fragment. in your case it might be easier to try and flip the TBN alltogether ?
Actually just flipping the whole tangent, not just the w component seemed to work. Now both front and backside look correct.
Although this still requires that NodeMaterialBlockTargets.Fragment compatible TBNBlock hack from above, as flipping either normal or tangent needs to indeed happen in the fragment shader.
While waiting for the MorphTargetBlock fix to go live, I did some additional chages to my attempt to make the TBNBlock fragment target compatible so that it can be used with flipped normals and tangents.
I updated my earlier comment above with code that should work for both VertexAndFragment and Fragment type TBNBlocks.
I’m not sure if that is the best way to do it, as now it doesn’t appear to be possible to change the TBNBlock type in NME, only in javascript. But maybe it is possible to automatically change the target to fragment if either tangent or normal inputs come from a Fragment block? Or maybe just make it Fragment block by default (since the output can only be used in fragment shader anyway), with optional VertexAndFragment mode if someone really wants to optimze handful of instructions to vertex shader.
Regardless… Hopefully someone can take a look at implementing something like my code to the official Babylon.js. Because while a custom TBNBlock works, it is going to require me to manually backport all future changes to it whenever Babylonjs version is upgraded.
