Fixed the built-in target to have collapsible menus to match other pipeline

[joshua-davis]

Please read the Contributing guide before making a PR.

• Read the Graphics repository & Yamato FAQ.

Checklist for PR maker

• Have you updated the changelog? Each package has a CHANGELOG.md file.

---

Purpose of this PR

Why is this PR needed, what hard problem is it solving/fixing?
https://fogbugz.unity3d.com/f/cases/1339256/

The material inspector for shader graph built-in target shader didn't have collapsible menus like URP did. This required a new assembly reference on the srp.core/editor assembly which was deemed fine.
Additionally a small bug with sorting priority not working and not always being exposed was fixed.

---

Testing status

• View a built-in SG material without material override:
  • The Surface Options section displays no properties
  • All of the user's SG properties are in Surface Inputs section
  • The advanced section contains the render queue (which is currently locked and can't be changed as intended)
  • The advanced section contains the Sorting Priority (queue offset) which can be changed and updates the render queue accordingly
• Change a built-in SG material to material override
  • All of the same surface properties as before are in the Surface Options section
  • All of the user's SG properties are in Surface Inputs section
  • The advanced section contains the render queue (which is currently locked and can't be changed as intended)
  • The advanced section contains the Sorting Priority (queue offset) which can be changed and updates the render queue accordingly
• Verify that toggling On/Off material override updates the Surface Inputs section dynamically, even while the inspector has focus
• Check all of the above for an unlit and lit target

---

Comments to reviewers

Currently some options are duplicated in the Surface Options setting. This is being handled in another PR
"Enable GPU Instancing" wasn't added in the Advanced Options section as it doesn't currently work (I believe).
You can currently manually set the RenderQueue to "From Shader" which is fine.
An assembly reference to srp.core/editor was added to shader graph so MaterialHeaderScopeList can be used.

[jessebarker]

LGTM. I'll follow this up with the "Queue Control" stuff I added to the URP material inspector.

[GrantLamb-Unity]

Sufficient testing for the original bugfix.

I have a couple questions for the secondary issue you found during this though.

Additionally a small bug with sorting priority not working and not always being exposed was fixed.

Was this specifically just an issue for Built-In like the original bugfix? It kind of looks like @jessebarker is referencing he's fixing it in URP as well.
Is this something we could create Unit test(s) for?

[joshua-davis]

Sufficient testing for the original bugfix.

I have a couple questions for the secondary issue you found during this though.

Additionally a small bug with sorting priority not working and not always being exposed was fixed.

Was this specifically just an issue for Built-In like the original bugfix? It kind of looks like @jessebarker is referencing he's fixing it in URP as well.
Is this something we could create Unit test(s) for?

Yes this was just a bug in built-in. Jesse is fixing an independent issue in URP (which will also need to be fixed, but after my change). High level I had made the sorting priority field but it wasn't being used. Jesse is fixing how the render queue gets applied, either always from shader (which then uses the priority) or manually controlled.

I don't think we can create a test for this as it'd involve verifying that changing the sort priority on the inspector also changes the render queue value, but maybe I'm wrong. I haven't seen any tests for this kind of control yet.

[jessebarker]

Was this specifically just an issue for Built-In like the original bugfix? It kind of looks like @jessebarker is referencing he's fixing it in URP as well.
Is this something we could create Unit test(s) for?

Yes this was just a bug in built-in. Jesse is fixing an independent issue in URP (which will also need to be fixed, but after my change). High level I had made the sorting priority field but it wasn't being used. Jesse is fixing how the render queue gets applied, either always from shader (which then uses the priority) or manually controlled.

Right. What I did for the URP shader GUI in !4610 + !4920 was fixed a bug where the material settings were sort of stepping on each other, so I enabled a new "queue control" setting to allow the user to choose between the hand-written (Lit.shader, Unlit.shader) shader behavior, and the flexibility of the shader graph behavior unambiguously. Once this PR goes in, I can apply that same behavior to the built-in target and shader GUI so it will "match" URP (not in all actual rendering behavior due to pipeline differences, but in UX/UI at least). In theory, Josh could do that on this PR, but it would make it much larger, and I think we'd prefer to keep them separate.

[GrantLamb-Unity]

Was this specifically just an issue for Built-In like the original bugfix? It kind of looks like @jessebarker is referencing he's fixing it in URP as well.
Is this something we could create Unit test(s) for?

Yes this was just a bug in built-in. Jesse is fixing an independent issue in URP (which will also need to be fixed, but after my change). High level I had made the sorting priority field but it wasn't being used. Jesse is fixing how the render queue gets applied, either always from shader (which then uses the priority) or manually controlled.

Right. What I did for the URP shader GUI in !4610 + !4920 was fixed a bug where the material settings were sort of stepping on each other, so I enabled a new "queue control" setting to allow the user to choose between the hand-written (Lit.shader, Unlit.shader) shader behavior, and the flexibility of the shader graph behavior unambiguously. Once this PR goes in, I can apply that same behavior to the built-in target and shader GUI so it will "match" URP (not in all actual rendering behavior due to pipeline differences, but in UX/UI at least). In theory, Josh could do that on this PR, but it would make it much larger, and I think we'd prefer to keep them separate.

Ah, I see! Thanks for the additional context. Also thank you for linking the other PR's as well, I took a look into them and read up a bit more and that cleared up what your discussions were about!

[joshua-davis]

Passing green nightly run before merging to fix changelog: https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/sg%252Fbuiltin-material-inspector/.yamato%252Fall-shadergraph.yml%2523Nightly_ShaderGraph_trunk/7286748/job