playcanvas / engine

JavaScript game engine built on WebGL, WebGPU, WebXR and glTF
https://playcanvas.com
MIT License
9.69k stars 1.36k forks source link

Artifacts on model when normal map is applied #2787

Open yaustar opened 3 years ago

yaustar commented 3 years ago

Thread: https://forum.playcanvas.com/t/weird-shading-lines-when-using-normal-maps/17207/

Repro: https://playcanvas.com/editor/scene/1078580

image

It should look more like this: image

OP mentions that lower image was taken when they use glb and export the mesh’s normals AND tangents (a setting which seems to have no effect in fbx files).

Importing the FBX as JSON and GLB into the Editor makes no difference.

Is this a problem with FBX or our rendering?

raytranuk commented 3 years ago

It is probably related to having per vertex tangents - most models now use partial derivatives to calculate tangent space from interpolated vertex normals (smaller mesh data size) - I will take a look and update this issue with my findings.

raytranuk commented 3 years ago

I investigated this and it seems vertex tangents are not coming through if the mesh is imported as fbx - so at runtime the hasTangents flag is not set so the path is taken in the shader is the non vertex tangent path - which is incorrect for the generated normals and normal map. The problem probably lies in the fbx importer - which I am not familiar with and so probably this bug should be assigned to someone who is - and looked into when that person has bandwidth - assigned back to @yaustar for now.

mvaligursky commented 3 years ago

Could perhaps the source mesh / normal map be authored / exported the way it's compatible with the rendering without mesh tangent space as a different possible solution?

This could be relevant https://bgolus.medium.com/generating-perfect-normal-maps-for-unity-f929e673fc57

yaustar commented 3 years ago

Another project with a similar issue: https://playcanvas.com/project/771818/overview/normal-mapping-test

potentially-null commented 3 years ago

Is there any progress on this?

We'd like to use PlayCanvas for product visualisation, but in its current state without normal mapping we are hitting a brick wall. As a workaround, we tried importing glb assets with tangents into the editor and loading them at runtime, which correctly imports the tangents and causes the expected shader variant to be used. It still does not look right with any maps we've used, though, which were exported from Substance. The normal maps and models we are using do work flawlessly in Unity.

As far as I understand, there are currently at least two issues: Firstly, as @raytranuk pointed out, tangents are dropped by the FBX importer, which causes an entirely different normal mapping algorithm to be used.

Secondly, most current applications use MikktSpace tangent frames. In Unity, the tangent frame is assembled in the fragment shader without re-normalisation as described here. inputData.normalWS = TransformTangentToWorld(normalTS, half3x3(input.tangentWS.xyz, bitangent.xyz, input.normalWS.xyz)) In contrast, PlayCanvas does this by default: dTBN = mat3(normalize(dTangentW), normalize(dBinormalW), normalize(dVertexNormalW)) There is also a fastTBN implementation, which skips re-normalisation in the fragment shader, but this option is disabled by default and is not exposed via the editor. In other words, by default PlayCanvas uses a method, which does not correctly work with normal maps created with apps such as Substance and xNormal, which implement the de-facto standard MikkT algorithm.

We'd be more than happy if somebody could have a look at this issue!

yaustar commented 3 years ago

Hi @potentially-null ,

Sorry for the lack of updates on this. We've just had a chat internally about this and basically come down to the following:

The bit I'm a bit surprised about is not getting any other reports from other Substance Painter users with PlayCanvas so I'm wondering if something is being missed or are they also going down the route of exporting to GLBs directly.

potentially-null commented 3 years ago

Hi @yaustar ,

thanks for looking into this.

I agree, I don't understand why there aren't more reports either. Possibly, most users avoid normal mapping to reduce download size?

Our workflow is as follows: Author 3d model in Blender, triangulate, smooth model, export FBX/GLB with tangents. Import FBX to Substance Designer/Painter, select tangent basis + OpenGL orientation, setup baker with high poly model, bake. I double checked every step but couldn't find any issues with it.

I've attached some more assets for testing, some screenshots of the results I got and the expected result I get in Unity. I hope this helps in resolving this issue.

assets.zip screenshots.zip

mvaligursky commented 3 years ago

@potentially-null - I wonder if you've testing this option mentioned in the documentation linked above: Compute tangent space per fragment: Enabled | Compatible with Unreal Engine 4, Blender and Unity HDRP workflow.

We compute the tangent space in the fragment shader, and so it's possible this would be more compatible.

potentially-null commented 3 years ago

