Skip to content

Move Screen Space Shadow to multicompile and remove config requirement for DXR #960

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Jun 18, 2020
Merged

Move Screen Space Shadow to multicompile and remove config requirement for DXR #960

merged 13 commits into from
Jun 18, 2020

Conversation

sebastienlagarde
Copy link
Contributor

Purpose of this PR

This PR is the final PR that allow to remove the requirement of shaderconfig to use DXR.
It move the usage of screen space shadow as a multicompile instead of a define.
#pragma multi_compile SCREEN_SPACE_SHADOWS_OFF SCREEN_SPACE_SHADOWS_ON
The extra multicompile is stripped based on the configuration. Support is also added to shader graph version.

The PR remove the macro:
ENABLE_RAYTRACING, SHADEROPTIONS_RAYTRACING,
All the relevant entry related to this change are removed from the wizard

The PR also re-introduce a small optimization for colored shadow

Testing status

Manual Tests: What did you do?

  • Opened test project + Run graphic tests locally
  • Built a player
  • Checked new UI names with UX convention
  • Tested UI multi-edition + Undo/Redo + Prefab overrides + Alignment in Preset
  • C# and shader warnings (supress shader cache to see them)
  • Checked new resources path for the reloader (in developer mode, you have a button at end of resources that check the paths)
  • Other:

Automated Tests: What did you setup? (Add a screenshot or the reference image of the test please)

Yamato: (Select your branch):
https://yamato.prd.cds.internal.unity3d.com/jobs/902-Graphics

Any test projects to go with this to help reviewers?

Comments to reviewers

Notes for the reviewers you have assigned.

