[ShaderGraph] [backport] [2021.2] Use VPOS for fragment stage screenspace calculations #5817
Conversation
…creenspace calculations (#5428) * Using SV_Position to calculate screenspace, instead of projection equations. This might be incorrect though for anyone who wants to read zw... * Working URP ScreenPositions * Adding changelog * Adding "ScreenPosition_Accuracy" to the input nodes tests * Fixes for HDRP target * Updating reference images * Fixing vertex color slot -- was using wrong interface * Fix for HDRP * Fix VFX ScreenPos usage * Convert VFX to use VPOS in fragment shader # Conflicts: # com.unity.shadergraph/CHANGELOG.md # com.unity.shadergraph/Editor/Data/Util/ScreenSpaceType.cs
|
Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed. HDRP URP Shader Graph VFX Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure. |
* Fix inconsistent NeedsPositionWorldInterpolator Use common function to factorize the requirement test from SG Regression introduced by #5428 * Better fix & Better performance Isolate screen computation from the world dependancy See #5784 (comment) /!\ Remove the VFXTransformPositionWorldToClip evaluated in fragment /!\ GraphicTest are 🟢 locally * Fix incorrect computation of INSG.ScreenPosition If we want to be consistent with SG implementation, we have to reproduced this code: https://github.com/Unity-Technologies/Graphics/blob/75f4b421fa774d43000d703f2bbf05d7a1ca6606/com.unity.render-pipelines.universal/Editor/ShaderGraph/Templates/SharedCode.template.hlsl#L56 ``` $SurfaceDescriptionInputs.ScreenPosition: output.ScreenPosition = ComputeScreenPos(TransformWorldToHClip(input.positionWS), _ProjectionParams.x); ``` Checked manually the values were consistent with VFX
cdxntchou
requested review from
a team,
RemyUnity,
julienf-unity and
xiaoxicici
cdxntchou
changed the title
|
@cdxntchou what is the status of this PR? |
|
Closing this for now, the number of regressions on the change in trunk is too high |
The refactor from #5817 hasn't been backported
7 participants
Purpose of this PR
*** This backport is paused, waiting on full fix for regression introduced by original PR ***
Fix for https://fogbugz.unity3d.com/f/cases/1352662/
2022.1:
2021.2 backport:
(Console Test PR)
This PR includes both of the 2022.1 PRs.
TODO: include third PR
The issue is that the calculation for screenspace position is calculated from world space position via applying the matrix transforms. However, the calculation is prone to floating point error, that is exacerbated greatly when the camera is far from the origin and the pipeline is not using camera-relative "world" space.
You can see in the examples below that the error can easily grow to be larger than a pixel.
This PR fixes this by changing the calculation in the fragment shader, to be based on the value returned by the VPOS fragment input. This value is not based on the projection matrices, independent of camera position, and much more accurate overall.
Docs ticket: https://jira.unity3d.com/browse/GSG-593
Test example:
Left sphere visualizes frac(screenspace pixel coordinate) using a gradient (red, black, green) where 0.5 is black.
Right sphere shows a screenspace checkerboard alphatest pattern
Before: (at 20000 distance from origin) -- quite a bit of inaccuracy
After: (at 20000 distance from origin) -- perfect 0.5 centered pixel coordinates everywhere!
Implementation Details:
We introduce two new global values that ShaderGraph can produce, and nodes can request, in addition to ScreenPosition.
The values are defined as:
PixelPosition 2D pixel coordinate of the current fragment or vertex. Pixels are centered at 0.5.
NDCPosition 2D coordinate (0,1) across the viewport; equal to PixelPosition / ScreenResolution.
ScreenPosition (raw) 4D Clip Space homogenous coordinates of the current fragment or vertex.
Both PixelPosition and NDCPosition are calculated from VPOS in the fragment stage, and are calculated from ScreenPosition in other stages.
Testing status
Tested in both HDRP and URP:
Added a Yamato test to validate screen position accuracy in fragment shader. Previously you would have a good amount of error in the screen position from the floating point roundoff on the matrix transform calculations. Now it gives exact pixel coordinate values out to at least 3 fractional digits.
Updated reference images. Note that the new accuracy actually changes the test results for the dither node, because the position is slightly different (now more accurate), and the specific test we use is extremely position sensitive.
Master base branch: 34fcac4
Yamato:
ShaderGraph: 🟢
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/2021.2%252Fsg%252Ffix%252F1352662/.yamato%252Fall-shadergraph.yml%2523Nightly_ShaderGraph_2021.2/9020876/job/pipeline
HDRP: 🟡 -- failures same as master
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/2021.2%252Fsg%252Ffix%252F1352662/.yamato%252Fall-hdrp.yml%2523PR_HDRP_2021.2/9018618/job/pipeline
master:
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/2021.2%252Fstaging/.yamato%252Fall-hdrp.yml%2523PR_HDRP_2021.2/8982528/job/pipeline
failures: -- same as master
HDRP on Linux_Vulkan_playmode_mono_Linear on version 2021.2 -- same 2 failures (after rerun) 🟡
HDRP on OSX_Metal_playmode_mono_Linear on version 2021.2 -- same 3 failures 🟡
HDRP on Win_Vulkan_playmode_mono_Linear on version 2021.2 -- same 2 failures 🟡
HDRP on Win_DX12_playmode_mono_Linear on version 2021.2 -- same 2 failures 🟡
HDRP on Win_DX11_playmode_XR_mono_Linear on version 2021.2 -- same 2 failures (after rerun) 🟡
HDRP on Win_DX11_playmode_mono_Linear on version 2021.2 -- same 2 failures 🟡
URP: 🟡 -- failures same as master
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/2021.2%252Fsg%252Ffix%252F1352662/.yamato%252Fall-urp.yml%2523PR_URP_2021.2/9021101/job/pipeline
master:
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/2021.2%252Fstaging/.yamato%252Fall-urp.yml%2523PR_URP_2021.2/8982696/job/pipeline
failures: -- same as master
URP_Lighting on Android_OpenGLES3_Standalone_il2cpp_Linear on version 2021.2 -- non-test-related failures on both 🟡
URP_Lighting on Win_Vulkan_Standalone_mono_Linear on version 2021.2 -- same 1 failure 🟡
URP_Terrain on OSX_Metal_playmode_mono_Linear on version 2021.2 -- green on rerun 🟢
URP_Foundation on Android_Vulkan_Standalone_il2cpp_Linear on version 2021.2 -- green on rerun 🟢
Console Tests:
Console Test PR:
Comments to reviewers
Notes for the reviewers you have assigned.