Search Unity

Question BUG?: Shader graph fragment-based low-poly normal regeneration = impossible?

Discussion in 'Shader Graph' started by Arkade, Oct 10, 2019.

  1. Arkade

    Arkade

    Joined:
    Oct 11, 2012
    Posts:
    655
    Yet another mediocre shader programmer trying out shader graph and getting confused so apologies if this is mistaken.

    I think it's impossible to regenerate normals in the current (2019.2.8f1) version of Shader Graph (6.9.1) at least in LWRP.

    ShaderGraph-both-2.gif

    I'm doing the usual low-poly water (which I've done using surface shaders with vertex code before). I'm using some time-based noise in the graph (executed in the vertex code) to push the vertices along their normals (or Y-up). I'm going for 'lit low-poly' so I then want to regenerate the normals (rather than colouring myself). I use the usual
    Code (CSharp):
    1. normalize(cross(ddx(posWorld),ddy(posWorld)))
    in the graph (executed in the fragment shader) and, since PBR shader requires tangent-space normals, I use a Transform` node to convert it. (for curiousity, here's the project I'm trying to convert while it's still using legacy shader -- you see the aesthetic difference here!)



    All sound fair?

    The problem is that I can see no observable change in the normals in the Game View.

    I checked the generated code and found that the generated vertex code calculates the
    Code (CSharp):
    1. WorldSpace(Position|Normal|Tangent|BiTangent|ViewDirection)
    which it passes to the graph logic (in a function called PopulateVertexData()) then copies the returned Position into the GraphVertexInput but copies the original WorldSpacePosition etc to the GraphVertexOutput -- the thing the fragment shader part of the graph/code gets to use. I even tried using Object space (rather than World space) in the normal calculation but the generated code is actually still using the same World space value but converting it back into Object space!

    What does this mean? Well, since those values ignore the graph's changes to the position, the fragment shader sees the original values of the position so generates the original normal = (0,1,0)!

    My next step to confirm this is copying the generated code and seeing if I can correct it and have it work correctly. Of course I recognise that I might need to do the same things to the Transform matrix since it is also using the unmodified position. What I'm not sure is whether it'll matter if I use the unmofidied tangent, bitangent etc -- my maths starts getting mighty rusty at this level!

    If you see any problems with my analysis, conclusions or plan, ... well, anything really, I'd love to hear. Hopefully someone will point me to a simple "oh you need to tick this box and it'll all work" feature I've missed :D I've read several posts from @bgolus explaining things around this subject including one that shows it working making me think this might be a regression?

    Thanks in advance for any thoughts / hints !

    p.s. graph attached as .txt since Forum doesn't allow .shadergraph yet!
     

    Attached Files:

    Last edited: Oct 10, 2019
  2. Arkade

    Arkade

    Joined:
    Oct 11, 2012
    Posts:
    655
    Done and it worked.

    Copied the generated code to a new shader and, after the shader graph code (PopulateVertexData()), added a duplication of the calculation of the WorldSpacePosition (using the v.vertex value from the graph), i.e.

    Code (CSharp):
    1. WorldSpacePosition = mul(UNITY_MATRIX_M,v.vertex).xyz;
    Works perfectly!

    So I'd say this is a bug/regression. Time to use the "Log a bug" facility in Unity!

    ShaderGraph-fixed.gif
     
  3. LandonTownsend

    LandonTownsend

    Unity Technologies

    Joined:
    Aug 21, 2019
    Posts:
    35
    Hello,
    This is fixed in the latest version of Shadergraph (version 7.1.2)
    Old:
    upload_2019-10-10_15-32-57.png
    New:
    upload_2019-10-10_15-33-13.png
    7.1.2 is available with 2019.3.0b4 and up. If you decide to upgrade your project, make sure to back it up before upgrading.
     
    Arkade likes this.
  4. Arkade

    Arkade

    Joined:
    Oct 11, 2012
    Posts:
    655
    Super & thanks for replying. I'm probably safe to upgrade. That said, this project is a test bed for another project that might not be able to so 2 questions:
    1. What plans are there for back-porting?
    2. Can you share a reference or link to refer to, please?
    Thanks again.
     
  5. LandonTownsend

    LandonTownsend

    Unity Technologies

    Joined:
    Aug 21, 2019
    Posts:
    35
    There are no plans for back-porting unfortunately, as the fix was a refactor that if back-ported could introduce regressions. I'm not entirely clear about what you mean by "reference or link to refer to," I don't believe that there was a case for this bug being tracked when it was fixed in the refactor, so I don't have a tracked issue link to share.
     
  6. YOSSI2010

    YOSSI2010

    Joined:
    Dec 26, 2016
    Posts:
    20
    Using the latest version of unity(2019.3) and URP and shadergraph version 7.1.5 the shader still looks like your old example and does not update the normals.
    upload_2019-11-28_16-16-14.png
     
  7. LandonTownsend

    LandonTownsend

    Unity Technologies

    Joined:
    Aug 21, 2019
    Posts:
    35
    Still works for me, make sure you recreate the "Normal" graph in the OP attachments and plug that into the normals slot.

    upload_2019-12-6_14-30-8.png
    upload_2019-12-6_14-35-10.png
     
    jjbish and florianhanke like this.