Base automatically changed from HDRP/cleanup-have-deffered-rendering to HDRP/staging June 18, 2020 01:16
@sebastienlagarde sebastienlagarde merged commit 7ad7c61 into HDRP/staging Jun 18, 2020
@sebastienlagarde sebastienlagarde deleted the HDRP/screen-space-shadow branch June 18, 2020 03:16
sebastienlagarde added a commit that referenced this pull request Jun 18, 2020
@sebastienlagarde @JulienIgnace-Unity
@FrancescoC-unity @JordanL8 @anisunity @fabien-unity @alelievr @jessebarker @EvgeniiG @remi-chapelain @pmavridis @skhiat
[10.x.x] Merge Hdrp/staging [Skip CI] (#933)
de56158 
* Adding purge of unused resources in render graph. (#872)

* First round of auto-exposure docs updates (#858)

* start of volume docs (need switching branch)

* moar docs

* Reviewed exposure override

* Reviewed render pipeline debug window doc and changed some formatting

* Changed html links to md

* Update HDRP-Camera.md

* Removed rogue space

* Small changes to the exposure page

Co-authored-by: Lewis Jordan <lewis.jordan@hotmail.co.uk>

* Fix depth offset and transparent motion vector for unlit transparent (#896)

* Fix depth offset and transparent motion vector for unlit transparent

* cleanup and fix the shadow pre/post alpha clip

* Fixed an exception occuring when a camera doesn't have an HDAdditionalCameraData (1254383). (#846)

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* Fix Generates Include command issue with Physically Based Sky (#908)

* Split GenerateHLSL code from PhysicallyBasedSkyRenderer in a separate file to fix Generator Includes tool

* Update ShaderVariablesPhysicallyBasedSky.cs.hlsl

* Fix ray tracing with XR single-pass (#891)

* removed warnings related to ray binning in XR

* fix RenderRayTracedReflections with XR

* add errors if Path Tracing is used with XR single-pass

* add missing shader macros

* add entry to changelog

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* Hdrp/clean shader stacks fields (#906)

* Cleanup the HD fields

* Fix distortion appearing two times in the UI

* Pbr sky fixes metal (#911)

* Split GenerateHLSL code from PhysicallyBasedSkyRenderer in a separate file to fix Generator Includes tool

* Update ShaderVariablesPhysicallyBasedSky.cs.hlsl

* Fix constant buffer layout for physically based sky
    - Update Vector3 to Vector4 to reflect actual padding/layout for non-DX
      platforms (both are 16 bytes in the shader space, but 12 bytes in C#)
    - Updates to shader code to reference only the float3 data being set up

* Update references screenshots

Co-authored-by: Sebastien Lagarde <sebastien@unity3d.com>

* Cleaun shader config for shadow (#917)

* Move HDShadowFilteringQuality to main package as it is not used anymore

* Update Upgrading-from-2020.1-to-2020.2.md

* Add comment for _IncludeIndirectLighting for emissive mesh and clean ShaderPas template code

* Fix shader warning

* Fix shader config in DXR projects

* Fix warnings (#920)

* Enable render graph test (#873)

* re revert the PR

* Fixed planar probes with render graph.

* Fixed an issue with msaa resolve pass when movecs aren't enabled.

Co-authored-by: Julien Ignace <julien@unity3d.com>

* Add light layer on indirect lighting controller (#777)

* Add support for reflection controller.

* Fix compilation

* Fixed compilation issue and naming

* Fixed compilation issue and naming

* Update IndirectLightingControllerEditor.cs

* Update CHANGELOG.md

* Update Override-Indirect-Lighting-Controller.md

* Update Override-Indirect-Lighting-Controller.md

* Update Override-Indirect-Lighting-Controller.md

* Update. Indirect controller also affect planar

Co-authored-by: Sebastien Lagarde <sebastien@unity3d.com>

* Update IndirectLightingController.cs

* Compute shader stripping (#919)

* Compute shader stripping

* changelog

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* Enable DXR automated tests for XR (#915)

* disable unsupported dxr tests in xr

* enable dxr tests in xr on Yamato

* add HDRP_DXR playmode_XR to ABV

* Change guards around ESRAM code  (#922)

* Change unity version guards for ESRAM

* move commas

* Bump min version (#923)

* Add a comment about packing float3/float4

* Added Opaque Cull Mode option for materials and ShaderGraph (#918)

* Added the CullMode property for opaque objects

* Updated changelog

* Updated Opaque 8101 test

* Fix SG preview and settings indent level

* Add culmode opaque unlit

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* Add test scene for SG vertex normal & tangent (#905)

* Add test scene for vertex normal and vertex tangent

* Add references images for other API

* Updated the 2020.1 upgrade guide for custom pass volumes (#924)

* HDRP Upgrade documentation for terrain new upgrade path (#901)

* Upgrading documentation update

* Typo

* Added link to Terrain class

* Fix warning in HDAdditionalLightData OnValidate (#885)

* fix warning in HDAdditionalLightData OnValidate

* Changelog

* Avoid calling update

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* [10.x.x@ Update screenshots of test 8101 Opaque for HDRP test for Linux and Win Vulkank

* Add the list of HDRP forbidden keywords in SG blackboard (#926)

* Add the list of forbidden keywords to use in SG blackboard in the doc

* Update Customizing-HDRP-materials-with-Shader-Graph.md

* Update Customizing-HDRP-materials-with-Shader-Graph.md

* Add [MainColor] and [MainTexture] on property (#939)

* Scene exposure override (#889)

* Scene Exposure Override

* Changelog

* better changelog

* fix

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* Exposure curve remapping can specify curves for min/max limits (#882)

* Port code

* changelog

* Revert HDRP asset

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* IES Code path refactoring to support multiple SRP (#929)

* Refactor

* Refacto to support multiple SRPs

* Missing doc + Re-enable PreviewGUI in the Inspector

* Add Icons for IES Profiles

* Rename Compositor to Graphics Compositor (#950)

* Rename (HDRP) Compositor to Graphics Compositor

* Missed a few occurancies

* Ray traced reflection presets and fixes (#912)

* - Fixed a bug related to denoising ray traced reflections.
- Added presets for ray traced reflections.

* Testing the asset of the ray tracing usage parameters in ssr

* Mistake in history rejection for RTR

* Only use one texture for ray traced reflection upscaling
Adjust the upscale radius based on the roughness value

* Adjusted the preset values

* Updating the screenshots after changing the upscale radius metric.

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* Fixed a warning in the ray tracing ambient occlusion compute shader. (#953)

* - Fixed a warning in the ray tracing ambient occlusion compute shader.

* Update CHANGELOG.md

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* - Changed the way the filter size is decided for directional, point and spot shadows (#951)

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* Various code cleanup in HDRP indirect diffuse lighting + Start remove macro RAYTRACING (#937)

* Change code to have a indirectDiffuseMode and start to clean RaytracingMacro

* Little extra cleanup

* Final Image Histogram Debug View (#887)

* Final image histogram ported

* changelog

Co-authored-by: sebastienlagarde <sebastien@unity3d.com>

* Cleanup area shadow code and Macro + remove more Raytracing macro (#945)

* Change code to have a indirectDiffuseMode and start to clean RaytracingMacro

* Little extra cleanup

* Clean area shadow code and remove usless macro

* update shader config files for projects

* Better naming for SKIP_RASTERIZED_AREA_SHADOWS

* Update Upgrading-from-2020.1-to-2020.2.md

* remove useless comment

* HDRP: Fix shader warning in RaytracingAmbientOcclusion.compute

* HDRP: Fix standalone build (Break in Exposure scene settings PR)

* HDRP: Update HDRenderPipeineResources.asset not up to date

* Cleanup HAVE_RECURSIVE_RENDERING usage (#958)

* Change code to have a indirectDiffuseMode and start to clean RaytracingMacro

* Little extra cleanup

* Clean area shadow code and remove usless macro

* update shader config files for projects

* Better naming for SKIP_RASTERIZED_AREA_SHADOWS

* Update Upgrading-from-2020.1-to-2020.2.md

* remove useless comment

* cleanup have deferred rendering

* Move Screen Space Shadow to multicompile and remove config requirement for DXR (#960)

* Change code to have a indirectDiffuseMode and start to clean RaytracingMacro

* Little extra cleanup

* Clean area shadow code and remove usless macro

* update shader config files for projects

* Better naming for SKIP_RASTERIZED_AREA_SHADOWS

* Update Upgrading-from-2020.1-to-2020.2.md

* remove useless comment

* cleanup have deferred rendering

* Clean RaytracingIndirectDiffuse.compute code

* Move screen space shadow to a multicompile + remove Raytracing macro

* Remove config package from DXR Test

Co-authored-by: JulienIgnace-Unity <julien@unity3d.com>
Co-authored-by: FrancescoC-unity <43168857+FrancescoC-unity@users.noreply.github.com>
Co-authored-by: Lewis Jordan <lewis.jordan@hotmail.co.uk>
Co-authored-by: anisunity <42026998+anisunity@users.noreply.github.com>
Co-authored-by: Fabien Houlmann <44069206+fabien-unity@users.noreply.github.com>
Co-authored-by: Antoine Lelievre <antoinel@unity3d.com>
Co-authored-by: Jesse Barker <jesseb@unity3d.com>
Co-authored-by: Evgenii Golubev <EvgeniiG@users.noreply.github.com>
Co-authored-by: remi-chapelain <57442369+remi-chapelain@users.noreply.github.com>
Co-authored-by: Pavlos Mavridis <pavlos.mavridis@unity3d.com>
Co-authored-by: skhiat <55133890+skhiat@users.noreply.github.com>
FrancescoC-unity
FrancescoC-unity reviewed Jun 18, 2020
View reviewed changes
com.unity.render-pipelines.high-definition/Runtime/Lighting/LightLoop/DeferredTile.shader
@@ -305,6 +305,7 @@ Shader "Hidden/HDRP/DeferredTile"
#pragma multi_compile _ OUTPUT_SPLIT_LIGHTING
#pragma multi_compile _ DEBUG_DISPLAY
#pragma multi_compile _ SHADOWS_SHADOWMASK /// Variant with and without shadowmask
#pragma multi_compile SCREEN_SPACE_SHADOWS_OFF SCREEN_SPACE_SHADOWS_ON
Copy link
Contributor

@FrancescoC-unity FrancescoC-unity Jun 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

why not _ SCREEN_SPACE_SHADOW ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the stripper. If you don't give a name to the "implicit" version, i.e
#pragma _ SCREEN_SPACE_SHADOWS
then you can't reject the implcit one. Here we want to reject one or the other, they are exclusive,

this is different to have always the implicit and deciding to have the second or not like for debug
#pragma multi_compile _ DEBUG_DISPLAY

anisunity
anisunity reviewed Jun 18, 2020
View reviewed changes
com.unity.render-pipelines.high-definition/Runtime/Lighting/LightEvaluation.hlsl
@@ -421,7 +421,7 @@ SHADOW_TYPE EvaluateShadow_Punctual(LightLoopContext lightLoopContext, PositionI
LightData light, BuiltinData builtinData, float3 N, float3 L, float4 distances)
{
#ifndef LIGHT_EVALUATION_NO_SHADOWS
SHADOW_TYPE shadow = 1.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

how did this happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In previous PR I have made the change. It was actually the right thing to do, but after discussing with Francesco, I was willing to come back to a single float even if SHADOW_TYPE is float3 to save ALU as long as we don't have supported of colored point light shadow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anisunity anisunity anisunity left review comments

@FrancescoC-unity FrancescoC-unity FrancescoC-unity left review comments

Assignees
No one assigned
Labels
HDRP testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sebastienlagarde @anisunity @FrancescoC-unity