Currently the TextureBlock always multiplies the level directly into the color value.
For normal maps this leads to behaviour that differs from the default materials. This makes changing textures or levels on normal maps from code very problematic.
As discussed here this is not a bug but the intended behaviour of the TextureBlock: Strange behaviours in Node Material normals - #12 by Evgeni_Popov
I’d love to have the possibility to use normal maps the way that I can use them as for example in PBRMaterial where the level changes the normal intensity.
It would be invaluable to have this solved for being able to make alternative, flexible materials based on NME that are usable from code.
I’m not entirely sure how to pull that off without breaking the compatibility to the current implementation though.
After thinking about it a while I came up with a few proposals. Not sure which is best and fits your vision best.
Give the TextureBlock a property “isNormalMap”
have a property “isNormalMap” that influences the calculation of the level
if true calculate the correct intensity of the normal based on the level instead of multiplying it.
convert it back to color (val * 0.5 + 0.5) to keep compatibility with the color input of the PerturbNormalBlock
Have a separate NormalTextureBlock
have a separate NormalTextureBlock that does not multiply the level with the color
have an output on the block for the level
this gives the ability to route the color to the input of a PerturbNormalBlock and the level to the strength input.
also gives possibility to work with the values between the NormalTextureBlock and the PerturbNormalBlock.
Have a NormalTextureBlock that outputs vector
have a separate NormalTextureBlock that converts the pixel value to vector
make it calculate the correct normal based on the level
make the PerturbNormalBlock accept a normal vector instead of color
possibly makes the PerturbNormalBlock strength input obsolete
The second proposal is the only one that would not interfere with the current implementation although I think the third one is the cleanest one.
Proposal one seems to be more overhead in calculations than necessary.
None of them is a perfect fitting solution but rather just ideas of how to make it possibly work.
I would very much appreciate it if you could look into this.
This playground shows the node material and the PBRMaterial side by side. PerturbNormal strength is exposed in the inspector for convenience. https://playground.babylonjs.com/#NCD99X#2
1/ and 3/ can’t work because depending on the case (standard material / pbr material), the normal from the normal map is not modified by the level/strength, it is the normal of the geometry that is modified when constructing the cotangent frame inside the PerturbNormal block.
I would go for a simplified 2/:
don’t create a new texture block
add a new “level” output to the existing texture block
add a switch on the texture block to disable multiplying the texture fetch by the level
For your use case, you would enable this switch and would connect the “level” output to the “strength” input of the PerturbNormal block.
@Evgeni_Popov I’ve tried coming up with a patch for this problem, however I’m stumbling over a shader compilation error if I remove the constant for “Perturb Normal Strength” and attach a variable float into strength input (e.g. r output from the BumpTexture or the proposed level output in the patch). See https://playground.babylonjs.com/#NCD99X#3. Any idea what’s wrong?
Yes, you should use a static value because the shader code is expecting a value that is defined at the global level (like a uniform), it can’t work with a value that is derived from other blocks.
It’s a current limitaion of the normal map code (not specific to the NME).
[…] You can fool the system by adding a block (a VectorSplitter for eg) between the texture and the perturb block and tag this block as being processed in the vertex shader: that way, the output value of this block will be passed as a varying variable to the fragment shader (where perturb normal is evaluated), which is defined at the global level and that will make the whole thing work:
I have added the VectorSplitter block, changed its target to be “Vertex”, linked BumpTexture.rgb to VectorSplitter.xyz and linked VectorSplitter.x to PerturbNormal.strength.
What should I do with the proposed level output? I can’t split with with any other block, can I? I could certainly make level a vector, which could be split, but it feels wrong. I could also tag that output to be processed in the vertex shader, but is that correct way or a hack?
The shader code is not meant to be used with something else than a global variable and I don’t think it’s straightforward to change that… You can try to link the Texture.level output to a VectorSplitter and tag this block as “Vertex” and see if that works. If it does, it would be a workaround that would make the feature usable until we could improve the system by not requiring this VectorSplitter…
add a new “level” output to the existing texture block
add a switch on the texture block to disable multiplying the texture fetch by the level
For your use case, you would enable this switch and would connect the “level” output to the “strength” input of the PerturbNormal block.
—
The problem is that if the input of PerturbNormal is not a global variable (either a uniform or a varying) it does not work, because the existing (bump) shader code expects a global variable. The best thing to do would be to update the code so that it can work but I’m not sure it’s an easy one…
I’ve been debugging a little more and I might have a kind of a workaround/solution hybrid: the problem with the bump fragment shader is that there is a two-parameter perturbNormal function and a three-parameter perturbNormal function. The two-parameter one uses vBumpInfos.y as a fallback for the third parameter, which is string-replaced in PerturbNormalBlock by an expression which might contain an undefined identifier, in our case level. Every instance of the three-parameter perturbNormal can use our level normally, since it’s defined before usage.
I tried grepping through the shader code and have seen only one instance of a two-parameter perturbNormal in bumpFragment.fx. Would anything bad happen if we removed the two-parameter function and by doing so prevented string replacement inside this function?
I could, however I’d like to have a working state, which I currently don’t have (after removing the two-parameter function, the shader suddenly can’t find the three-parameter one, normalW=perturbNormal(TBN,vBumpUV+uvOffset,vBumpInfos.y): 'perturbNormal' : no matching overloaded function found). Should I put my current changes in a PR for initial discussion nevertheless?
BJS - [18:43:24]: Offending line [776] in fragment code:
normalW=perturbNormal(TBN,vBumpUV+uvOffset,vBumpInfos.y);
Error: FRAGMENT SHADER ERROR: 0:776: 'perturbNormal' : no matching overloaded function found
ERROR: 0:776: '=' : dimension mismatch
ERROR: 0:776: 'assign' : cannot convert from 'const mediump float' to 'highp 3-component vector of float'
This looks to me like it either can’t find the three-parameter function (which is rather improbable) or vBumpInfos.y, which would be strange too, since it’s being used couple of lines down the code. Either way, I’d really appreciate your input!
OK, I’ll update the PR in a couple of minutes to my current state. Apparently, I’ve managed to move past both TextureBlock and PerturbNormalBlock, since the error I’m now getting is somewhere in the PbrMetallicRoughnessBlock:
BJS - [20:25:40]: Offending line [765] in fragment code:
vec3 viewDirectionW = normalize(u_cameraPosition - v_output1.xyz);
BJS - [20:25:40]: Error: FRAGMENT SHADER ERROR: 0:765: 'cotangent_frame' : no matching overloaded function found
ERROR: 0:765: '=' : dimension mismatch
ERROR: 0:765: '=' : cannot convert from 'const mediump float' to 'highp 3X3 matrix of float'
I’m still figuring out why the offending line is not something with cotangent_frame and what could be wrong, but I’m still on it, even though I appreciate a glance from an experienced eye
@Deltakosh@Evgeni_Popov I think I’m done I’ve updated the PR with the current state of affairs, the code actually works for me. Have tested the following based on the previously used playground:
level output from TextureBlock into strength of PerturbNormalBlock with level multiplication
level output from TextureBlock into strength of PerturbNormalBlock without level multiplication
Static float into strength of PerturbNormalBlock.
All three worked as expected. Could you please review the PR, merge it if it’s fine with you and make an alpha release so we can start working with this? Thanks.
