From b55576e0a4ac4f5ca8da8b61ce5856ce447d5341 Mon Sep 17 00:00:00 2001 From: Emmanuel Turquin Date: Tue, 7 Jul 2020 17:51:57 +0200 Subject: [PATCH 1/7] Fixed robustness issue with GetOddNegativeScale() in ray tracing. --- .../ShaderLibrary/SpaceTransforms.hlsl | 5 ++++- .../Raytracing/Shaders/RaytracingFragInputs.hlsl | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/com.unity.render-pipelines.core/ShaderLibrary/SpaceTransforms.hlsl b/com.unity.render-pipelines.core/ShaderLibrary/SpaceTransforms.hlsl index c7cbe86d610..fcf51f82fa9 100644 --- a/com.unity.render-pipelines.core/ShaderLibrary/SpaceTransforms.hlsl +++ b/com.unity.render-pipelines.core/ShaderLibrary/SpaceTransforms.hlsl @@ -49,7 +49,10 @@ float3 GetCameraRelativePositionWS(float3 positionWS) real GetOddNegativeScale() { - return unity_WorldTransformParams.w; + // FIXME: We should be able to just return unity_WorldTransformParams.w, but it is not + // properly set at the moment, when doing ray-tracing; once this has been fixed in cpp, + // we can revert back to the former implementation. + return unity_WorldTransformParams.w >= 0.0 ? 1.0 : -1.0; } float3 TransformObjectToWorld(float3 positionOS) diff --git a/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/Raytracing/Shaders/RaytracingFragInputs.hlsl b/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/Raytracing/Shaders/RaytracingFragInputs.hlsl index 189b8fb48a4..a1835907d3d 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/Raytracing/Shaders/RaytracingFragInputs.hlsl +++ b/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/Raytracing/Shaders/RaytracingFragInputs.hlsl @@ -10,8 +10,8 @@ void BuildFragInputsFromIntersection(IntersectionVertex currentVertex, float3 in outFragInputs.color = currentVertex.color; float3 normalWS = normalize(mul(currentVertex.normalOS, (float3x3)WorldToObject3x4())); - float4 tangentWS = float4(normalize(mul(currentVertex.tangentOS.xyz, (float3x3)WorldToObject3x4())), currentVertex.tangentOS.w); - outFragInputs.tangentToWorld = BuildTangentToWorld(tangentWS, normalWS); + float3 tangentWS = normalize(mul(currentVertex.tangentOS.xyz, (float3x3)WorldToObject3x4())); + outFragInputs.tangentToWorld = CreateTangentToWorld(normalWS, tangentWS, sign(currentVertex.tangentOS.w)); outFragInputs.isFrontFace = dot(incidentDirection, outFragInputs.tangentToWorld[2]) < 0.0f; } \ No newline at end of file From d88c21082a9c702183fc6ab63963a824554bf19c Mon Sep 17 00:00:00 2001 From: Emmanuel Turquin Date: Tue, 7 Jul 2020 17:56:37 +0200 Subject: [PATCH 2/7] Updated changelog. --- com.unity.render-pipelines.high-definition/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/com.unity.render-pipelines.high-definition/CHANGELOG.md b/com.unity.render-pipelines.high-definition/CHANGELOG.md index c15c475ca09..b0b36e6eec8 100644 --- a/com.unity.render-pipelines.high-definition/CHANGELOG.md +++ b/com.unity.render-pipelines.high-definition/CHANGELOG.md @@ -723,6 +723,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixed overused the atlas for Animated/Render Target Cookies (1259930). - Fixed sky asserts with XR multipass - Fixed for area light not updating baked light result when modifying with gizmo. +- Fixed robustness issue with GetOddNegativeScale() in ray tracing (that was impacting normal mapping). ### Changed - Improve MIP selection for decals on Transparents From 696203162d4c457f2f2fc6bbd7036c91cf4c0350 Mon Sep 17 00:00:00 2001 From: Emmanuel Turquin Date: Tue, 7 Jul 2020 19:13:12 +0200 Subject: [PATCH 3/7] Added fogbugz case id in changelog. --- com.unity.render-pipelines.high-definition/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.render-pipelines.high-definition/CHANGELOG.md b/com.unity.render-pipelines.high-definition/CHANGELOG.md index b0b36e6eec8..d1c141710f6 100644 --- a/com.unity.render-pipelines.high-definition/CHANGELOG.md +++ b/com.unity.render-pipelines.high-definition/CHANGELOG.md @@ -723,7 +723,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixed overused the atlas for Animated/Render Target Cookies (1259930). - Fixed sky asserts with XR multipass - Fixed for area light not updating baked light result when modifying with gizmo. -- Fixed robustness issue with GetOddNegativeScale() in ray tracing (that was impacting normal mapping). +- Fixed robustness issue with GetOddNegativeScale() in ray tracing, which was impacting normal mapping (1261160). ### Changed - Improve MIP selection for decals on Transparents From e95218965cb6cec1b918e4f35bf4497ba9448dd6 Mon Sep 17 00:00:00 2001 From: Emmanuel Turquin Date: Thu, 9 Jul 2020 16:27:35 +0200 Subject: [PATCH 4/7] Mofified way diffuse and SSS are mixed in path tracing. --- .../Runtime/Material/Lit/LitPathTracing.hlsl | 14 +++++------ .../PathTracing/Shaders/PathTracingBSDF.hlsl | 24 +++++++++++++++---- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/com.unity.render-pipelines.high-definition/Runtime/Material/Lit/LitPathTracing.hlsl b/com.unity.render-pipelines.high-definition/Runtime/Material/Lit/LitPathTracing.hlsl index b10668cf278..90ebe9aa393 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/Material/Lit/LitPathTracing.hlsl +++ b/com.unity.render-pipelines.high-definition/Runtime/Material/Lit/LitPathTracing.hlsl @@ -118,7 +118,7 @@ bool CreateMaterialData(PathIntersection pathIntersection, BuiltinData builtinDa // Otherwise, we just compute BSDFs as usual mtlData.subsurfaceWeightFactor = 1.0 - subsurfaceWeight; - mtlData.bsdfWeight[0] -= subsurfaceWeight; + mtlData.bsdfWeight[0] = max(mtlData.bsdfWeight[0] - subsurfaceWeight, BSDF_WEIGHT_EPSILON); mtlData.bsdfWeight /= mtlData.subsurfaceWeightFactor; sample -= subsurfaceWeight; @@ -147,7 +147,7 @@ bool SampleMaterial(MaterialData mtlData, float3 inputSample, out float3 sampleD if (!BRDF::SampleLambert(mtlData, inputSample, sampleDir, result.diffValue, result.diffPdf)) return false; - result.diffValue *= mtlData.bsdfData.ambientOcclusion * mtlData.bsdfData.subsurfaceMask * (1.0 - mtlData.bsdfData.transmittanceMask); + result.diffValue *= mtlData.bsdfData.ambientOcclusion * (1.0 - mtlData.bsdfData.transmittanceMask); result.diffPdf *= mtlData.subsurfaceWeightFactor; return true; @@ -175,7 +175,7 @@ bool SampleMaterial(MaterialData mtlData, float3 inputSample, out float3 sampleD result.specPdf += mtlData.bsdfWeight[1] * pdf; } - result.diffValue *= mtlData.bsdfData.ambientOcclusion * (1.0 - mtlData.bsdfData.subsurfaceMask) * (1.0 - mtlData.bsdfData.transmittanceMask) * (1.0 - fresnelClearCoat); + result.diffValue *= mtlData.bsdfData.ambientOcclusion * (1.0 - mtlData.bsdfData.transmittanceMask) * (1.0 - fresnelClearCoat); if (mtlData.bsdfWeight[2] > BSDF_WEIGHT_EPSILON) { @@ -196,7 +196,7 @@ bool SampleMaterial(MaterialData mtlData, float3 inputSample, out float3 sampleD if (mtlData.bsdfWeight[0] > BSDF_WEIGHT_EPSILON) { BRDF::EvaluateDiffuse(mtlData, sampleDir, result.diffValue, result.diffPdf); - result.diffValue *= mtlData.bsdfData.ambientOcclusion * (1.0 - mtlData.bsdfData.subsurfaceMask) * (1.0 - mtlData.bsdfData.transmittanceMask) * (1.0 - fresnelClearCoat); + result.diffValue *= mtlData.bsdfData.ambientOcclusion * (1.0 - mtlData.bsdfData.transmittanceMask) * (1.0 - fresnelClearCoat); result.diffPdf *= mtlData.bsdfWeight[0]; } @@ -226,7 +226,7 @@ bool SampleMaterial(MaterialData mtlData, float3 inputSample, out float3 sampleD if (mtlData.bsdfWeight[0] > BSDF_WEIGHT_EPSILON) { BRDF::EvaluateDiffuse(mtlData, sampleDir, result.diffValue, result.diffPdf); - result.diffValue *= mtlData.bsdfData.ambientOcclusion * (1.0 - mtlData.bsdfData.subsurfaceMask) * (1.0 - mtlData.bsdfData.transmittanceMask) * (1.0 - fresnelClearCoat); + result.diffValue *= mtlData.bsdfData.ambientOcclusion * (1.0 - mtlData.bsdfData.transmittanceMask) * (1.0 - fresnelClearCoat); result.diffPdf *= mtlData.bsdfWeight[0]; } } @@ -295,7 +295,7 @@ void EvaluateMaterial(MaterialData mtlData, float3 sampleDir, out MaterialResult if (mtlData.isSubsurface) { BRDF::EvaluateLambert(mtlData, sampleDir, result.diffValue, result.diffPdf); - result.diffValue *= mtlData.bsdfData.subsurfaceMask * (1.0 - mtlData.bsdfData.transmittanceMask); // AO purposedly ignored here + result.diffValue *= (1.0 - mtlData.bsdfData.transmittanceMask); // AO purposedly ignored here result.diffPdf *= mtlData.subsurfaceWeightFactor; return; @@ -319,7 +319,7 @@ void EvaluateMaterial(MaterialData mtlData, float3 sampleDir, out MaterialResult if (mtlData.bsdfWeight[0] > BSDF_WEIGHT_EPSILON) { BRDF::EvaluateDiffuse(mtlData, sampleDir, result.diffValue, result.diffPdf); - result.diffValue *= (1.0 - mtlData.bsdfData.transmittanceMask) * (1.0 - mtlData.bsdfData.subsurfaceMask) * (1.0 - fresnelClearCoat); // AO purposedly ignored here + result.diffValue *= (1.0 - mtlData.bsdfData.transmittanceMask) * (1.0 - fresnelClearCoat); // AO purposedly ignored here result.diffPdf *= mtlData.bsdfWeight[0]; } diff --git a/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/PathTracing/Shaders/PathTracingBSDF.hlsl b/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/PathTracing/Shaders/PathTracingBSDF.hlsl index 526b983bb9f..a56b52c75bb 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/PathTracing/Shaders/PathTracingBSDF.hlsl +++ b/com.unity.render-pipelines.high-definition/Runtime/RenderPipeline/PathTracing/Shaders/PathTracingBSDF.hlsl @@ -478,7 +478,7 @@ bool RandomWalk(float3 position, float3 normal, float3 diffuseColor, float3 mean // Evaluate the length of our steps rayDesc.TMax = -log(1.0 - distSample) / sigmaT[channelIdx]; - // Sample our next sepath segment direction + // Sample our next path segment direction rayDesc.Direction = walkIdx ? SampleSphereUniform(dirSample0, dirSample1) : SampleHemisphereCosine(dirSample0, dirSample1, -normal); @@ -489,7 +489,7 @@ bool RandomWalk(float3 position, float3 normal, float3 diffuseColor, float3 mean TraceRay(_RaytracingAccelerationStructure, RAY_FLAG_FORCE_OPAQUE | RAY_FLAG_CULL_FRONT_FACING_TRIANGLES, RAYTRACINGRENDERERFLAG_PATH_TRACING, 0, 1, 1, rayDesc, intersection); - // Define if we did a hit + // Check if we hit something hit = intersection.t > 0.0; // How much did the ray travel? @@ -513,10 +513,24 @@ bool RandomWalk(float3 position, float3 normal, float3 diffuseColor, float3 mean while (!hit && walkIdx < MAX_WALK_STEPS); // Set the exit intersection position and normal - result.exitPosition = rayDesc.Origin; - result.exitNormal = intersection.value; + if (!hit) + { + result.exitPosition = position; + result.exitNormal = normal; + result.throughput = diffuseColor; + + // By not returning false here, we default to a diffuse BRDF when an intersection is not found; + // this is physically wrong, but may prove more convenient for a user, as results will look + // like diffuse instead of getting slightly darker when the mean free path becomes shorter. + //return false; + } + else + { + result.exitPosition = rayDesc.Origin; + result.exitNormal = intersection.value; + } - return hit; + return true; } } // namespace SSS From 05f036bca07dea7e4bea853e234c341246e99828 Mon Sep 17 00:00:00 2001 From: Emmanuel Turquin Date: Thu, 9 Jul 2020 19:53:23 +0200 Subject: [PATCH 5/7] Fixed issue with spec balance. --- .../Runtime/Material/Lit/LitPathTracing.hlsl | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/com.unity.render-pipelines.high-definition/Runtime/Material/Lit/LitPathTracing.hlsl b/com.unity.render-pipelines.high-definition/Runtime/Material/Lit/LitPathTracing.hlsl index 90ebe9aa393..3ec50c5253f 100644 --- a/com.unity.render-pipelines.high-definition/Runtime/Material/Lit/LitPathTracing.hlsl +++ b/com.unity.render-pipelines.high-definition/Runtime/Material/Lit/LitPathTracing.hlsl @@ -148,7 +148,6 @@ bool SampleMaterial(MaterialData mtlData, float3 inputSample, out float3 sampleD return false; result.diffValue *= mtlData.bsdfData.ambientOcclusion * (1.0 - mtlData.bsdfData.transmittanceMask); - result.diffPdf *= mtlData.subsurfaceWeightFactor; return true; } @@ -248,8 +247,8 @@ bool SampleMaterial(MaterialData mtlData, float3 inputSample, out float3 sampleD #endif #ifdef _MATERIAL_FEATURE_SUBSURFACE_SCATTERING - result.diffPdf *= mtlData.subsurfaceWeightFactor; - result.specPdf *= mtlData.subsurfaceWeightFactor; + // We compensate for the fact that there is no spec when computing SSS + result.specValue /= mtlData.subsurfaceWeightFactor; #endif } else // Below @@ -295,8 +294,7 @@ void EvaluateMaterial(MaterialData mtlData, float3 sampleDir, out MaterialResult if (mtlData.isSubsurface) { BRDF::EvaluateLambert(mtlData, sampleDir, result.diffValue, result.diffPdf); - result.diffValue *= (1.0 - mtlData.bsdfData.transmittanceMask); // AO purposedly ignored here - result.diffPdf *= mtlData.subsurfaceWeightFactor; + result.diffValue *= 1.0 - mtlData.bsdfData.transmittanceMask; // AO purposedly ignored here return; } @@ -331,8 +329,8 @@ void EvaluateMaterial(MaterialData mtlData, float3 sampleDir, out MaterialResult } #ifdef _MATERIAL_FEATURE_SUBSURFACE_SCATTERING - result.diffPdf *= mtlData.subsurfaceWeightFactor; - result.specPdf *= mtlData.subsurfaceWeightFactor; + // We compensate for the fact that there is no spec when computing SSS + result.specValue /= mtlData.subsurfaceWeightFactor; #endif } } From ded510b97f944b3e0bfe2c818f786f6636583e91 Mon Sep 17 00:00:00 2001 From: Emmanuel Turquin Date: Thu, 9 Jul 2020 19:57:17 +0200 Subject: [PATCH 6/7] Updated Changelog. --- com.unity.render-pipelines.high-definition/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/com.unity.render-pipelines.high-definition/CHANGELOG.md b/com.unity.render-pipelines.high-definition/CHANGELOG.md index d1c141710f6..47979c31bd4 100644 --- a/com.unity.render-pipelines.high-definition/CHANGELOG.md +++ b/com.unity.render-pipelines.high-definition/CHANGELOG.md @@ -724,6 +724,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixed sky asserts with XR multipass - Fixed for area light not updating baked light result when modifying with gizmo. - Fixed robustness issue with GetOddNegativeScale() in ray tracing, which was impacting normal mapping (1261160). +- Fixed path-traced subsurface scattering mixing with diffuse and specular BRDFs (1250601). ### Changed - Improve MIP selection for decals on Transparents From c12e0e3a3d526c8f7cd9ef5a8ea704a83ae7e5d8 Mon Sep 17 00:00:00 2001 From: Emmanuel Turquin Date: Fri, 10 Jul 2020 12:24:16 +0200 Subject: [PATCH 7/7] Updated ref image for test 5007. --- .../Direct3D12/None/5007_PathTracing_Materials_SG_Lit.png | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/TestProjects/HDRP_DXR_Tests/Assets/ReferenceImages/Linear/WindowsEditor/Direct3D12/None/5007_PathTracing_Materials_SG_Lit.png b/TestProjects/HDRP_DXR_Tests/Assets/ReferenceImages/Linear/WindowsEditor/Direct3D12/None/5007_PathTracing_Materials_SG_Lit.png index 3ab8e3d69dc..2f222184692 100644 --- a/TestProjects/HDRP_DXR_Tests/Assets/ReferenceImages/Linear/WindowsEditor/Direct3D12/None/5007_PathTracing_Materials_SG_Lit.png +++ b/TestProjects/HDRP_DXR_Tests/Assets/ReferenceImages/Linear/WindowsEditor/Direct3D12/None/5007_PathTracing_Materials_SG_Lit.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:c5b765dace74f6bdd1005f95800e3f7680f3c8c3b6f50936da14f2c5d41f146d -size 509269 +oid sha256:111ddeed9399678e4d5816f4a49ff792d79b121b9e6abe7f17e47c37e3e8c0d1 +size 512669