Unity-Technologies / com.unity.toonshader

Unity Toon Shader ( an experimental package )
Other
1.04k stars 162 forks source link

Shader error in 'Toon': 'OUTPUT_SH': Too many arguments for a macro call #325

Closed tangx246 closed 5 days ago

tangx246 commented 11 months ago

Describe the bug Starting from Unity 2023.1.7 (also reproducible in 2023.1.8), the shader fails with an error "Shader error in 'Toon': 'OUTPUT_SH': Too many arguments for a macro call", and we get the purple effect. In 2023.1.6, everything is working fine as expected.

To Reproduce Steps to reproduce the behavior:

  1. Create a new blank URP project using Unity 2023.1.8
  2. Add 'com.unity.toonshader' via the Package Manager
  3. Add the Toon Shader URP Samples
  4. Open up any of the sample scenes (e.g. in the screenshot, I used 'ToonShader_LightCulling'
  5. Observe error

Expected behavior The toon shader should work

Screenshots If applicable, add screenshots to help explain your problem. image

Desktop (please complete the following information):

viitana commented 11 months ago

Just confirming this is not a setup issue, I am also experiencing this on version 0.9.4-preview on Unity 2023.1.9f1 & URP 15.0.6.

yuki4080 commented 11 months ago

Hi, it seems that this issue has been impacted by the following bug fix. https://issuetracker.unity3d.com/product/unity/issues/guid/UUM-37723

2023.1.7f1 (Backport) https://github.com/Unity-Technologies/Graphics/commit/904e198aa8f7c04072ba3e6573f086162a62c0e0

2023.2.0a22 https://github.com/Unity-Technologies/Graphics/commit/e13cbd5495386bcc07f1dc1f5edaec7ddf7d6f25

You can resolve this issue by applying the following fix.

image
H3idi-X commented 11 months ago

Thank you for reporting! We really appreciate these contribution.  We confirmed the issue and the provided fix can improve it. We are checking the following fix with some other versions.

Screenshot 2023-08-22 at 5 37 03
yuki4080 commented 10 months ago

Is the Unity version specification in the macro incorrect? This issue occurs in Unity 2023.1.7 or later. Perhaps it should be #if UNITY_VERSION >= 202317. Therefore, Unity 2023.1.6 will cause shader errors.

H3idi-X commented 10 months ago

Thanks for correcting my mistake. We are checking with your suggested fix.

H3idi-X commented 10 months ago

Hi, we released Unity Toon Shader 0.9.5-preview. Could you please check the latest version?.

tangx246 commented 10 months ago

Issue appears to be fixed on Unity 2023.1.10 + Toon Shader 0.9.5-preview. Thanks a lot for the help!

KichangKim commented 1 week ago

Unity 6000.0.9f1 changed OUTPUT_SH4() and compilation error is revived.

H3idi-X commented 1 week ago

Thanks for reporting @KichangKim I confiremd the issue.

joe2275 commented 1 week ago

Unity 6000.0.9f1 changed OUTPUT_SH4() and compilation error is revived.

I'm not sure exactly what this function does, but I made the following modifications to prevent the error from occurring in the first place.

OUTPUT_SH4(positionWS, o.normalDir.xyz, GetWorldSpaceNormalizeViewDir(positionWS), o.vertexSH, o.vertexSH);

For me, this seems to work fine, but you might run into a problem.

H3idi-X commented 1 week ago

Thanks for the suggestion. We are checking whether the following fix works fine.

#if UNITY_VERSION >= 600009
                OUTPUT_SH4(positionWS, o.normalDir.xyz, GetWorldSpaceNormalizeViewDir(positionWS), o.vertexSH, o.vertexSH);
#elif UNITY_VERSION >= 202317
                OUTPUT_SH4(positionWS, o.normalDir.xyz, GetWorldSpaceNormalizeViewDir(positionWS), o.vertexSH);
#elif UNITY_VERSION >= 202310
                OUTPUT_SH(positionWS, o.normalDir.xyz, GetWorldSpaceNormalizeViewDir(positionWS), o.vertexSH);
#else
                OUTPUT_SH(o.normalDir.xyz, o.vertexSH);
#endif
riina commented 1 week ago

@H3idi-X the format for the UNITY_VERSION macro was changed for Unity 6, as seen in the Unity 6 documentation. In this case, the correct value for 6000.0.9f1 would be 60000009.

riina commented 1 week ago

Also, judging by the changes in the Graphics repo mirror, the added parameter is actually a float4 output for a light probe occlusion value which in this case should be some dummy variable or NOT_USED. I believe you should not pass o.vertexSH but instead do something like:

float4 probeOcclusionUnused;
OUTPUT_SH4(positionWS, o.normalDir.xyz, GetWorldSpaceNormalizeViewDir(positionWS), o.vertexSH, probeOcclusionUnused);
H3idi-X commented 1 week ago

Thank you very much @riina. We applied the fix and found no side effects in our graphics tests. We merged into the fix into develpment/v1.

H3idi-X commented 5 days ago

Hi, we releaseed 0.10.1-preview that includes the fix. Thank you!

image