Hi @mvaligursky ,

yes I have tested that option as well. The resulting normal map is in assets.zip. It‘s the one with _frag in its filename. I’ve also included a screenshot of the render in screenshots.zip. I have tested both the option you mention and the mikktvert option and both of them caused similar, slightly different unexpected artefacts when using them in PlayCanvas.

yaustar commented 3 years ago

@potentially-null I'm going to reach out to a few of our users that use a similar worklow to see if we can get more information

Edit: Looks like one of the artists uses Marmoset which apparently doesn't have this problem. The other developer uses the following workflow: https://magazine.substance3d.com/high-end-renders-online-bike-configurator-substance/

mvaligursky commented 3 years ago

Hi @potentially-null - you mention there's the different in how we handle tangent space in the engine as well .. I wonder if you could use the glb files with tangent space, but also replace this chunk to remove the normalization: https://github.com/playcanvas/engine/blob/master/src/graphics/program-lib/chunks/TBN.frag

you could either fix the code in your local engine and use local engine to test this .. or you can replace the chunk at runtime.

I'm curious if this would allow you to render your meshes correctly until we can find a better solution. If this works, we could probably easily add this chunk as an optional to the engine.

Maksims commented 3 years ago

@guycalledfrank mentioned a long time ago that dropping tangent space from the vertex data and generating it automatically, can lead to such weird cases, where advanced tools require specific tangent space.

potentially-null commented 3 years ago

@yaustar Do you know when they last tried the workflow they are describing? The article is from 2019. Maybe you can ask them to send you a model + normal map they have successfully used in the engine, so you can double check if it still works. You'd have to load it at runtime as glb, though, as the FBX importer removes the tangent data from the asset.

Loading the assets I've previously shared in the babylon sandbox earlier, I got the expected result similar to when I tested it in Unity:

babylonSandbox

@mvaligursky Yeah, there's a minor difference compared to the mikkt algorithm. There's actually two flavours: Either you calculate the bitangent in the vertex shader and pass it to the fragment shader via an interpolator along with normal and tangent vectors, or you just pass normal and tangent vectors and calculate the bitangent in the fragment shader, thereby saving an interpolator but computing the cross product more often. In both cases, normal and tangent vectors are normalised in the vertex shader, but not re-normalized after interpolation in the fragment shader. This is what you guys do in the fastTBN chunk, which is not used by default. This is what I was talking about. I took the information from here .

Pixel Shader Transformation In regards to the interpolated tangent space the baker in both Blender and the xnormal plugin will keep the vertex normal and the vertex tangent normalized in the vertex shader. However, in the pixel shader the "unnormalized" and interpolated vertex normal and tangent are used to decode the tangent space normal. The bitangent is constructed here to avoid the use of an additional interpolater and again is NOT normalized. // vNt is the tangent space normal vB = sign cross(vN, vT); vNout = normalize( vNt.x vT + vNt.y vB + vNt.z vN ); The key to get flawless results is that the baker is designed to do the EXACT inverse of this very transformation allowing the pixel shader to remain fast and simple.

He is describing the MikkTFrag algorithm. In the engine, the bitangent is not computed in the fragment but in the vertex shader.

Removing normalization doesn't fix the problem with reflective materials, unfortunately:

playcanvas_normal_mapped_reflect

However, it seems to look correct without reflections:

playcanvas_normal_mapped_no_reflect

@Maksims If there are vertex tangents present in the FBX, which have been used to generate a given normal map, and the editor strips them and instead computes new ones using another algorithm - in this case on the fly in the fragment shader - , this will certainly cause artifacts in places where the computed tangent differs from the one used when baking the normal map.

As to the workaround: Loading the GLBs at runtime in initialize() causes them to pop up after the loading screen has finished. Any idea how we can prevent that from happening?

guycalledfrank commented 3 years ago

Current derivative-based non-precomputed tangent space is pretty random and definitely doesn't have to match mikktspace. As far as I remember, Will decided to keep it as default because it saves memory/startup time and works OK on many models.

Engine's vertex tangent computation is based on Lengyel's method: http://web.archive.org/web/20180620024439/http://www.terathon.com/code/tangent.html

... which is also not mikktspace.

Ideally there should be a switch somewhere:

yaustar commented 3 years ago

@yaustar Do you know when they last tried the workflow they are describing? The article is from 2019. Maybe you can ask them to send you a model + normal map they have successfully used in the engine, so you can double check if it still works. You'd have to load it at runtime as glb, though, as the FBX importer removes the tangent data from the asset.

