Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Per-stage shader keywords in HDRP #2996

Merged
merged 22 commits into from
Jan 18, 2021

Conversation

FrancescoC-unity
Copy link
Contributor

With this PR HDRP will use per-stage keywords what that means is that we can do stuff like multi_compile_fragment to have only the fragment shader variant computed. It is very important to define one separate line per stage that needs to include the keyword. This work for both multi compiles and shader_features

This is has to be done manually for code shaders (I did a pass on our shaders, being conservative to be safe) and I added a field to have that easily generated in SG.

This reduces the number of compiled shader programs with all the benefits that come from that.

I ran test manually and they pass.

# Conflicts:
#	com.unity.render-pipelines.high-definition/Editor/Material/Decal/ShaderGraph/HDDecalSubTarget.cs
#	com.unity.render-pipelines.high-definition/Editor/ShaderGraph/HDTarget.cs
#	com.unity.render-pipelines.high-definition/Editor/ShaderGraph/Nodes/RayTracingNode.cs
#	com.unity.render-pipelines.high-definition/Runtime/Lighting/LightLoop/DeferredTile.shader
#	com.unity.render-pipelines.high-definition/Runtime/Material/AxF/AxF.shader
#	com.unity.render-pipelines.high-definition/Runtime/Material/LayeredLit/LayeredLit.shader
#	com.unity.render-pipelines.high-definition/Runtime/Material/Lit/Lit.shader
# Conflicts:
#	com.unity.render-pipelines.high-definition/Runtime/Material/AxF/AxF.shader
#	com.unity.render-pipelines.high-definition/Runtime/Material/Decal/Decal.shader
#	com.unity.shadergraph/Editor/Generation/Processors/Generator.cs
@FrancescoC-unity FrancescoC-unity self-assigned this Jan 6, 2021
@FrancescoC-unity FrancescoC-unity requested a review from a team as a code owner January 6, 2021 14:18
@FrancescoC-unity FrancescoC-unity changed the title Per-stage shader keyword Per-stage shader keywords in HDRP Jan 6, 2021
@FrancescoC-unity FrancescoC-unity marked this pull request as draft January 6, 2021 14:18
RSlysz pushed a commit that referenced this pull request Jan 6, 2021
* Enable greetings.yaml

* Save yml files

* Add comments

* Add .github folder to codeowners
RSlysz pushed a commit that referenced this pull request Jan 6, 2021
* Revert "Explicitly disable unsupported github actions (#2996)"

This reverts commit 6491e3f.

* Revert "Update CODEOWNERS"

This reverts commit abd1554.

* Add .github to CODEOWNERS + Uncomment github action
@github-actions
Copy link

github-actions bot commented Jan 6, 2021

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page)
and cancel any jobs that started on Yamato.
See the PR template for more information.
Thank you!

Copy link
Member

@alelievr alelievr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

(with the TODO fixed)

@sebastienlagarde sebastienlagarde marked this pull request as ready for review January 14, 2021 15:57
@sebastienlagarde sebastienlagarde requested a review from a team January 14, 2021 15:58
@sebastienlagarde
Copy link
Contributor

Adding QA: QA please try to load a project like SpaceShip or Palace, and build a player a check that everything is fine. Thanks

@ghost ghost self-requested a review January 14, 2021 22:20
Copy link
Contributor

@iM0ve iM0ve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have upgraded Amalienborg Palace project and also built a Player. Did not notice any visual issues or any new console errors

image

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just made sure that URP still works properly when HDRP is in project.

@sebastienlagarde sebastienlagarde merged commit d09ee40 into master Jan 18, 2021
@sebastienlagarde sebastienlagarde deleted the HDRP/shader-features-frequency branch January 18, 2021 18:49
@sebastienlagarde sebastienlagarde restored the HDRP/shader-features-frequency branch January 19, 2021 19:09
sebastienlagarde added a commit that referenced this pull request Jan 19, 2021
* Per-stage shader keywords in HDRP -bis #2996

* Fix DX12 alembic test

* miss SG

* Update LayeredLit.shader
@sebastienlagarde sebastienlagarde deleted the HDRP/shader-features-frequency branch April 26, 2021 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants