Fix the clip-space bug (+ add explanation)

ScriptableRenderPipeline/Core/CoreRP/ShaderLibrary/Common.hlsl

 
 // Using pow often result to a warning like this
 // "pow(f, e) will not work for negative f, use abs(f) or conditionally handle negative values if you expect them"
-// PositivePow remove this warning when you know the value is positive or 0 and avoid inf/NAN.
+// PositivePow remove this warning when you know the value is positive or 0 and avoid inf/NAN.
 // Note: https://msdn.microsoft.com/en-us/library/windows/desktop/bb509636(v=vs.85).aspx pow(0, >0) == 0
 TEMPLATE_2_REAL(PositivePow, base, power, return pow(abs(base), power))
 
                                        0, 0, 1, 0,
                                        0, 0, 0, 1};
 
-// Use case examples:
-// (position = positionCS) => (clipSpaceTransform = use default)
-// (position = positionVS) => (clipSpaceTransform = UNITY_MATRIX_P)
-// (position = positionWS) => (clipSpaceTransform = UNITY_MATRIX_VP)
-float4 ComputeClipSpaceCoordinates(float3 position, float4x4 clipSpaceTransform = k_identity4x4)
+float4 ComputeClipSpacePosition(float2 positionNDC, float deviceDepth)
-    float4 positionCS = mul(clipSpaceTransform, float4(position, 1.0));
+    float4 positionCS = float4(positionNDC * 2.0 - 1.0, deviceDepth, 1.0);
-    // Our clip space is correct, but the NDC is flipped.
-    // Conceptually, it should be (positionNDC.y = 1.0 - positionNDC.y), but this is more efficient.
+    // Our world space, view space, screen space and NDC space are Y-up.
+    // Our clip space is flipped upside-down due to poor legacy Unity design.
+    // The flip is baked into the projection matrix, so we only have to flip
+    // manually when going from CS to NDC and back.
     positionCS.y = -positionCS.y;
 #endif
 
 // (position = positionCS) => (clipSpaceTransform = use default)
 // (position = positionVS) => (clipSpaceTransform = UNITY_MATRIX_P)
 // (position = positionWS) => (clipSpaceTransform = UNITY_MATRIX_VP)
-float2 ComputeNormalizedDeviceCoordinates(float3 position, float4x4 clipSpaceTransform = k_identity4x4)
+float4 ComputeClipSpacePosition(float3 position, float4x4 clipSpaceTransform = k_identity4x4)
-    float4 positionCS = ComputeClipSpaceCoordinates(position, clipSpaceTransform);
-
-    return positionCS.xy * (rcp(positionCS.w) * 0.5) + 0.5;
+    return mul(clipSpaceTransform, float4(position, 1.0));
-float4 ComputeClipSpacePosition(float2 positionNDC, float deviceDepth)
+// Use case examples:
+// (position = positionCS) => (clipSpaceTransform = use default)
+// (position = positionVS) => (clipSpaceTransform = UNITY_MATRIX_P)
+// (position = positionWS) => (clipSpaceTransform = UNITY_MATRIX_VP)
+float2 ComputeNormalizedDeviceCoordinates(float3 position, float4x4 clipSpaceTransform = k_identity4x4)
-    float4 positionCS = float4(positionNDC * 2.0 - 1.0, deviceDepth, 1.0);
+    float4 positionCS = ComputeClipSpacePosition(position, clipSpaceTransform);
-    // Our clip space is correct, but the NDC is flipped.
-    // Conceptually, it should be (positionNDC.y = 1.0 - positionNDC.y), but this is more efficient.
+    // Our world space, view space, screen space and NDC space are Y-up.
+    // Our clip space is flipped upside-down due to poor legacy Unity design.
+    // The flip is baked into the projection matrix, so we only have to flip
+    // manually when going from CS to NDC and back.
-    return positionCS;
+    return positionCS.xy * (rcp(positionCS.w) * 0.5) + 0.5;
 }
 
 float3 ComputeViewSpacePosition(float2 positionNDC, float deviceDepth, float4x4 invProjMatrix)

ScriptableRenderPipeline/HDRenderPipeline/HDRP/Lighting/LightLoop/LightLoopDef.hlsl

         if (cacheType == ENVCACHETYPE_TEXTURE2D)
         {
             //_Env2DCaptureVP is in capture space
-            float4 ndc = ComputeClipSpaceCoordinates(texCoord, _Env2DCaptureVP[index]);
+            float4 ndc = ComputeClipSpacePosition(texCoord, _Env2DCaptureVP[index]);
             ndc *= rcp(ndc.w);
             ndc.xy = ndc.xy * 0.5 + 0.5;