I've pinged them about this issue to see if they can help. Hoping they can provide some more insight :)

yaustar commented 3 years ago

@potentially-null

As to the workaround: Loading the GLBs at runtime in initialize() causes them to pop up after the loading screen has finished. Any idea how we can prevent that from happening?

A way I can think of doing this is to have a custom pre-loading script (https://developer.playcanvas.com/en/user-manual/designer/loading-screen/) where the instead of hiding the splash when the loading is done, it loads all the GLB containers. Once that is done, then hide the splash screen.

Note that the app is still being updated and rendered behind the splash screen though. It is possible to patch the engine (and this is where we starting to get a bit hacky) so that you can delay the call to application.start() to prevent this from happening.

If you would like an example, post in the forums and I whip one up as soon as I can :) (I've done something similar before with another client so shouldn't take too long)

potentially-null commented 3 years ago

As to the workaround: Loading the GLBs at runtime in initialize() causes them to pop up after the loading screen has finished. Any idea how we can prevent that from happening?

A way I can think of doing this is to have a custom pre-loading script (https://developer.playcanvas.com/en/user-manual/designer/loading-screen/) where the instead of hiding the splash when the loading is done, it loads all the GLB containers. Once that is done, then hide the splash screen.

Note that the app is still being updated and rendered behind the splash screen though. It is possible to patch the engine (and this is where we starting to get a bit hacky) so that you can delay the call to application.start() to prevent this from happening.

If you would like an example, post in the forums and I whip one up as soon as I can :) (I've done something similar before with another client so shouldn't take too long)

@yaustar Thanks for your suggestions :) This really sounds like a quick solution with does the job. As soon as we find a way to work around the rendering issues, this will definitely come in handy.

FrantzVZ777 commented 3 years ago

@potentially-null Here below are feedbacks from the Artist who worked on this: https://magazine.substance3d.com/high-end-renders-online-bike-configurator-substance/

Regarding your issue, I'm not sure where the file comes from. The lowpoly i downloaded had custom normals, some Mesh Optimisation softwares like "pixyz" can do weird things with object's shading, maybe it's just corrupt?

The normal map looks clean (which means the highpoly is clean) so I really think its something to do with low-poly object. I'm pretty sure if you re-builds the lowpoly by hand (just as a test) and then tries to bake the same thing, then the problem will go away.

Maybe you can explain us the process you used to create the lowpoly model?

potentially-null commented 3 years ago

Hi @FrantzVZ777, thanks for your feedback.

The asset looks correct when imported to Unity with import settings that just load what's in the file, which is why I do not think the data is corrupt. Also, as mentioned above, the artifacts are much more visible on reflective materials. When did you last try import + setup one of your assets in PlayCanvas? Does this work for you without any issues with reflective, normal-mapped materials?

The low poly asset I shared above has not been auto-optimised, just exported from blender.

Our workflow:

Author 3d model in Blender, triangulate, smooth model, export FBX/GLB with tangents. Import FBX to Substance Designer/Painter, select tangent basis + OpenGL orientation, setup baker with high poly model, bake.

What do you mean by custom normals, exactly? Usually, we smooth normals of the low poly across uv islands and harden edges/split normals at uv seams, though the normals in the sample asset I've uploaded are just smooth. This should not make any difference, though, as long as the same model is used for baking and displaying the normal map.

Following your advice, I have re-edited and re-baked the test asset with hardened edges/split normals:

comaprison

With split normals, the reflection issues are gone. Unfortunately, split normals cause very noticable seems on the edges.

Could you possibly try if you get it to work with one of your assets, when you import it into a new project and assign any reflective, normal-mapped material?

psychoboy852 commented 3 years ago

@potentially-null Hi, I'm former colleague of @FrantzVZ777 Can you send me both highpoly and lowpoly models of yours? I will try to run them through my Marmoset workflow and tell you if the problem remains. If the problem disappears then I will do a write up on what you have to do.

potentially-null commented 3 years ago

@psychoboy852 Hi, sounds good. Thanks for your help! sample assets.zip

psychoboy852 commented 3 years ago

@potentially-null Hi again, I did a few tests and when I got rid of custom normals you had, got rid of triangles, re-arranged uv-layout and marked all sharp edges as seams it looked correct. Also your transforms (scale, rotation, position) were not at 1,1,1, but I don't think it was the issue. I have a feeling main issue was that you triangulating the mesh. I never do it - I think all modern softwares do it automatically on import by default. May sound silly but try to stick to defaults and not overcomplicate things. Screenshot (982) Screenshot (981) Screenshot (983) Screenshot (984) I highly suggest you use Marmoset, it's miles ahead of blender when it comes to baking. It's very easy to use. I put together few files so you can inspect them: Cube_test_2.zip

