From 3f1af19ae353d5094c45840d13947807357cc664 Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Thu, 21 Apr 2022 00:02:03 -0700 Subject: [PATCH 1/4] Added new screenshot tests that focus on exposing depth-test artifacts in parallax surfaces. I added a new setting to the Parallax sample that allows turning off all lights, so the surface is rendered black. This allows us to use a much tighter threshold on the screenshot comparison, since an entirely black unlit surface will not exhibit the sensitive platform differences we are used to seeing on parallax materials. This should ensure any depth-test calculation noise, which show up as gray firefly pixels, will cause the test threshold to fail. I included many screenshots because the noise seems to be platform - and/or driver-dependent, so having more screenshots increases the chances of detecting failures. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../ParallaxMappingExampleComponent.cpp | 41 +++++++----- .../Source/ParallaxMappingExampleComponent.h | 9 ++- .../ParallaxDepthArtifacts/screenshot_1.png | 3 + .../ParallaxDepthArtifacts/screenshot_2.png | 3 + .../ParallaxDepthArtifacts/screenshot_3.png | 3 + .../ParallaxDepthArtifacts/screenshot_4.png | 3 + .../ParallaxDepthArtifacts/screenshot_5.png | 3 + .../ParallaxDepthArtifacts/screenshot_6.png | 3 + Scripts/ParallaxDepthArtifacts.bv.lua | 65 +++++++++++++++++++ 9 files changed, 117 insertions(+), 16 deletions(-) create mode 100644 Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_1.png create mode 100644 Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_2.png create mode 100644 Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_3.png create mode 100644 Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_4.png create mode 100644 Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_5.png create mode 100644 Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_6.png create mode 100644 Scripts/ParallaxDepthArtifacts.bv.lua diff --git a/Gem/Code/Source/ParallaxMappingExampleComponent.cpp b/Gem/Code/Source/ParallaxMappingExampleComponent.cpp index 1d382e1c..ffd33123 100644 --- a/Gem/Code/Source/ParallaxMappingExampleComponent.cpp +++ b/Gem/Code/Source/ParallaxMappingExampleComponent.cpp @@ -205,24 +205,34 @@ namespace AtomSampleViewer auto transform = AZ::Transform::CreateLookAt( location, AZ::Vector3::CreateZero()); + + using Lux = AZ::Render::PhotometricColor; + using Candela = AZ::Render::PhotometricColor; - if (m_lightType) - { - AZ::Render::PhotometricColor directionalLightColor(AZ::Color::CreateZero()); - AZ::Render::PhotometricColor diskLightColor(AZ::Color::CreateOne() * 500.f); - m_directionalLightFeatureProcessor->SetRgbIntensity(m_directionalLightHandle, directionalLightColor); - m_diskLightFeatureProcessor->SetRgbIntensity(m_diskLightHandle, diskLightColor); - } - else + Lux directionalLightColor{AZ::Color::CreateZero()}; + Candela diskLightColor{AZ::Color::CreateZero()}; + + switch (static_cast(m_lightType)) { - AZ::Render::PhotometricColor directionalLightColor(AZ::Color::CreateOne() * 5.f); - AZ::Render::PhotometricColor diskLightColor(AZ::Color::CreateZero()); - m_directionalLightFeatureProcessor->SetRgbIntensity(m_directionalLightHandle, directionalLightColor); - m_diskLightFeatureProcessor->SetRgbIntensity(m_diskLightHandle, diskLightColor); + case LightSelection::Directional: + directionalLightColor = Lux{AZ::Color::CreateOne() * 5.f}; + break; + case LightSelection::Spot: + diskLightColor = Candela{AZ::Color::CreateOne() * 500.f}; + break; + case LightSelection::None: + // Keep initial 0 values + break; + default: + AZ_Assert(false, "Unhandled case"); + break; } - + + m_diskLightFeatureProcessor->SetRgbIntensity(m_diskLightHandle, diskLightColor); m_diskLightFeatureProcessor->SetPosition(m_diskLightHandle, location); m_diskLightFeatureProcessor->SetDirection(m_diskLightHandle, transform.GetBasis(1)); + + m_directionalLightFeatureProcessor->SetRgbIntensity(m_directionalLightHandle, directionalLightColor); m_directionalLightFeatureProcessor->SetDirection(m_directionalLightHandle, transform.GetBasis(1)); // Camera Configuration @@ -305,8 +315,9 @@ namespace AtomSampleViewer ImGui::Text("Lighting"); ImGui::Indent(); { - ScriptableImGui::RadioButton("Directional Light", &m_lightType, 0); - ScriptableImGui::RadioButton("Spot Light", &m_lightType, 1); + ScriptableImGui::RadioButton("No Light", &m_lightType, 0); + ScriptableImGui::RadioButton("Directional Light", &m_lightType, 1); + ScriptableImGui::RadioButton("Spot Light", &m_lightType, 2); ScriptableImGui::Checkbox("Auto Rotation", &m_lightAutoRotate); ScriptableImGui::SliderAngle("Direction", &m_lightRotationAngle, 0, 360); } diff --git a/Gem/Code/Source/ParallaxMappingExampleComponent.h b/Gem/Code/Source/ParallaxMappingExampleComponent.h index 0bdd892e..08e6500f 100644 --- a/Gem/Code/Source/ParallaxMappingExampleComponent.h +++ b/Gem/Code/Source/ParallaxMappingExampleComponent.h @@ -66,7 +66,14 @@ namespace AtomSampleViewer float m_lightRotationAngle = 0.f; // in radian bool m_lightAutoRotate = true; - int m_lightType = 0; // 0: diectionalLight, 1: diskLight + + enum class LightSelection : int + { + None, + Directional, + Spot + }; + int m_lightType = static_cast(LightSelection::Directional); //Assets AZ::Data::Asset m_planeAsset; diff --git a/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_1.png b/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_1.png new file mode 100644 index 00000000..38bd18e5 --- /dev/null +++ b/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_1.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:649410344b69ce478d45939b6c6762d1aa4ff6df79c7ef4b617d40eee2100bf9 +size 2687 diff --git a/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_2.png b/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_2.png new file mode 100644 index 00000000..53a54b8c --- /dev/null +++ b/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_2.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:fc30248a3e98dba397eedb36d153cf80c16d3683cff49a2f9d2f1e15cfde2f6f +size 2704 diff --git a/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_3.png b/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_3.png new file mode 100644 index 00000000..8f5a9168 --- /dev/null +++ b/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_3.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:616cac57e0c140585775c8aa34ee856a56e48ce15ebc98d40e23670fb2ed4a0e +size 2676 diff --git a/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_4.png b/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_4.png new file mode 100644 index 00000000..cc1a4b09 --- /dev/null +++ b/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_4.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:2d45bd0bfa87ad9a9959556c5bd04e282ef4beb2036dda0a19ab59de2d051444 +size 2657 diff --git a/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_5.png b/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_5.png new file mode 100644 index 00000000..a8ae7157 --- /dev/null +++ b/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_5.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:1dd397a5191ac14b4adfc5919e62a5954dbfca7020fde2629d432d3c6c60838b +size 2631 diff --git a/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_6.png b/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_6.png new file mode 100644 index 00000000..eb41283c --- /dev/null +++ b/Scripts/ExpectedScreenshots/ParallaxDepthArtifacts/screenshot_6.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:71208d13cb03bbca75c0c3a0f3b08c170b4c569354542790f45239187bf6e3cc +size 2729 diff --git a/Scripts/ParallaxDepthArtifacts.bv.lua b/Scripts/ParallaxDepthArtifacts.bv.lua new file mode 100644 index 00000000..07024746 --- /dev/null +++ b/Scripts/ParallaxDepthArtifacts.bv.lua @@ -0,0 +1,65 @@ +---------------------------------------------------------------------------------------------------- +-- +-- Copyright (c) Contributors to the Open 3D Engine Project. +-- For complete copyright and license terms please see the LICENSE at the root of this distribution. +-- +-- SPDX-License-Identifier: Apache-2.0 OR MIT +-- +-- +-- +---------------------------------------------------------------------------------------------------- + +g_screenshotOutputFolder = ResolvePath('@user@/Scripts/Screenshots/ParallaxDepthArtifacts/') +Print('Saving screenshots to ' .. NormalizePath(g_screenshotOutputFolder)) + +OpenSample('Features/Parallax') +ResizeViewport(512, 512) + +-- Several times there have bugs that appeared, related to inconsistent depth calculations causing unwanted clipping of pixels on parallax surfaces. +-- We attempt to detect this by putting the camera at several angles that have been observed to reveal these artifacts in the past. +-- All lights are turned off to avoid the sensitive platform differences we are used to seeing on parallax materials, which allows us to use a much +-- tighter tolerance level than other parallax test cases, so that even small amounts of artifacts will be detected and fail the test. These +-- artifacts were showing up as gray firefly pixels on the otherwise black parallax surface. +-- Many camera angles are used because the noise seems to be platform- and/or driver-dependent, so having more angles increases the chances of detecting failures. + +SelectImageComparisonToleranceLevel("Level B") + +SetImguiValue('Lighting/Auto Rotation', false) +SetImguiValue('Lighting/Direction', DegToRad(110)) +SetImguiValue('Parallax Setting/Heightmap Scale', 0.1) +SetImguiValue('Parallax Setting/Enable Pdo', true) +SetImguiValue('Lighting/No Light', true) + +ArcBallCameraController_SetDistance(3.000000) + +ArcBallCameraController_SetHeading(DegToRad(-38.356312)) +ArcBallCameraController_SetPitch(DegToRad(-2.705635)) +IdleFrames(1) +CaptureScreenshot(g_screenshotOutputFolder .. '/screenshot_1.png') + +ArcBallCameraController_SetHeading(DegToRad(-66.861877)) +ArcBallCameraController_SetPitch(DegToRad(-4.933800)) +IdleFrames(1) +CaptureScreenshot(g_screenshotOutputFolder .. '/screenshot_2.png') + +ArcBallCameraController_SetHeading(DegToRad(30.230936)) +ArcBallCameraController_SetPitch(DegToRad(-3.819724)) +IdleFrames(1) +CaptureScreenshot(g_screenshotOutputFolder .. '/screenshot_3.png') + +ArcBallCameraController_SetHeading(DegToRad(-140.709763)) +ArcBallCameraController_SetPitch(DegToRad(-3.501410)) +IdleFrames(1) +CaptureScreenshot(g_screenshotOutputFolder .. '/screenshot_4.png') + +ArcBallCameraController_SetHeading(DegToRad(135.264740)) +ArcBallCameraController_SetPitch(DegToRad(-2.387333)) +IdleFrames(1) +CaptureScreenshot(g_screenshotOutputFolder .. '/screenshot_5.png') + +ArcBallCameraController_SetHeading(DegToRad(20.355005)) +ArcBallCameraController_SetPitch(DegToRad(-4.456343)) +IdleFrames(1) +CaptureScreenshot(g_screenshotOutputFolder .. '/screenshot_6.png') + +OpenSample(nil) \ No newline at end of file From b60969ab9314e578b719d4e7fd071fc4c80e4456 Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Thu, 21 Apr 2022 09:55:39 -0700 Subject: [PATCH 2/4] Added the new ParallaxDepthArtifacts test script to the full test suite. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- Scripts/_FullTestSuite_.bv.lua | 1 + 1 file changed, 1 insertion(+) diff --git a/Scripts/_FullTestSuite_.bv.lua b/Scripts/_FullTestSuite_.bv.lua index c53d4062..3584ef68 100644 --- a/Scripts/_FullTestSuite_.bv.lua +++ b/Scripts/_FullTestSuite_.bv.lua @@ -55,6 +55,7 @@ tests= { RunScriptWrapper('scripts/transparenttest.bv.luac'), RunScriptWrapper('scripts/streamingimagetest.bv.luac'), RunScriptWrapper('scripts/parallaxtest.bv.luac'), + RunScriptWrapper('scripts/parallaxdepthartifacts.bv.luac'), RunScriptWrapper('scripts/checkerboardtest.bv.luac'), RunScriptWrapper('scripts/scenereloadsoaktest.bv.luac'), RunScriptWrapper('scripts/diffusegitest.bv.luac'), From 3494fe1ebb359c3c06b764be1b047e6036b84d55 Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Thu, 21 Apr 2022 10:08:11 -0700 Subject: [PATCH 3/4] Made sure any IBL is cleared when closing a sample. There were some cases where the IBL from one sample could persist when opening another sample, for example when going directly from the Mesh sample to the Parallax sample. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- Gem/Code/Source/CommonSampleComponentBase.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Gem/Code/Source/CommonSampleComponentBase.cpp b/Gem/Code/Source/CommonSampleComponentBase.cpp index 7322b687..4c9dcd3f 100644 --- a/Gem/Code/Source/CommonSampleComponentBase.cpp +++ b/Gem/Code/Source/CommonSampleComponentBase.cpp @@ -121,6 +121,7 @@ namespace AtomSampleViewer { AZ::Render::SkyBoxFeatureProcessorInterface* skyboxFeatureProcessor = AZ::RPI::Scene::GetFeatureProcessorForEntityContextId(m_entityContextId); AZ::Render::DirectionalLightFeatureProcessorInterface* directionalLightFeatureProcessor = AZ::RPI::Scene::GetFeatureProcessorForEntityContextId(m_entityContextId); + AZ::Render::ImageBasedLightFeatureProcessorInterface* iblFeatureProcessor = AZ::RPI::Scene::GetFeatureProcessorForEntityContextId(m_entityContextId); for (AZ::Render::DirectionalLightFeatureProcessorInterface::LightHandle& handle : m_lightHandles) { @@ -137,6 +138,8 @@ namespace AtomSampleViewer skyboxFeatureProcessor->Enable(false); + iblFeatureProcessor->Reset(); + AZ::TransformNotificationBus::MultiHandler::BusDisconnect(); AZ::EntityBus::MultiHandler::BusDisconnect(); } From 1c8e8da6d7868de97327b3963b0b8d9bfbcd9c22 Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Mon, 25 Apr 2022 16:57:00 -0700 Subject: [PATCH 4/4] Fixed code comment Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- Scripts/ParallaxDepthArtifacts.bv.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Scripts/ParallaxDepthArtifacts.bv.lua b/Scripts/ParallaxDepthArtifacts.bv.lua index 07024746..fb9dd98f 100644 --- a/Scripts/ParallaxDepthArtifacts.bv.lua +++ b/Scripts/ParallaxDepthArtifacts.bv.lua @@ -15,7 +15,7 @@ Print('Saving screenshots to ' .. NormalizePath(g_screenshotOutputFolder)) OpenSample('Features/Parallax') ResizeViewport(512, 512) --- Several times there have bugs that appeared, related to inconsistent depth calculations causing unwanted clipping of pixels on parallax surfaces. +-- There have been several bugs related to inconsistent depth calculations causing unwanted clipping of pixels on parallax surfaces. -- We attempt to detect this by putting the camera at several angles that have been observed to reveal these artifacts in the past. -- All lights are turned off to avoid the sensitive platform differences we are used to seeing on parallax materials, which allows us to use a much -- tighter tolerance level than other parallax test cases, so that even small amounts of artifacts will be detected and fail the test. These