[ShaderGraph][2019.4][Backport] Ensure varying ordering by making instance ID SystemGenerated

[jessebarker]

Purpose of this PR

This PR fixes https://fogbugz.unity3d.com/f/cases/1369011/

We have a mechanism for ensuring that varyings with certain semantics are ordered last in the structure declaration. Instance ID needs to be handled this way, but wasn't marked with the correct attributes. This fixes that (could not be a direct backport of #4806 due to changes in the Generator and Targets).

---

Testing status

Created PBR shader graph, and generated code from it to observe that instance ID follows the texcoord varyings.

---

Comments to reviewers

Notes for the reviewers you have assigned.

[thomas-zeng]

Hi Ryan, Could you help test this PR on Quest? (see https://fogbugz.unity3d.com/f/cases/1369011/)
Thank you in advance!

[thomas-zeng]

Hi @ryanhy-unity,
Could you also help verifying the issue doesn't show up with latest URP11(2020.3)? Jesse was not able to repro the issue on 2020.3 and if you could help confirm it that would be super helpful!
Thank you in advance!

[ryanhy-unity]

Hi @ryanhy-unity, Could you also help verifying the issue doesn't show up with latest URP11(2020.3)? Jesse was not able to repro the issue on 2020.3 and if you could help confirm it that would be super helpful! Thank you in advance!

in 2020.3 using URP 10.8.0 (verified package) I see no shader compile errors with these shaders. However, the unlit versions of these shaders do not appear on the Quest. They are casting shadows like they are there but are not rendering themselves.

I also checked 2019.4 and the latest 7.x and all the shaders built and rendered.

Hope this info helps.

[thomas-zeng]

Thanks Ryan for testing out the PR.
I believe the shaders do not appear on Quest for 20.3 is because of the shader compile issue(not sure why the issue is not showing up in editor). So it is very likely we will want to backport the same fix to 20.3 as well.

@jessebarker what do you think?

[jessebarker]

Thanks Ryan for testing out the PR. I believe the shaders do not appear on Quest for 20.3 is because of the shader compile issue(not sure why the issue is not showing up in editor). So it is very likely we will want to backport the same fix to 20.3 as well.

@jessebarker what do you think?

My testing on 2020.3 shows the packed varyings in the correct order in the generated shader, so I would not expect to see it reproduce there. My real concern is that Ryan was unable to reproduce on 2019.4, which is the version the original bug was filed against (https://fogbugz.unity3d.com/f/cases/1361049/). I really don't want to introduce code that isn't demonstrably fixing something (especially to an LTS release).

[thomas-zeng]

@jessebarker I completely agree with you. If the issue is not reproducible then we should not do this backport. Checking the fogbugz case, looks like our QA reproduced it at some point though.
Is this something @ryanhy-unity you could help with? (confirm the issue is reproducible without the PR change and is fixed by the PR change)

[ryanhy-unity]

ran through a couple scenarios with the project on the bug and there were no compilation errors for the shader (in either the 7.x release URP or this branch). However, when running the built app on the Quest I would get OpenGL errors when using multiview but not multipass. The shader was not rendering using multiview. So maybe the shader is compiling now but is still causing issues on device?
Here's the error I'm seeing spam into the log

10-04 16:11:45.001 26075 26120 E Unity :
10-04 16:11:45.015 26075 26120 E Unity : OPENGL NATIVE PLUG-IN ERROR: GL_INVALID_OPERATION: Operation illegal in current state
10-04 16:11:45.015 26075 26120 E Unity : (Filename: ./Runtime/GfxDevice/opengles/GfxDeviceGLES.cpp Line: 437)

I'm not approving this as it doesn't fix any shader compilation error and the issue where the shader is not rendering and throwing errors is still there. Overall this PR doesn't seem to fix anything.

[jessebarker]

Since we can't seem to reproduce the problem we're trying to fix, I'm going to close this PR and the backport cases. We can reopen if we get a more reliable repro case.