willeastcott commented 3 years ago

Wow, this is amazing - thanks so much for taking the time to investigate that @psychoboy852!

potentially-null commented 3 years ago

@psychoboy852 Hi, thanks again for looking into this and for sharing your altered assets. Importing your asset into the engine produces both correct lighting and non-distorted reflections for me as well - even with the editor fbx importer, which drops the tangents in the asset. However, now all of your vertex normals coincidentally are perfectly perpendicular to the geometry. It's good to know that it works in this particular case, however, for more organic models this would mean to split normals of the entire model. So, I am still unsure how to get correct results in those other cases where normals are not perfectly perpendicular. If you check the image @yaustar uploaded in the initial post, it's exaclty those cases I am talking about. Do you have any thoughts on that? How do you prep your normals of more organic assets before export? Hardening every single triangle and therefore essentially duplicating vertices cannot be a solution in the long run, but it's definitely a good quick-fix.

Also thank you for your suggestion regarding Marmoset. We aren't using blender but mostly the baker integrated to substance designer, though, which should not make a difference really. What tangent basis did you use in Marmoset when baking your normal map?

The scaling not being indentity is a good catch as well. I will double check the assets used in production tomorrow and see if this could have been (part of) the issue.

Not triangulating will work as long as all apps used in your asset pipeline triangulate the exact same way. If not, normal mapping will break unless normals are perfectly perpendicular, in which case is does not make any difference.

Thanks a lot for your feedback again! This has been super helpful already :)

Edit: Regarding custom split normals: Re-importing your fbx back into blender also ends up with custom split normals. Looks like imported normals simply show up as custom split normals in blender.

psychoboy852 commented 3 years ago

@potentially-null I see. Well, in that particular example that you sent (the one i replicated) it makes a lot of sense to square off uv's. Generally, if you can rectify the uv island than you should do it - it prevents pixels from stepping (when the edge of uv island is at 45 degree angle than in low res texture that will look like a staircase.

The process is the same for organic models too, just mark sharp + mark seam where it's logical to have hard edge. If you can send me a model (high and low) of that door handle I can try to do the same.

psychoboy852 commented 3 years ago

After a bit of google search it appears that your issue has most likely to do with triangulation and tangent space maybe they are out of synch, there are a lot of information on it, I'm sure you'll find your answer in here: https://marmoset.co/posts/toolbag-baking-tutorial/#tangents (scroll down to "Baking Basics" section ) https://polycount.com/discussion/198435/marmoset-toolbag-3-normal-baking-to-substance-painter-normals-not-displaying-properly https://polycount.com/discussion/214085/confused-about-tangent-spaces-marmo-sp-unity https://polycount.com/discussion/199865/marmoset-toolbag-how-do-i-fix-lighting-errors-in-a-tangent-space-normal-map https://forum.substance3d.com/index.php?topic=30395.0

potentially-null commented 3 years ago

@psychoboy852 The handle asset isn't mine, but I downloaded the low poly from the sample project @yaustar linked above, cleaned it up a bit and created a very basic high-poly for testing. Edit: Sorry, I uploaded to wrong zip handle sample assets.zip This time, the transformations should be identity.

@mvaligursky posted this link above, which talks about this exact issue. https://bgolus.medium.com/generating-perfect-normal-maps-for-unity-f929e673fc57 The question is, if you get the handle asset to work without hardening every polygon, what settings work for you when baking your normal map. I have tried all three built-in tangent plugins in substance designer, which can be loaded via project settings/general and none of them produced a normal map, which worked correctly in PlayCanvas. The exact same asset looked as expected in Unity when using the unrealmikkt.dll tangent basis plugin. Unity recently switched to mikktfrag tangent basis in URP, but Adobe didn't update the name of their dll.

Generally, if you can rectify the uv island than you should do it - it prevents pixels from stepping (when the edge of uv island is at 45 degree angle than in low res texture that will look like a staircase.

I agree, those stepping artifacts were even visible when I loaded my asset in Unity. I just kept it that way, because I wanted to test the common case where not all vertex normals are perpendicular to the triangles they belong to as is the case with many of our actual assets. I probably should have used a more organic object in the first place.

Thanks again for your time. I really appreciate it.

psychoboy852 commented 3 years ago

@potentially-null I did some more tests with this handle model and have the same ugly results as you do. Both in playcanvas and glb viewer. Marmoset and Eevee look fine however. Very confused about it as well now, because I never noticed these issues happening before. And we've done a lot of projects in playcanvas so if something this bad happen surely someone would've noticed.

I'll try to look into it more when I have time, but for now I have no idea.

potentially-null commented 3 years ago

We have finally found and implemented a workaround for this issue, which works for our assets without any modifications. As suspected in previous posts, the tangent basis is the issue here.

To make it work, we first load our geometry from .glb assets, which weren't created via the FBX import workflow. By doing so, tangent data of our assets is preserved and loaded at runtime. Additionally, our loading script assigns materials to submeshes and modifies the normal mapping shader code based on the specified tangent basis. We implemented MikktVert and MikktFrag.

loading_script

This is the shader code we inject:

if(material instanceof pc.StandardMaterial)
{
    if(mat.tangentBasis === TangentBasis.MikktVert)
    {
        material.onUpdateShader = (options : any) =>
        {
            options.chunks.tangentBinormalVS = [
                "vec3 getTangent() {",
                    "return normalize(dNormalMatrix * vertex_tangent.xyz);",
                "}",
                "vec3 getBinormal() {",
                    "return cross(vNormalW, vTangentW) * vertex_tangent.w;",
                "}",
                "vec3 getObjectSpaceUp() {",
                    "return normalize(dNormalMatrix * vec3(0, 1, 0));",
                "}\n"
            ].join("\n");
            //mikkt: do not re-normalize t, b, n after interpolation - mat3(t, b, n);
            options.fastTbn = true;

            options.normalizeNormalMap = false;
            options.chunks.normalMapFastPS = [
                "uniform sampler2D texture_normalMap;",
                "void getNormal() {",
                    "vec3 normalMap = unpackNormal(texture2D(texture_normalMap, $UV));",
                    "dNormalMap = normalMap;", // Detail normal mapping support omitted
                    //normalize after transformation
                    "dNormalW = normalize(dTBN * dNormalMap);",
                "}\n"
            ].join("\n");

            return options;
        };
    }
    else if (mat.tangentBasis === TangentBasis.MikktFrag)
    {
        material.onUpdateShader = (options : any) =>
        {
            options.chunks.tangentBinormalVS = [
                "vec3 getTangent() {",
                    "return normalize(dNormalMatrix * vertex_tangent.xyz);",
                "}",
                //add interpolator for sign of bitangent"
                "varying float vertex_tangent_sign;",
                "vec3 getBinormal() {",
                    //stream sign of bitangent to fragment shader
                    "vertex_tangent_sign = vertex_tangent.w;",
                    //unused - bitangent computed in fragment shader
                    "return vec3(0,0,0);",
                "}",
                "vec3 getObjectSpaceUp() {",
                    "return normalize(dNormalMatrix * vec3(0, 1, 0));",
                "}\n"
            ].join("\n");

            options.chunks.TBNPS = [
                "varying float vertex_tangent_sign;",
                "void getTBN() {",
                    //calculate bitangent in fragment shader
                    "dBinormalW = cross(dVertexNormalW, dTangentW) * vertex_tangent_sign;",
                    //mikkt: do not re-normalize t, b, n after interpolation - mat3(t, b, n);
                    "dTBN = mat3(dTangentW, dBinormalW, dVertexNormalW);",
                "}\n"
            ].join("\n");

            options.normalizeNormalMap = false;
            options.chunks.normalMapFastPS = [
                "uniform sampler2D texture_normalMap;",
                "void getNormal() {",
                    "vec3 normalMap = unpackNormal(texture2D(texture_normalMap, $UV));",
                    "dNormalMap = normalMap;", // Detail normal mapping support omitted
                    //normalize after transformation
                    "dNormalW = normalize(dTBN * dNormalMap);",
                "}\n"
            ].join("\n");

            return options;
        };
    }
}

@mvaligursky Thanks again for pointing us to the shader chunks API :)

Would be great if we could a) import tangents from an FBX if the FBX contains tangent data and b) add a dropdown in the normal mapping section of the standard material where the user can select the tangent basis as @guycalledfrank suggested

Also, if the user tries to render a model with a normal mapped material with a tangent basis requiring tangent data, i.e. any other than Derivative, but the model doesn't contain any, probably a warning should be emitted.