From 72f808876c9bdaa4127edb2aae4b73e746858baa Mon Sep 17 00:00:00 2001 From: antonmic <56370189+antonmic@users.noreply.github.com> Date: Wed, 28 Jul 2021 16:35:51 -0700 Subject: [PATCH 1/7] Improvements to pass statistics and SRG debugability Signed-off-by: antonmic <56370189+antonmic@users.noreply.github.com> --- .../Atom/RHI.Reflect/ConstantsLayout.h | 5 +- .../Atom/RHI.Reflect/NameIdReflectionMap.h | 9 +++ .../RHI/Code/Include/Atom/RHI/ConstantsData.h | 8 +++ .../Atom/RHI/ShaderResourceGroupData.h | 3 + .../Atom/RHI/ShaderResourceGroupDebug.h | 21 ++++++ .../Source/RHI.Reflect/ConstantsLayout.cpp | 35 ++++++---- .../RHI/Code/Source/RHI/ConstantsData.cpp | 66 +++++++++++++++++++ .../Source/RHI/ShaderResourceGroupData.cpp | 5 ++ .../Source/RHI/ShaderResourceGroupDebug.cpp | 59 +++++++++++++++++ .../Atom/RHI/Code/atom_rhi_public_files.cmake | 2 + .../Include/Atom/RPI.Public/Pass/PassSystem.h | 15 ++++- .../RPI.Public/Pass/PassSystemInterface.h | 41 ++++++++---- .../Include/Atom/RPI.Public/Pass/RasterPass.h | 3 + .../Source/RPI.Public/Pass/PassSystem.cpp | 24 +++++++ .../Source/RPI.Public/Pass/RasterPass.cpp | 7 ++ .../Source/RPI.Public/Pass/RenderPass.cpp | 2 + ...AtomViewportDisplayInfoSystemComponent.cpp | 25 +++---- 17 files changed, 289 insertions(+), 41 deletions(-) create mode 100644 Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h create mode 100644 Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/ConstantsLayout.h b/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/ConstantsLayout.h index 09415f1bc2..003a7dcd23 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/ConstantsLayout.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/ConstantsLayout.h @@ -81,6 +81,10 @@ namespace AZ //! If validation is disabled, true is always returned. bool ValidateAccess(ShaderInputConstantIndex inputIndex) const; + //! Prints to the console the shader input names specified by input list of indices + //! Will ignore any indices outside of the inputs array bounds + void DebugPrintNames(AZStd::array_view constantList) const; + protected: ConstantsLayout() = default; @@ -94,7 +98,6 @@ namespace AZ AZStd::vector m_inputs; IdReflectionMapForConstants m_idReflection; - AZStd::vector m_intervals; uint32_t m_sizeInBytes = 0; HashValue64 m_hash = InvalidHash; }; diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/NameIdReflectionMap.h b/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/NameIdReflectionMap.h index 7756e7d290..b6042ab34c 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/NameIdReflectionMap.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI.Reflect/NameIdReflectionMap.h @@ -68,6 +68,9 @@ namespace AZ /// Return the number of entries size_t Size() const; + // Returns true if size is zero + bool IsEmpty() const; + class NameIdReflectionMapSerializationEvents : public SerializeContext::IEventHandler { @@ -168,6 +171,12 @@ namespace AZ return m_reflectionMap.size(); } + template + bool NameIdReflectionMap::IsEmpty() const + { + return Size() == 0; + } + template void NameIdReflectionMap::Sort() { diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/ConstantsData.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/ConstantsData.h index 5fd10212d4..27bd418b8a 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/ConstantsData.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/ConstantsData.h @@ -84,6 +84,14 @@ namespace AZ //! Returns the constants layout. const ConstantsLayout* GetLayout() const; + //! Returns whether other constant data and this have the same value at the specified shader input index + bool ConstantIsEqual(const ConstantsData& other, ShaderInputConstantIndex inputIndex) const; + + //! Performs a diff between this and input constant data and returns a list of all the shader input indices + //! for which the constants are not the same between the two. If one of the two has more constants than the + //! other, these additional constants will be added to the end of the returned list. + AZStd::vector GetIndicesOfDifferingConstants(const ConstantsData& other) const; + private: enum class ValidateConstantAccessExpect : uint32_t { diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupData.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupData.h index a145ba7ece..17ed3a64f1 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupData.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupData.h @@ -172,6 +172,9 @@ namespace AZ //! Different platforms might follow different packing rules for the internally-managed SRG constant buffer. AZStd::array_view GetConstantData() const; + //! Returns the underlying ConstantsData struct + const ConstantsData& GetConstantsData() const; + //! Returns the shader resource layout for this group. const ShaderResourceGroupLayout* GetLayout() const; diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h new file mode 100644 index 0000000000..b81f9e0c0b --- /dev/null +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h @@ -0,0 +1,21 @@ +/* + * 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 + * + */ +#pragma once + +namespace AZ +{ + namespace RHI + { + class ConstantsData; + struct DrawItem; + class ShaderResourceGroup; + + void PrintConstantDataDiff(const ShaderResourceGroup& shaderResourceGroup, ConstantsData& referenceData, bool updateReferenceData = false); + void PrintConstantDataDiff(const DrawItem& drawItem, ConstantsData& referenceData, u32 srgBindingSlot, bool updateReferenceData = false); + + } +} diff --git a/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp b/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp index 3f9d9d9d63..fa85fbd884 100644 --- a/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp @@ -17,10 +17,9 @@ namespace AZ if (SerializeContext* serializeContext = azrtti_cast(context)) { serializeContext->Class() - ->Version(0) + ->Version(1) // Version 1: Adding debug helper functions to Shader Resource Groups ->Field("m_inputs", &ConstantsLayout::m_inputs) ->Field("m_idReflection", &ConstantsLayout::m_idReflection) - ->Field("m_intervals", &ConstantsLayout::m_intervals) ->Field("m_sizeInBytes", &ConstantsLayout::m_sizeInBytes) ->Field("m_hash", &ConstantsLayout::m_hash); } @@ -42,7 +41,6 @@ namespace AZ { m_inputs.clear(); m_idReflection.Clear(); - m_intervals.clear(); m_sizeInBytes = 0; m_hash = InvalidHash; } @@ -66,11 +64,8 @@ namespace AZ return false; } - // The constant data size is the maximum offset + size from the start of the struct. - constantDataSize = AZStd::max(constantDataSize, constantDescriptor.m_constantByteOffset + constantDescriptor.m_constantByteCount); - - // Add the [min, max) interval for the inline constant. - m_intervals.emplace_back(constantDescriptor.m_constantByteOffset, constantDescriptor.m_constantByteOffset + constantDescriptor.m_constantByteCount); + uint32_t end = constantDescriptor.m_constantByteOffset + constantDescriptor.m_constantByteCount; + constantDataSize = AZStd::max(constantDataSize, end); ++constantInputIndex; m_hash = TypeHash64(constantDescriptor.GetHash(), m_hash); @@ -99,7 +94,10 @@ namespace AZ Interval ConstantsLayout::GetInterval(ShaderInputConstantIndex inputIndex) const { - return m_intervals[inputIndex.GetIndex()]; + const ShaderInputConstantDescriptor& desc = GetShaderInput(inputIndex); + uint32_t start = desc.m_constantByteOffset; + uint32_t end = start + desc.m_constantByteCount; + return Interval(start, end); } const ShaderInputConstantDescriptor& ConstantsLayout::GetShaderInput(ShaderInputConstantIndex inputIndex) const @@ -138,8 +136,8 @@ namespace AZ { if (!m_sizeInBytes) { - AZ_Assert(m_intervals.empty(), "Constants size is not valid."); - return m_intervals.empty(); + AZ_Assert(m_idReflection.IsEmpty(), "Constants size is not valid."); + return m_idReflection.IsEmpty(); } } @@ -148,5 +146,20 @@ namespace AZ return true; } + + void ConstantsLayout::DebugPrintNames(AZStd::array_view constantList) const + { + AZStd::string output; + for (const ShaderInputConstantIndex& constandIdx : constantList) + { + if (constandIdx.GetIndex() < m_inputs.size()) + { + output += m_inputs[constandIdx.GetIndex()].m_name.GetCStr(); + output += " - "; + } + } + AZ_Printf("RHI", output.c_str()); + } + } } diff --git a/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp b/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp index e00896d44e..29d451b4d5 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp @@ -404,5 +404,71 @@ namespace AZ AZ_Assert(m_layout, "Constants layout is null"); return m_layout.get(); } + + bool ConstantsData::ConstantIsEqual(const ConstantsData& other, ShaderInputConstantIndex inputIndex) const + { + AZStd::array_view myConstans = GetConstantRaw(inputIndex); + AZStd::array_view otherConstans = other.GetConstantRaw(inputIndex); + + // If they point to the same data, they are equal + if (myConstans == otherConstans) + { + return true; + } + + // If they point to data of different size, they are not equal + if (myConstans.size() != otherConstans.size()) + { + return false; + } + + // If they point to differing data of same size, compare the data + // Note: due to small size of data this loop will be faster than a mem compare + for(uint32_t i = 0; i < myConstans.size(); ++i) + { + if (myConstans[i] != otherConstans[i]) + { + return false; + } + } + + // Arrays point to different locations in memory but all bytes match, return true + return true; + } + + AZStd::vector ConstantsData::GetIndicesOfDifferingConstants(const ConstantsData& other) const + { + AZStd::vector differingIndices; + + if (m_layout == nullptr || other.m_layout == nullptr) + { + return differingIndices; + } + + AZStd::array_view myShaderInputs = m_layout->GetShaderInputList(); + AZStd::array_view otherShaderInputs = other.m_layout->GetShaderInputList(); + + size_t minSize = AZStd::min(myShaderInputs.size(), otherShaderInputs.size()); + size_t maxSize = AZStd::max(myShaderInputs.size(), otherShaderInputs.size()); + + for (size_t idx = 0; idx < minSize; ++idx) + { + const ShaderInputConstantIndex inputIndex(idx); + if (!ConstantIsEqual(other, inputIndex)) + { + differingIndices.push_back(inputIndex); + } + } + + // If sizes are different, add difference at the end + for (size_t idx = minSize; idx < maxSize; ++idx) + { + const ShaderInputConstantIndex inputIndex(idx); + differingIndices.push_back(inputIndex); + } + + return differingIndices; + } + } } diff --git a/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupData.cpp b/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupData.cpp index f2b57bf1e5..65499fb57d 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupData.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupData.cpp @@ -334,5 +334,10 @@ namespace AZ return m_constantsData.GetConstantData(); } + const ConstantsData& ShaderResourceGroupData::GetConstantsData() const + { + return m_constantsData; + } + } // namespace RHI } // namespace AZ diff --git a/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp b/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp new file mode 100644 index 0000000000..4bd8d0aab8 --- /dev/null +++ b/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp @@ -0,0 +1,59 @@ +/* + * 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 + * + */ + +#include +#include +#include +#include + +namespace AZ +{ + namespace RHI + { + + void PrintConstantDataDiff(const ShaderResourceGroup& shaderResourceGroup, ConstantsData& referenceData, bool updateReferenceData) + { + const RHI::ConstantsData& currentData = shaderResourceGroup.GetData().GetConstantsData(); + + AZStd::vector differingIndices = currentData.GetIndicesOfDifferingConstants(referenceData); + + if (differingIndices.size() > 0) + { + AZ_Printf("RHI", "Detected different SRG values for the following fields:\n"); + if (currentData.GetLayout()) + { + currentData.GetLayout()->DebugPrintNames(differingIndices); + } + } + + if (updateReferenceData) + { + referenceData = currentData; + } + } + + void PrintConstantDataDiff(const DrawItem& drawItem, ConstantsData& referenceData, u32 srgBindingSlot, bool updateReferenceData) + { + s32 srgIndex = -1; + for (u32 i = 0; i < drawItem.m_shaderResourceGroupCount; ++i) + { + if (drawItem.m_shaderResourceGroups[i]->GetBindingSlot() == srgBindingSlot) + { + srgIndex = i; + break; + } + } + + if (srgIndex != -1) + { + const ShaderResourceGroup& srg = *drawItem.m_shaderResourceGroups[srgIndex]; + PrintConstantDataDiff(srg, referenceData, updateReferenceData); + } + } + + } +} diff --git a/Gems/Atom/RHI/Code/atom_rhi_public_files.cmake b/Gems/Atom/RHI/Code/atom_rhi_public_files.cmake index b489cbbbb4..2bf2acd892 100644 --- a/Gems/Atom/RHI/Code/atom_rhi_public_files.cmake +++ b/Gems/Atom/RHI/Code/atom_rhi_public_files.cmake @@ -156,10 +156,12 @@ set(FILES Source/RHI/ScopeAttachment.cpp Include/Atom/RHI/ShaderResourceGroup.h Include/Atom/RHI/ShaderResourceGroupData.h + Include/Atom/RHI/ShaderResourceGroupDebug.h Include/Atom/RHI/ShaderResourceGroupInvalidateRegistry.h Include/Atom/RHI/ShaderResourceGroupPool.h Source/RHI/ShaderResourceGroup.cpp Source/RHI/ShaderResourceGroupData.cpp + Source/RHI/ShaderResourceGroupDebug.cpp Source/RHI/ShaderResourceGroupInvalidateRegistry.cpp Source/RHI/ShaderResourceGroupPool.cpp Include/Atom/RHI/MemoryStatisticsBuilder.h diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystem.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystem.h index de57e2e3f6..5998798ae8 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystem.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystem.h @@ -74,6 +74,12 @@ namespace AZ const AZ::Name& GetTargetedPassDebuggingName() const override; void ConnectEvent(OnReadyLoadTemplatesEvent::Handler& handler) override; PassSystemState GetState() const override; + SwapChainPass* FindSwapChainPass(AzFramework::NativeWindowHandle windowHandle) const override; + + // PassSystemInterface statistics related functions + void IncrementFrameDrawItemCount(u32 numDrawItems) override; + void IncrementFrameRenderPassCount() override; + PassSystemFrameStatistics GetFrameStatistics() override; // PassSystemInterface factory related functions... void AddPassCreator(Name className, PassCreator createFunction) override; @@ -92,7 +98,6 @@ namespace AZ void RegisterPass(Pass* pass) override; void UnregisterPass(Pass* pass) override; AZStd::vector FindPasses(const PassFilter& passFilter) const override; - SwapChainPass* FindSwapChainPass(AzFramework::NativeWindowHandle windowHandle) const override; private: // Returns the root of the pass tree hierarchy @@ -115,6 +120,9 @@ namespace AZ void QueueForRemoval(Pass* pass) override; void QueueForInitialization(Pass* pass) override; + // Resets the frame statistic counters + void ResetFrameStatistics(); + // Lists for queuing passes for various function calls // Name of the list reflects the pass function it will call AZStd::vector< Ptr > m_buildPassList; @@ -140,13 +148,16 @@ namespace AZ AZ::Name m_targetedPassDebugName; // Counts the number of passes - int32_t m_passCounter = 0; + u32 m_passCounter = 0; // Events OnReadyLoadTemplatesEvent m_loadTemplatesEvent; // Used to track what phase of execution the pass system is in PassSystemState m_state = PassSystemState::Unitialized; + + // Counters used to gather statistics about the frame + PassSystemFrameStatistics m_frameStatistics; }; } // namespace RPI } // namespace AZ diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystemInterface.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystemInterface.h index cc66a7c228..246e43a2f7 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystemInterface.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/PassSystemInterface.h @@ -66,6 +66,14 @@ namespace AZ FrameEnd, }; + //! Frame counters used for collecting statistics + struct PassSystemFrameStatistics + { + u32 m_numRenderPassesExecuted = 0; + u32 m_totalDrawItemsRendered = 0; + u32 m_maxDrawItemsRenderedInAPass = 0; + }; + class PassSystemInterface { friend class Pass; @@ -115,6 +123,27 @@ namespace AZ virtual void SetTargetedPassDebuggingName(const AZ::Name& targetPassName) = 0; virtual const AZ::Name& GetTargetedPassDebuggingName() const = 0; + //! Find the SwapChainPass associated with window Handle + virtual SwapChainPass* FindSwapChainPass(AzFramework::NativeWindowHandle windowHandle) const = 0; + + using OnReadyLoadTemplatesEvent = AZ::Event<>; + //! Connect a handler to listen to the event that the pass system is ready to load pass templates + //! The event is triggered when pass system is initialized and asset system is ready. + //! The handler can add new pass templates or load pass template mappings from assets + virtual void ConnectEvent(OnReadyLoadTemplatesEvent::Handler& handler) = 0; + + virtual PassSystemState GetState() const = 0; + + // Passes call this function to notify the pass system that they are drawing X draw items this frame + // Used for Pass System statistics + virtual void IncrementFrameDrawItemCount(u32 numDrawItems) = 0; + + // Increments the counter for the number of render passes executed this frame (does not include passes that are disabled) + virtual void IncrementFrameRenderPassCount() = 0; + + // Get frame statistics from the Pass System + virtual PassSystemFrameStatistics GetFrameStatistics() = 0; + // --- Pass Factory related functionality --- //! Directly creates a pass given a PassDescriptor @@ -171,17 +200,6 @@ namespace AZ //! Find matching passes from registered passes with specified filter virtual AZStd::vector FindPasses(const PassFilter& passFilter) const = 0; - //! Find the SwapChainPass associated with window Handle - virtual SwapChainPass* FindSwapChainPass(AzFramework::NativeWindowHandle windowHandle) const = 0; - - using OnReadyLoadTemplatesEvent = AZ::Event<>; - //! Connect a handler to listen to the event that the pass system is ready to load pass templates - //! The event is triggered when pass system is initialized and asset system is ready. - //! The handler can add new pass templates or load pass template mappings from assets - virtual void ConnectEvent(OnReadyLoadTemplatesEvent::Handler& handler) = 0; - - virtual PassSystemState GetState() const = 0; - private: // These functions are only meant to be used by the Pass class @@ -199,7 +217,6 @@ namespace AZ //! Unregisters the pass with the pass library. Called in the Pass destructor. virtual void UnregisterPass(Pass* pass) = 0; - }; namespace PassSystemEvents diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h index 735b3930c8..b0d941083b 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h @@ -42,6 +42,8 @@ namespace AZ //! Expose shader resource group. ShaderResourceGroup* GetShaderResourceGroup(); + u32 GetDrawItemCount(); + protected: explicit RasterPass(const PassDescriptor& descriptor); @@ -68,6 +70,7 @@ namespace AZ RHI::Viewport m_viewportState; bool m_overrideScissorSate = false; bool m_overrideViewportState = false; + u32 m_drawItemCount = 0; }; } // namespace RPI } // namespace AZ diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/PassSystem.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/PassSystem.cpp index 91cc645947..3092b9ecd0 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/PassSystem.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/PassSystem.cpp @@ -308,6 +308,7 @@ namespace AZ AZ_PROFILE_FUNCTION(AZ::Debug::ProfileCategory::AzRender); AZ_ATOM_PROFILE_FUNCTION("RPI", "PassSystem: FrameUpdate"); + ResetFrameStatistics(); ProcessQueuedChanges(); m_state = PassSystemState::Rendering; @@ -392,6 +393,29 @@ namespace AZ handler.Connect(m_loadTemplatesEvent); } + void PassSystem::ResetFrameStatistics() + { + m_frameStatistics.m_numRenderPassesExecuted = 0; + m_frameStatistics.m_totalDrawItemsRendered = 0; + m_frameStatistics.m_maxDrawItemsRenderedInAPass = 0; + } + + PassSystemFrameStatistics PassSystem::GetFrameStatistics() + { + return m_frameStatistics; + } + + void PassSystem::IncrementFrameDrawItemCount(u32 numDrawItems) + { + m_frameStatistics.m_totalDrawItemsRendered += numDrawItems; + m_frameStatistics.m_maxDrawItemsRenderedInAPass = AZStd::max(m_frameStatistics.m_maxDrawItemsRenderedInAPass, numDrawItems); + } + + void PassSystem::IncrementFrameRenderPassCount() + { + ++m_frameStatistics.m_numRenderPassesExecuted; + } + // --- Pass Factory Functions --- void PassSystem::AddPassCreator(Name className, PassCreator createFunction) diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp index ee38c1e5ef..ae74e355f2 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp @@ -112,6 +112,11 @@ namespace AZ return m_shaderResourceGroup.get(); } + u32 RasterPass::GetDrawItemCount() + { + return m_drawItemCount; + } + // --- Pass behaviour overrides --- void RasterPass::Validate(PassValidationResults& validationResults) @@ -145,6 +150,8 @@ namespace AZ // Draw List m_drawListView = view->GetDrawList(m_drawListTag); + m_drawItemCount = m_drawListView.size(); + PassSystemInterface::Get()->IncrementFrameDrawItemCount(m_drawItemCount); } RenderPass::FrameBeginInternal(params); diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RenderPass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RenderPass.cpp index 96649573b2..92e6d8b2b6 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RenderPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RenderPass.cpp @@ -200,6 +200,8 @@ namespace AZ m_attachmentCopy.lock()->FrameBegin(params); } CollectSrgs(); + + PassSystemInterface::Get()->IncrementFrameRenderPassCount(); } diff --git a/Gems/AtomLyIntegration/AtomViewportDisplayInfo/Code/Source/AtomViewportDisplayInfoSystemComponent.cpp b/Gems/AtomLyIntegration/AtomViewportDisplayInfo/Code/Source/AtomViewportDisplayInfoSystemComponent.cpp index af9fd7fdd8..ef341f8dc8 100644 --- a/Gems/AtomLyIntegration/AtomViewportDisplayInfo/Code/Source/AtomViewportDisplayInfoSystemComponent.cpp +++ b/Gems/AtomLyIntegration/AtomViewportDisplayInfo/Code/Source/AtomViewportDisplayInfoSystemComponent.cpp @@ -229,25 +229,20 @@ namespace AZ::Render AZ::RPI::ViewportContextPtr viewportContext = GetViewportContext(); auto rootPass = viewportContext->GetCurrentPipeline()->GetRootPass(); const RPI::PipelineStatisticsResult stats = rootPass->GetLatestPipelineStatisticsResult(); - AZStd::function)> containingPassCount = [&containingPassCount](const AZ::RPI::Ptr pass) - { - int count = 1; - if (auto passAsParent = pass->AsParent()) - { - for (const auto& child : passAsParent->GetChildren()) - { - count += containingPassCount(child); - } - } - return count; - }; - const int numPasses = containingPassCount(rootPass); + + RPI::PassSystemFrameStatistics passSystemFrameStatistics = AZ::RPI::PassSystemInterface::Get()->GetFrameStatistics(); + DrawLine(AZStd::string::format( - "Total Passes: %d Vertex Count: %lld Primitive Count: %lld", - numPasses, + "RenderPasses: %d Vertex Count: %lld Primitive Count: %lld", + passSystemFrameStatistics.m_numRenderPassesExecuted, aznumeric_cast(stats.m_vertexCount), aznumeric_cast(stats.m_primitiveCount) )); + DrawLine(AZStd::string::format( + "Total Draw Item Count: %d Max Draw Items in a Pass: %d", + passSystemFrameStatistics.m_totalDrawItemsRendered, + passSystemFrameStatistics.m_maxDrawItemsRenderedInAPass + )); } void AtomViewportDisplayInfoSystemComponent::UpdateFramerate() From 64cff38f8294338371555ef737008be3879f2a28 Mon Sep 17 00:00:00 2001 From: antonmic <56370189+antonmic@users.noreply.github.com> Date: Sun, 8 Aug 2021 16:46:42 -0700 Subject: [PATCH 2/7] Addressing PR feedback and fixed a small issue with the draw item count display Signed-off-by: antonmic <56370189+antonmic@users.noreply.github.com> --- .../Code/Include/Atom/RHI/ShaderResourceGroupDebug.h | 12 +++++++++++- .../RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp | 6 +++--- Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp | 12 ++++++------ .../RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp | 6 +++--- .../RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp | 4 +++- 5 files changed, 26 insertions(+), 14 deletions(-) diff --git a/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h b/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h index b81f9e0c0b..d9ea77d823 100644 --- a/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h +++ b/Gems/Atom/RHI/Code/Include/Atom/RHI/ShaderResourceGroupDebug.h @@ -14,8 +14,18 @@ namespace AZ struct DrawItem; class ShaderResourceGroup; + /// Given a ShaderResourceGroup and a reference ConstantsData input, this function will fetch the ConstantsData on the SRG and compare it + /// to the reference ConstantsData. It will print the names of any constants that are different between the two. + /// The parameter updateReferenceData can be used to set the reference data to the SRG's constant data after the comparison. This is + /// useful for keeping track of differences in between calls to the function, such as between frames. void PrintConstantDataDiff(const ShaderResourceGroup& shaderResourceGroup, ConstantsData& referenceData, bool updateReferenceData = false); - void PrintConstantDataDiff(const DrawItem& drawItem, ConstantsData& referenceData, u32 srgBindingSlot, bool updateReferenceData = false); + + /// Given a DrawItem, an SRG binding slot on that draw item and a reference ConstantsData input, this function will fetch the ConstantsData + /// from the draw item's SRG at the binding slot and compare it to the reference ConstantsData. It will print the names of any constants + /// that are different between the two. + /// The parameter updateReferenceData can be used to set the reference data to the draw item's constant data after the comparison. This is + /// useful for keeping track of differences in between calls to the function, such as between frames. + void PrintConstantDataDiff(const DrawItem& drawItem, ConstantsData& referenceData, uint32_t srgBindingSlot, bool updateReferenceData = false); } } diff --git a/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp b/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp index f14d1d4c4e..8be053e4ef 100644 --- a/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI.Reflect/ConstantsLayout.cpp @@ -151,11 +151,11 @@ namespace AZ void ConstantsLayout::DebugPrintNames(AZStd::array_view constantList) const { AZStd::string output; - for (const ShaderInputConstantIndex& constandIdx : constantList) + for (const ShaderInputConstantIndex& constantIdx : constantList) { - if (constandIdx.GetIndex() < m_inputs.size()) + if (constantIdx.GetIndex() < m_inputs.size()) { - output += m_inputs[constandIdx.GetIndex()].m_name.GetCStr(); + output += m_inputs[constantIdx.GetIndex()].m_name.GetCStr(); output += " - "; } } diff --git a/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp b/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp index d4caa3dd66..1524883e54 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp @@ -408,26 +408,26 @@ namespace AZ bool ConstantsData::ConstantIsEqual(const ConstantsData& other, ShaderInputConstantIndex inputIndex) const { - AZStd::array_view myConstans = GetConstantRaw(inputIndex); - AZStd::array_view otherConstans = other.GetConstantRaw(inputIndex); + AZStd::array_view myConstant = GetConstantRaw(inputIndex); + AZStd::array_view otherConstant = other.GetConstantRaw(inputIndex); // If they point to the same data, they are equal - if (myConstans == otherConstans) + if (myConstant == otherConstant) { return true; } // If they point to data of different size, they are not equal - if (myConstans.size() != otherConstans.size()) + if (myConstant.size() != otherConstant.size()) { return false; } // If they point to differing data of same size, compare the data // Note: due to small size of data this loop will be faster than a mem compare - for(uint32_t i = 0; i < myConstans.size(); ++i) + for(uint32_t i = 0; i < myConstant.size(); ++i) { - if (myConstans[i] != otherConstans[i]) + if (myConstant[i] != otherConstant[i]) { return false; } diff --git a/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp b/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp index 4bd8d0aab8..54399eba88 100644 --- a/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp +++ b/Gems/Atom/RHI/Code/Source/RHI/ShaderResourceGroupDebug.cpp @@ -36,10 +36,10 @@ namespace AZ } } - void PrintConstantDataDiff(const DrawItem& drawItem, ConstantsData& referenceData, u32 srgBindingSlot, bool updateReferenceData) + void PrintConstantDataDiff(const DrawItem& drawItem, ConstantsData& referenceData, uint32_t srgBindingSlot, bool updateReferenceData) { - s32 srgIndex = -1; - for (u32 i = 0; i < drawItem.m_shaderResourceGroupCount; ++i) + int srgIndex = -1; + for (uint32_t i = 0; i < drawItem.m_shaderResourceGroupCount; ++i) { if (drawItem.m_shaderResourceGroups[i]->GetBindingSlot() == srgBindingSlot) { diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp index 13f2215c8b..2e61542c36 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp @@ -166,11 +166,14 @@ namespace AZ // clean up data m_drawListView = {}; m_combinedDrawList.clear(); + m_drawItemCount = 0; // draw list from view was sorted and if it's the only draw list then we can use it directly if (viewDrawList.size() > 0 && drawLists.size() == 0) { m_drawListView = viewDrawList; + m_drawItemCount += viewDrawList.size(); + PassSystemInterface::Get()->IncrementFrameDrawItemCount(m_drawItemCount); return; } @@ -178,7 +181,6 @@ namespace AZ drawLists.push_back(viewDrawList); // combine draw items from mutiple draw lists to one draw list and sort it. - m_drawItemCount = 0; for (auto drawList : drawLists) { m_drawItemCount += drawList.size(); From 43ae25b49b40b888d2e99d8d570e4fd23c0fdb13 Mon Sep 17 00:00:00 2001 From: antonmic <56370189+antonmic@users.noreply.github.com> Date: Tue, 10 Aug 2021 10:00:42 -0700 Subject: [PATCH 3/7] fixed compiler loss of precision warning Signed-off-by: antonmic <56370189+antonmic@users.noreply.github.com> --- Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp index 2e61542c36..4da5560908 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp @@ -172,7 +172,7 @@ namespace AZ if (viewDrawList.size() > 0 && drawLists.size() == 0) { m_drawListView = viewDrawList; - m_drawItemCount += viewDrawList.size(); + m_drawItemCount += (u32)viewDrawList.size(); PassSystemInterface::Get()->IncrementFrameDrawItemCount(m_drawItemCount); return; } @@ -183,7 +183,7 @@ namespace AZ // combine draw items from mutiple draw lists to one draw list and sort it. for (auto drawList : drawLists) { - m_drawItemCount += drawList.size(); + m_drawItemCount += (u32)drawList.size(); } PassSystemInterface::Get()->IncrementFrameDrawItemCount(m_drawItemCount); m_combinedDrawList.resize(m_drawItemCount); From 5472d6768e8a3ceb81f901251858bb58600439d0 Mon Sep 17 00:00:00 2001 From: antonmic <56370189+antonmic@users.noreply.github.com> Date: Tue, 10 Aug 2021 12:48:26 -0700 Subject: [PATCH 4/7] Minor improvements Signed-off-by: antonmic <56370189+antonmic@users.noreply.github.com> --- .../RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h | 6 +++--- .../RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h index cdbf983561..2c884ceedf 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Public/Pass/RasterPass.h @@ -38,12 +38,12 @@ namespace AZ void SetDrawListTag(Name drawListName); - void SetPipelineStateDataIndex(u32 index); + void SetPipelineStateDataIndex(uint32_t index); //! Expose shader resource group. ShaderResourceGroup* GetShaderResourceGroup(); - u32 GetDrawItemCount(); + uint32_t GetDrawItemCount(); protected: explicit RasterPass(const PassDescriptor& descriptor); @@ -78,7 +78,7 @@ namespace AZ RHI::Viewport m_viewportState; bool m_overrideScissorSate = false; bool m_overrideViewportState = false; - u32 m_drawItemCount = 0; + uint32_t m_drawItemCount = 0; }; } // namespace RPI } // namespace AZ diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp index 4da5560908..ce02e1c570 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/RasterPass.cpp @@ -104,7 +104,7 @@ namespace AZ m_flags.m_hasDrawListTag = true; } - void RasterPass::SetPipelineStateDataIndex(u32 index) + void RasterPass::SetPipelineStateDataIndex(uint32_t index) { m_pipelineStateDataIndex.m_index = index; } @@ -114,7 +114,7 @@ namespace AZ return m_shaderResourceGroup.get(); } - u32 RasterPass::GetDrawItemCount() + uint32_t RasterPass::GetDrawItemCount() { return m_drawItemCount; } @@ -172,7 +172,7 @@ namespace AZ if (viewDrawList.size() > 0 && drawLists.size() == 0) { m_drawListView = viewDrawList; - m_drawItemCount += (u32)viewDrawList.size(); + m_drawItemCount += static_cast(viewDrawList.size()); PassSystemInterface::Get()->IncrementFrameDrawItemCount(m_drawItemCount); return; } @@ -183,7 +183,7 @@ namespace AZ // combine draw items from mutiple draw lists to one draw list and sort it. for (auto drawList : drawLists) { - m_drawItemCount += (u32)drawList.size(); + m_drawItemCount += static_cast(drawList.size()); } PassSystemInterface::Get()->IncrementFrameDrawItemCount(m_drawItemCount); m_combinedDrawList.resize(m_drawItemCount); @@ -211,7 +211,7 @@ namespace AZ void RasterPass::SetupFrameGraphDependencies(RHI::FrameGraphInterface frameGraph) { RenderPass::SetupFrameGraphDependencies(frameGraph); - frameGraph.SetEstimatedItemCount(static_cast(m_drawListView.size())); + frameGraph.SetEstimatedItemCount(static_cast(m_drawListView.size())); } void RasterPass::CompileResources(const RHI::FrameGraphCompileContext& context) From fbcb6510e68721c19ac17c5618fd1d021514d901 Mon Sep 17 00:00:00 2001 From: sconel <32552662+sconel@users.noreply.github.com> Date: Wed, 11 Aug 2021 10:45:31 -0700 Subject: [PATCH 5/7] Update storage of Prefab Dom info to be best effort (#2862) * Update storage of Prefab Dom info to be best effort Signed-off-by: sconel * Update IssueReporter to Skipped instead of PartialSkip Signed-off-by: sconel * Address PR feedback Signed-off-by: sconel * Fix failing Prefab Unit Test that expected default values to be stripped Signed-off-by: sconel --- .../Instance/InstanceToTemplatePropagator.cpp | 20 +--- .../AzToolsFramework/Prefab/Link/Link.cpp | 3 +- .../Prefab/PrefabDomUtils.cpp | 94 +++++++++++++++---- .../AzToolsFramework/Prefab/PrefabDomUtils.h | 28 ++++-- .../AzToolsFramework/Prefab/PrefabLoader.cpp | 2 +- .../AzToolsFramework/Prefab/PrefabUndo.cpp | 4 +- .../Prefab/Spawnable/EditorInfoRemover.cpp | 2 +- .../Prefab/Spawnable/SpawnableUtils.cpp | 2 +- ...refabInstanceToTemplatePropagatorTests.cpp | 93 ++++++++++++++++++ .../Tests/Prefab/PrefabTestComponent.cpp | 14 +++ .../Tests/Prefab/PrefabTestComponent.h | 18 ++++ .../Tests/Prefab/PrefabTestFixture.cpp | 1 + .../Prefab/PrefabUpdateWithPatchesTests.cpp | 5 +- .../Pipeline/NetworkPrefabProcessor.cpp | 2 +- 14 files changed, 241 insertions(+), 47 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp index 814c4e2a7f..36b39f3a72 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Instance/InstanceToTemplatePropagator.cpp @@ -46,7 +46,6 @@ namespace AzToolsFramework bool InstanceToTemplatePropagator::GenerateDomForEntity(PrefabDom& generatedEntityDom, const AZ::Entity& entity) { - //grab the owning instance so we can use the entityIdMapper in settings InstanceOptionalReference owningInstance = m_instanceEntityMapperInterface->FindOwningInstance(entity.GetId()); if (!owningInstance) @@ -54,19 +53,8 @@ namespace AzToolsFramework AZ_Error("Prefab", false, "Entity does not belong to an instance"); return false; } - - InstanceEntityIdMapper entityIdMapper; - entityIdMapper.SetStoringInstance(owningInstance->get()); - - //create settings so that the serialized entity dom undergoes mapping from entity id to entity alias - AZ::JsonSerializerSettings settings; - settings.m_metadata.Add(static_cast(&entityIdMapper)); - - //generate PrefabDom using Json serialization system - AZ::JsonSerializationResult::ResultCode result = AZ::JsonSerialization::Store( - generatedEntityDom, generatedEntityDom.GetAllocator(), entity, settings); - return result.GetOutcome() == AZ::JsonSerializationResult::Outcomes::Success; + return PrefabDomUtils::StoreEntityInPrefabDomFormat(entity, owningInstance->get(), generatedEntityDom); } bool InstanceToTemplatePropagator::GenerateDomForInstance(PrefabDom& generatedInstanceDom, const Prefab::Instance& instance) @@ -185,8 +173,10 @@ namespace AzToolsFramework else { AZ_Error( - "Prefab", result.GetOutcome() != AZ::JsonSerializationResult::Outcomes::PartialSkip, - "Some of the patches are not successfully applied."); + "Prefab", + (result.GetOutcome() != AZ::JsonSerializationResult::Outcomes::Skipped) && + (result.GetOutcome() != AZ::JsonSerializationResult::Outcomes::PartialSkip), + "Some of the patches were not successfully applied."); m_prefabSystemComponentInterface->SetTemplateDirtyFlag(templateId, true); m_prefabSystemComponentInterface->PropagateTemplateChanges(templateId, instanceToExclude); return true; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp index 3c7e5ff3f8..940a2787cb 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Link/Link.cpp @@ -196,7 +196,8 @@ namespace AzToolsFramework m_sourceTemplateId, m_targetTemplateId); return false; } - if (applyPatchResult.GetOutcome() == AZ::JsonSerializationResult::Outcomes::PartialSkip) + if (applyPatchResult.GetOutcome() == AZ::JsonSerializationResult::Outcomes::PartialSkip || + applyPatchResult.GetOutcome() == AZ::JsonSerializationResult::Outcomes::Skipped) { AZ_Error( "Prefab", false, diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp index 86983d7912..1ad88e53d2 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.cpp @@ -26,6 +26,30 @@ namespace AzToolsFramework { namespace PrefabDomUtils { + namespace Internal + { + AZ::JsonSerializationResult::ResultCode JsonIssueReporter(AZStd::string& scratchBuffer, + AZStd::string_view message, AZ::JsonSerializationResult::ResultCode result, AZStd::string_view path) + { + namespace JSR = AZ::JsonSerializationResult; + + if (result.GetProcessing() == JSR::Processing::Halted) + { + scratchBuffer.append(message.begin(), message.end()); + scratchBuffer.append("\n Reason: "); + result.AppendToString(scratchBuffer, path); + scratchBuffer.append("."); + AZ_Warning("Prefab Serialization", false, "%s", scratchBuffer.c_str()); + + scratchBuffer.clear(); + + return JSR::ResultCode(result.GetTask(), JSR::Outcomes::Skipped); + } + + return result; + } + } + PrefabDomValueReference FindPrefabDomValue(PrefabDomValue& parentValue, const char* valueName) { PrefabDomValue::MemberIterator valueIterator = parentValue.FindMember(valueName); @@ -48,7 +72,7 @@ namespace AzToolsFramework return valueIterator->value; } - bool StoreInstanceInPrefabDom(const Instance& instance, PrefabDom& prefabDom, StoreInstanceFlags flags) + bool StoreInstanceInPrefabDom(const Instance& instance, PrefabDom& prefabDom, StoreFlags flags) { InstanceEntityIdMapper entityIdMapper; entityIdMapper.SetStoringInstance(instance); @@ -59,11 +83,21 @@ namespace AzToolsFramework settings.m_metadata.Add(static_cast(&entityIdMapper)); settings.m_metadata.Add(&entityIdMapper); - if ((flags & StoreInstanceFlags::StripDefaultValues) != StoreInstanceFlags::StripDefaultValues) + if ((flags & StoreFlags::StripDefaultValues) != StoreFlags::StripDefaultValues) { settings.m_keepDefaults = true; } + AZStd::string scratchBuffer; + auto issueReportingCallback = [&scratchBuffer] + (AZStd::string_view message, AZ::JsonSerializationResult::ResultCode result, + AZStd::string_view path) -> AZ::JsonSerializationResult::ResultCode + { + return Internal::JsonIssueReporter(scratchBuffer, message, result, path); + }; + + settings.m_reporting = AZStd::move(issueReportingCallback); + AZ::JsonSerializationResult::ResultCode result = AZ::JsonSerialization::Store(prefabDom, prefabDom.GetAllocator(), instance, settings); @@ -80,7 +114,38 @@ namespace AzToolsFramework return true; } - bool LoadInstanceFromPrefabDom(Instance& instance, const PrefabDom& prefabDom, LoadInstanceFlags flags) + bool StoreEntityInPrefabDomFormat(const AZ::Entity& entity, Instance& owningInstance, PrefabDom& prefabDom, StoreFlags flags) + { + InstanceEntityIdMapper entityIdMapper; + entityIdMapper.SetStoringInstance(owningInstance); + + //create settings so that the serialized entity dom undergoes mapping from entity id to entity alias + AZ::JsonSerializerSettings settings; + settings.m_metadata.Add(static_cast(&entityIdMapper)); + + if ((flags & StoreFlags::StripDefaultValues) != StoreFlags::StripDefaultValues) + { + settings.m_keepDefaults = true; + } + + AZStd::string scratchBuffer; + auto issueReportingCallback = [&scratchBuffer] + (AZStd::string_view message, AZ::JsonSerializationResult::ResultCode result, + AZStd::string_view path) -> AZ::JsonSerializationResult::ResultCode + { + return Internal::JsonIssueReporter(scratchBuffer, message, result, path); + }; + + settings.m_reporting = AZStd::move(issueReportingCallback); + + //generate PrefabDom using Json serialization system + AZ::JsonSerializationResult::ResultCode result = AZ::JsonSerialization::Store( + prefabDom, prefabDom.GetAllocator(), entity, settings); + + return result.GetOutcome() == AZ::JsonSerializationResult::Outcomes::Success; + } + + bool LoadInstanceFromPrefabDom(Instance& instance, const PrefabDom& prefabDom, LoadFlags flags) { // When entities are rebuilt they are first destroyed. As a result any assets they were exclusively holding on to will // be released and reloaded once the entities are built up again. By suspending asset release temporarily the asset reload @@ -89,7 +154,7 @@ namespace AzToolsFramework InstanceEntityIdMapper entityIdMapper; entityIdMapper.SetLoadingInstance(instance); - if ((flags & LoadInstanceFlags::AssignRandomEntityId) == LoadInstanceFlags::AssignRandomEntityId) + if ((flags & LoadFlags::AssignRandomEntityId) == LoadFlags::AssignRandomEntityId) { entityIdMapper.SetEntityIdGenerationApproach(InstanceEntityIdMapper::EntityIdGenerationApproach::Random); } @@ -119,7 +184,7 @@ namespace AzToolsFramework } bool LoadInstanceFromPrefabDom( - Instance& instance, const PrefabDom& prefabDom, AZStd::vector>& referencedAssets, LoadInstanceFlags flags) + Instance& instance, const PrefabDom& prefabDom, AZStd::vector>& referencedAssets, LoadFlags flags) { // When entities are rebuilt they are first destroyed. As a result any assets they were exclusively holding on to will // be released and reloaded once the entities are built up again. By suspending asset release temporarily the asset reload @@ -128,7 +193,7 @@ namespace AzToolsFramework InstanceEntityIdMapper entityIdMapper; entityIdMapper.SetLoadingInstance(instance); - if ((flags & LoadInstanceFlags::AssignRandomEntityId) == LoadInstanceFlags::AssignRandomEntityId) + if ((flags & LoadFlags::AssignRandomEntityId) == LoadFlags::AssignRandomEntityId) { entityIdMapper.SetEntityIdGenerationApproach(InstanceEntityIdMapper::EntityIdGenerationApproach::Random); } @@ -161,7 +226,7 @@ namespace AzToolsFramework } bool LoadInstanceFromPrefabDom( - Instance& instance, Instance::EntityList& newlyAddedEntities, const PrefabDom& prefabDom, LoadInstanceFlags flags) + Instance& instance, Instance::EntityList& newlyAddedEntities, const PrefabDom& prefabDom, LoadFlags flags) { // When entities are rebuilt they are first destroyed. As a result any assets they were exclusively holding on to will // be released and reloaded once the entities are built up again. By suspending asset release temporarily the asset reload @@ -170,7 +235,7 @@ namespace AzToolsFramework InstanceEntityIdMapper entityIdMapper; entityIdMapper.SetLoadingInstance(instance); - if ((flags & LoadInstanceFlags::AssignRandomEntityId) == LoadInstanceFlags::AssignRandomEntityId) + if ((flags & LoadFlags::AssignRandomEntityId) == LoadFlags::AssignRandomEntityId) { entityIdMapper.SetEntityIdGenerationApproach(InstanceEntityIdMapper::EntityIdGenerationApproach::Random); } @@ -240,15 +305,12 @@ namespace AzToolsFramework AZ::JsonSerializationResult::ResultCode ApplyPatches( PrefabDomValue& prefabDomToApplyPatchesOn, PrefabDom::AllocatorType& allocator, const PrefabDomValue& patches) { - auto issueReportingCallback = [](AZStd::string_view, AZ::JsonSerializationResult::ResultCode result, - AZStd::string_view) -> AZ::JsonSerializationResult::ResultCode + AZStd::string scratchBuffer; + auto issueReportingCallback = [&scratchBuffer] + (AZStd::string_view message, AZ::JsonSerializationResult::ResultCode result, + AZStd::string_view path) -> AZ::JsonSerializationResult::ResultCode { - using namespace AZ::JsonSerializationResult; - if (result.GetProcessing() == Processing::Halted) - { - return ResultCode(result.GetTask(), Outcomes::PartialSkip); - } - return result; + return Internal::JsonIssueReporter(scratchBuffer, message, result, path); }; AZ::JsonApplyPatchSettings applyPatchSettings; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h index e773b581dd..b04cf9f57d 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabDomUtils.h @@ -38,7 +38,7 @@ namespace AzToolsFramework PrefabDomValueReference FindPrefabDomValue(PrefabDomValue& parentValue, const char* valueName); PrefabDomValueConstReference FindPrefabDomValue(const PrefabDomValue& parentValue, const char* valueName); - enum class StoreInstanceFlags : uint8_t + enum class StoreFlags : uint8_t { //! No flags used during the call to LoadInstanceFromPrefabDom. None = 0, @@ -47,7 +47,7 @@ namespace AzToolsFramework //! such as saving to disk, this flag will control that behavior. StripDefaultValues = 1 << 0 }; - AZ_DEFINE_ENUM_BITWISE_OPERATORS(StoreInstanceFlags); + AZ_DEFINE_ENUM_BITWISE_OPERATORS(StoreFlags); /** * Stores a valid Prefab Instance within a Prefab Dom. Useful for generating Templates @@ -56,9 +56,21 @@ namespace AzToolsFramework * @param flags Controls behavior such as whether to store default values * @return bool on whether the operation succeeded */ - bool StoreInstanceInPrefabDom(const Instance& instance, PrefabDom& prefabDom, StoreInstanceFlags flags = StoreInstanceFlags::None); + bool StoreInstanceInPrefabDom(const Instance& instance, PrefabDom& prefabDom, StoreFlags flags = StoreFlags::None); - enum class LoadInstanceFlags : uint8_t + /** + * Stores a valid entity in Prefab Dom format. + * @param entity The entity to store + * @param owningInstance The instance owning the passed in entity. + * Used for contextualizing the entity's place in a Prefab hierarchy. + * @param prefabDom The prefabDom that will be used to store the entity data + * @param flags controls behavior such as whether to store default values + * @return bool on whether the operation succeeded + */ + bool StoreEntityInPrefabDomFormat(const AZ::Entity& entity, Instance& owningInstance, PrefabDom& prefabDom, + StoreFlags flags = StoreFlags::None); + + enum class LoadFlags : uint8_t { //! No flags used during the call to LoadInstanceFromPrefabDom. None = 0, @@ -66,7 +78,7 @@ namespace AzToolsFramework //! unique, e.g. when they are duplicates of live entities, this flag will assign them a random new id. AssignRandomEntityId = 1 << 0 }; - AZ_DEFINE_ENUM_BITWISE_OPERATORS(LoadInstanceFlags); + AZ_DEFINE_ENUM_BITWISE_OPERATORS(LoadFlags); /** * Loads a valid Prefab Instance from a Prefab Dom. Useful for generating Instances. @@ -76,7 +88,7 @@ namespace AzToolsFramework * @return bool on whether the operation succeeded. */ bool LoadInstanceFromPrefabDom( - Instance& instance, const PrefabDom& prefabDom, LoadInstanceFlags flags = LoadInstanceFlags::None); + Instance& instance, const PrefabDom& prefabDom, LoadFlags flags = LoadFlags::None); /** * Loads a valid Prefab Instance from a Prefab Dom. Useful for generating Instances. @@ -88,7 +100,7 @@ namespace AzToolsFramework */ bool LoadInstanceFromPrefabDom( Instance& instance, const PrefabDom& prefabDom, AZStd::vector>& referencedAssets, - LoadInstanceFlags flags = LoadInstanceFlags::None); + LoadFlags flags = LoadFlags::None); /** * Loads a valid Prefab Instance from a Prefab Dom. Useful for generating Instances. @@ -101,7 +113,7 @@ namespace AzToolsFramework */ bool LoadInstanceFromPrefabDom( Instance& instance, Instance::EntityList& newlyAddedEntities, const PrefabDom& prefabDom, - LoadInstanceFlags flags = LoadInstanceFlags::None); + LoadFlags flags = LoadFlags::None); inline PrefabDomPath GetPrefabDomInstancePath(const char* instanceName) { diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp index ea54c9d10c..5091a2d1bc 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabLoader.cpp @@ -328,7 +328,7 @@ namespace AzToolsFramework PrefabDom storedPrefabDom(&savingTemplateDom->get().GetAllocator()); if (!PrefabDomUtils::StoreInstanceInPrefabDom(savingPrefabInstance, storedPrefabDom, - PrefabDomUtils::StoreInstanceFlags::StripDefaultValues)) + PrefabDomUtils::StoreFlags::StripDefaultValues)) { return false; } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp index 955af18a66..fc64e0b5b3 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabUndo.cpp @@ -266,8 +266,8 @@ namespace AzToolsFramework AZ_Error( "Prefab", - result.GetOutcome() == AZ::JsonSerializationResult::Outcomes::PartialSkip || - result.GetOutcome() == AZ::JsonSerializationResult::Outcomes::Success, + (result.GetOutcome() != AZ::JsonSerializationResult::Outcomes::Skipped) && + (result.GetOutcome() != AZ::JsonSerializationResult::Outcomes::PartialSkip), "Some of the patches are not successfully applied."); //remove the link id placed into the instance diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/EditorInfoRemover.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/EditorInfoRemover.cpp index 1d856fa395..dfba1d54ba 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/EditorInfoRemover.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/EditorInfoRemover.cpp @@ -513,7 +513,7 @@ exportComponent, prefabProcessorContext); // convert Prefab DOM into Prefab Instance. AZStd::unique_ptr instance(aznew Instance()); if (!Prefab::PrefabDomUtils::LoadInstanceFromPrefabDom(*instance, prefab, - Prefab::PrefabDomUtils::LoadInstanceFlags::AssignRandomEntityId)) + Prefab::PrefabDomUtils::LoadFlags::AssignRandomEntityId)) { PrefabDomValueReference sourceReference = PrefabDomUtils::FindPrefabDomValue(prefab, PrefabDomUtils::SourceName); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp index 1373d2fa75..902f439280 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/Spawnable/SpawnableUtils.cpp @@ -30,7 +30,7 @@ namespace AzToolsFramework::Prefab::SpawnableUtils { Instance instance; if (Prefab::PrefabDomUtils::LoadInstanceFromPrefabDom(instance, prefabDom, referencedAssets, - Prefab::PrefabDomUtils::LoadInstanceFlags::AssignRandomEntityId)) // Always assign random entity ids because the spawnable is + Prefab::PrefabDomUtils::LoadFlags::AssignRandomEntityId)) // Always assign random entity ids because the spawnable is // going to be used to create clones of the entities. { AzFramework::Spawnable::EntityList& entities = spawnable.GetEntities(); diff --git a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabInstanceToTemplatePropagatorTests.cpp b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabInstanceToTemplatePropagatorTests.cpp index 8379992553..ebe35a5402 100644 --- a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabInstanceToTemplatePropagatorTests.cpp +++ b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabInstanceToTemplatePropagatorTests.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -18,6 +19,98 @@ namespace UnitTest { using PrefabInstanceToTemplateTests = PrefabTestFixture; + TEST_F(PrefabInstanceToTemplateTests, GenerateEntityDom_InvalidType_InvalidTypeSkipped) + { + const char* newEntityName = "New Entity"; + AZ::Entity* newEntity = CreateEntity(newEntityName, false); + ASSERT_TRUE(newEntity); + + // Add a component with a member that is missing reflection info + // and a member that is properly reflected + PrefabTestComponentWithUnReflectedTypeMember* newComponent = + newEntity->CreateComponent(); + + ASSERT_TRUE(newComponent); + + AZStd::unique_ptr prefabInstance = m_prefabSystemComponent->CreatePrefab({ newEntity }, {}, "test/path"); + ASSERT_TRUE(prefabInstance); + + PrefabDom entityDom; + m_instanceToTemplateInterface->GenerateDomForEntity(entityDom, *newEntity); + + auto componentListDom = entityDom.FindMember("Components"); + + // Confirm that there is only one component in the entityDom + ASSERT_NE(componentListDom, entityDom.MemberEnd()); + ASSERT_TRUE(componentListDom->value.IsObject()); + ASSERT_EQ(componentListDom->value.MemberCount(), 1); + + auto testComponentDom = componentListDom->value.MemberBegin(); + ASSERT_TRUE(testComponentDom->value.IsObject()); + + // Confirm that the componentDom does not contained the invalid UnReflectedType + // We want to skip over it and produce a best effort entityDom + auto unReflectedTypeDom = testComponentDom->value.FindMember("UnReflectedType"); + ASSERT_EQ(unReflectedTypeDom, testComponentDom->value.MemberEnd()); + + // Confirm the presence of the valid ReflectedType + auto reflectedTypeDom = testComponentDom->value.FindMember("ReflectedType"); + ASSERT_NE(reflectedTypeDom, testComponentDom->value.MemberEnd()); + + // Confirm the reflected type has the correct type and value + ASSERT_TRUE(reflectedTypeDom->value.IsInt()); + EXPECT_EQ(reflectedTypeDom->value.GetInt(), newComponent->m_reflectedType); + } + + TEST_F(PrefabInstanceToTemplateTests, GenerateInstanceDom_InvalidType_InvalidTypeSkipped) + { + const char* newEntityName = "New Entity"; + AZ::Entity* newEntity = CreateEntity(newEntityName, false); + ASSERT_TRUE(newEntity); + + // Add a component with a member that is missing reflection info + // and a member that is properly reflected + PrefabTestComponentWithUnReflectedTypeMember* newComponent = + newEntity->CreateComponent(); + + ASSERT_TRUE(newComponent); + + AZStd::unique_ptr prefabInstance = m_prefabSystemComponent->CreatePrefab({ newEntity }, {}, "test/path"); + ASSERT_TRUE(prefabInstance); + + PrefabDom instanceDom; + m_instanceToTemplateInterface->GenerateDomForInstance(instanceDom, *prefabInstance); + + // Acquire the entity out of the instanceDom + auto entitiesDom = instanceDom.FindMember(PrefabDomUtils::EntitiesName); + ASSERT_NE(entitiesDom, instanceDom.MemberEnd()); + ASSERT_EQ(entitiesDom->value.MemberCount(), 1); + + auto entityDom = entitiesDom->value.MemberBegin(); + auto componentListDom = entityDom->value.FindMember("Components"); + + // Confirm that there is only one component in the entityDom + ASSERT_NE(componentListDom, entityDom->value.MemberEnd()); + ASSERT_TRUE(componentListDom->value.IsObject()); + ASSERT_EQ(componentListDom->value.MemberCount(), 1); + + auto testComponentDom = componentListDom->value.MemberBegin(); + ASSERT_TRUE(testComponentDom->value.IsObject()); + + // Confirm that the componentDom does not contained the invalid UnReflectedType + // We want to skip over it and produce a best effort entityDom + auto unReflectedTypeDom = testComponentDom->value.FindMember("UnReflectedType"); + ASSERT_EQ(unReflectedTypeDom, testComponentDom->value.MemberEnd()); + + // Confirm the presence of the valid ReflectedType + auto reflectedTypeDom = testComponentDom->value.FindMember("ReflectedType"); + ASSERT_NE(reflectedTypeDom, testComponentDom->value.MemberEnd()); + + // Confirm the reflected type has the correct type and value + ASSERT_TRUE(reflectedTypeDom->value.IsInt()); + EXPECT_EQ(reflectedTypeDom->value.GetInt(), newComponent->m_reflectedType); + } + TEST_F(PrefabInstanceToTemplateTests, PrefabUpdateTemplate_UpdateEntityOnInstance) { //create template with single entity diff --git a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestComponent.cpp b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestComponent.cpp index 8f7ee398a0..5d29b179c4 100644 --- a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestComponent.cpp +++ b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestComponent.cpp @@ -28,4 +28,18 @@ namespace UnitTest : m_boolProperty(boolProperty) { } + + void PrefabTestComponentWithUnReflectedTypeMember::Reflect(AZ::ReflectContext* reflection) + { + AZ::SerializeContext* serializeContext = AZ::RttiCast(reflection); + + // We reflect our member but not its type this will result in missing reflection data + // when we try to store or load this field + if (serializeContext) + { + serializeContext->Class() + ->Field("UnReflectedType", &PrefabTestComponentWithUnReflectedTypeMember::m_unReflectedType) + ->Field("ReflectedType", &PrefabTestComponentWithUnReflectedTypeMember::m_reflectedType); + } + } } diff --git a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestComponent.h b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestComponent.h index 0ed8f6d9c9..e0e986b7db 100644 --- a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestComponent.h +++ b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestComponent.h @@ -27,4 +27,22 @@ namespace UnitTest int m_intProperty = 0; AZ::EntityId m_entityIdProperty; }; + + class UnReflectedType + { + public: + AZ_TYPE_INFO(UnReflectedType, "{FB65262C-CE9A-45CA-99EB-4DDCB19B32DB}"); + int m_unReflectedInt = 42; + }; + + class PrefabTestComponentWithUnReflectedTypeMember + : public AzToolsFramework::Components::EditorComponentBase + { + public: + AZ_EDITOR_COMPONENT(PrefabTestComponentWithUnReflectedTypeMember, "{726281E1-8E47-46AB-8018-D3F4BA823D74}"); + static void Reflect(AZ::ReflectContext* reflection); + + UnReflectedType m_unReflectedType; + int m_reflectedType = 52; + }; } diff --git a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestFixture.cpp b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestFixture.cpp index ded88d2da3..ed30de09c4 100644 --- a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestFixture.cpp +++ b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabTestFixture.cpp @@ -49,6 +49,7 @@ namespace UnitTest EXPECT_TRUE(m_instanceToTemplateInterface); GetApplication()->RegisterComponentDescriptor(PrefabTestComponent::CreateDescriptor()); + GetApplication()->RegisterComponentDescriptor(PrefabTestComponentWithUnReflectedTypeMember::CreateDescriptor()); } AZStd::unique_ptr PrefabTestFixture::CreateTestApplication() diff --git a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabUpdateWithPatchesTests.cpp b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabUpdateWithPatchesTests.cpp index 72e88c6336..c17763f778 100644 --- a/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabUpdateWithPatchesTests.cpp +++ b/Code/Framework/AzToolsFramework/Tests/Prefab/PrefabUpdateWithPatchesTests.cpp @@ -136,7 +136,10 @@ namespace UnitTest PrefabDomValueReference wheelEntityComponentBoolPropertyValue = PrefabDomUtils::FindPrefabDomValue(wheelEntityComponentValue->get(), PrefabTestDomUtils::BoolPropertyName); - ASSERT_FALSE(wheelEntityComponentBoolPropertyValue.has_value()); + + ASSERT_TRUE(wheelEntityComponentBoolPropertyValue.has_value()); + ASSERT_TRUE(wheelEntityComponentBoolPropertyValue->get().IsBool()); + ASSERT_FALSE(wheelEntityComponentBoolPropertyValue->get().GetBool()); // Validate that the axles under the car have the same DOM as the axle template. PrefabTestDomUtils::ValidatePrefabDomInstances(axleInstanceAliasesUnderCar, carTemplateDom, axleTemplateDom); diff --git a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp index d49f6901f3..acfec5eb38 100644 --- a/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp +++ b/Gems/Multiplayer/Code/Source/Pipeline/NetworkPrefabProcessor.cpp @@ -56,7 +56,7 @@ namespace Multiplayer // convert Prefab DOM into Prefab Instance. AZStd::unique_ptr sourceInstance(aznew Instance()); - if (!PrefabDomUtils::LoadInstanceFromPrefabDom(*sourceInstance, prefab, PrefabDomUtils::LoadInstanceFlags::AssignRandomEntityId)) + if (!PrefabDomUtils::LoadInstanceFromPrefabDom(*sourceInstance, prefab, PrefabDomUtils::LoadFlags::AssignRandomEntityId)) { PrefabDomValueConstReference sourceReference = PrefabDomUtils::FindPrefabDomValue(prefab, PrefabDomUtils::SourceName); From 6d1bb8e439eb2b0b8aeaebd45e69115b156275f7 Mon Sep 17 00:00:00 2001 From: jromnoa <80134229+jromnoa@users.noreply.github.com> Date: Wed, 11 Aug 2021 11:19:22 -0700 Subject: [PATCH 6/7] Add Light component GPU (screenshot) test to AutomatedTesting nightly o3de runs. (#2923) * adds the Light component GPU screenshot test to AutomatedTesting Signed-off-by: jromnoa * add LIGHT_TYPE_PROPERTY constant to test script since it is re-used multiple times Signed-off-by: jromnoa * removes redundant f strings, adds LIGHT_COMPONENT and LIGHT_TYPE_PROPERTY constants for re-use, removed formattng errors, increase CMakeLists.txt timeout, add attach_component_to_entity() to hydra_editor_utils.py Signed-off-by: jromnoa * fix ImportError Signed-off-by: jromnoa * moves the LIGHT_TYPES constant to a new file since scripts rely on it that do not have the Editor launched (can't see hydra bindings so failes with ModuleNotFound error) Signed-off-by: jromnoa --- .../hydra_editor_utils.py | 30 ++ .../PythonTests/atom_renderer/CMakeLists.txt | 2 +- ...dra_AtomEditorComponents_LightComponent.py | 2 +- .../hydra_GPUTest_LightComponent.py | 268 ++++++++++++++++++ .../atom_utils/atom_component_helper.py | 191 ++++++++++++- .../atom_utils/atom_constants.py | 19 ++ .../atom_utils/screenshot_utils.py | 17 ++ .../golden_images/AreaLight_1.ppm | 3 + .../golden_images/AreaLight_2.ppm | 3 + .../golden_images/AreaLight_3.ppm | 3 + .../golden_images/AreaLight_4.ppm | 3 + .../golden_images/AreaLight_5.ppm | 3 + .../golden_images/SpotLight_1.ppm | 3 + .../golden_images/SpotLight_2.ppm | 3 + .../golden_images/SpotLight_3.ppm | 3 + .../golden_images/SpotLight_4.ppm | 3 + .../golden_images/SpotLight_5.ppm | 3 + .../golden_images/SpotLight_6.ppm | 3 + .../atom_renderer/test_Atom_GPUTests.py | 61 +++- .../atom_renderer/test_Atom_MainSuite.py | 2 +- 20 files changed, 607 insertions(+), 18 deletions(-) create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/hydra_GPUTest_LightComponent.py create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/atom_constants.py create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_1.ppm create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_2.ppm create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_3.ppm create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_4.ppm create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_5.ppm create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_1.ppm create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_2.ppm create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_3.ppm create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_4.ppm create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_5.ppm create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_6.ppm diff --git a/AutomatedTesting/Gem/PythonTests/EditorPythonTestTools/editor_python_test_tools/hydra_editor_utils.py b/AutomatedTesting/Gem/PythonTests/EditorPythonTestTools/editor_python_test_tools/hydra_editor_utils.py index cec1b6456b..985e32ede5 100644 --- a/AutomatedTesting/Gem/PythonTests/EditorPythonTestTools/editor_python_test_tools/hydra_editor_utils.py +++ b/AutomatedTesting/Gem/PythonTests/EditorPythonTestTools/editor_python_test_tools/hydra_editor_utils.py @@ -8,6 +8,7 @@ SPDX-License-Identifier: Apache-2.0 OR MIT import azlmbr.bus as bus import azlmbr.editor as editor import azlmbr.entity as entity +import azlmbr.legacy.general as general import azlmbr.object from typing import List @@ -428,3 +429,32 @@ def get_component_type_id_map(component_name_list): type_ids_by_component[component_names[i]] = typeId return type_ids_by_component + + +def attach_component_to_entity(entity_id, component_name): + # type: (azlmbr.entity.EntityId, str) -> azlmbr.entity.EntityComponentIdPair + """ + Adds the component if not added already. + :param entity_id: EntityId of the entity to attach the component to + :param component_name: name of the component + :return: If successful, returns the EntityComponentIdPair, otherwise returns None. + """ + type_ids_list = editor.EditorComponentAPIBus( + bus.Broadcast, 'FindComponentTypeIdsByEntityType', [component_name], 0) + general.log(f"Components found = {len(type_ids_list)}") + if len(type_ids_list) < 1: + general.log(f"ERROR: A component class with name {component_name} doesn't exist") + return None + elif len(type_ids_list) > 1: + general.log(f"ERROR: Found more than one component classes with same name: {component_name}") + return None + # Before adding the component let's check if it is already attached to the entity. + component_outcome = editor.EditorComponentAPIBus(bus.Broadcast, 'GetComponentOfType', entity_id, type_ids_list[0]) + if component_outcome.IsSuccess(): + return component_outcome.GetValue() # In this case the value is not a list. + component_outcome = editor.EditorComponentAPIBus(bus.Broadcast, 'AddComponentsOfType', entity_id, type_ids_list) + if component_outcome.IsSuccess(): + general.log(f"{component_name} Component added to entity.") + return component_outcome.GetValue()[0] + general.log(f"ERROR: Failed to add component [{component_name}] to entity") + return None diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/CMakeLists.txt b/AutomatedTesting/Gem/PythonTests/atom_renderer/CMakeLists.txt index d4f036faeb..f056623ecd 100644 --- a/AutomatedTesting/Gem/PythonTests/atom_renderer/CMakeLists.txt +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/CMakeLists.txt @@ -39,7 +39,7 @@ if(PAL_TRAIT_BUILD_HOST_TOOLS AND PAL_TRAIT_BUILD_TESTS_SUPPORTED AND AutomatedT TEST_SUITE main TEST_REQUIRES gpu TEST_SERIAL - TIMEOUT 800 + TIMEOUT 1200 PATH ${CMAKE_CURRENT_LIST_DIR}/test_Atom_GPUTests.py RUNTIME_DEPENDENCIES AssetProcessor diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/hydra_AtomEditorComponents_LightComponent.py b/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/hydra_AtomEditorComponents_LightComponent.py index ec8dc199ae..24866f3b19 100644 --- a/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/hydra_AtomEditorComponents_LightComponent.py +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/hydra_AtomEditorComponents_LightComponent.py @@ -19,7 +19,7 @@ import azlmbr.legacy.general as general sys.path.append(os.path.join(azlmbr.paths.devassets, "Gem", "PythonTests")) import editor_python_test_tools.hydra_editor_utils as hydra -from atom_renderer.atom_utils.atom_component_helper import LIGHT_TYPES +from atom_renderer.atom_utils.atom_constants import LIGHT_TYPES LIGHT_TYPE_PROPERTY = 'Controller|Configuration|Light type' SPHERE_AND_SPOT_DISK_LIGHT_PROPERTIES = [ diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/hydra_GPUTest_LightComponent.py b/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/hydra_GPUTest_LightComponent.py new file mode 100644 index 0000000000..08f921e68b --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/hydra_GPUTest_LightComponent.py @@ -0,0 +1,268 @@ +""" +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 + +Hydra script that is used to create an entity with a Light component attached. +It then updates the property values of the Light component and takes a screenshot. +The screenshot is compared against an expected golden image for test verification. + +See the run() function for more in-depth test info. +""" +import os +import sys + +import azlmbr.asset as asset +import azlmbr.bus as bus +import azlmbr.editor as editor +import azlmbr.math as math +import azlmbr.paths +import azlmbr.legacy.general as general + +sys.path.append(os.path.join(azlmbr.paths.devroot, "AutomatedTesting", "Gem", "PythonTests")) + +import editor_python_test_tools.hydra_editor_utils as hydra +from atom_renderer.atom_utils import screenshot_utils +from atom_renderer.atom_utils import atom_component_helper +from editor_python_test_tools.editor_test_helper import EditorTestHelper + +helper = EditorTestHelper(log_prefix="Atom_EditorTestHelper") + +LEVEL_NAME = "auto_test" +LIGHT_COMPONENT = "Light" +LIGHT_TYPE_PROPERTY = 'Controller|Configuration|Light type' +DEGREE_RADIAN_FACTOR = 0.0174533 + + +def run(): + """ + Sets up the tests by making sure the required level is created & setup correctly. + It then executes 2 test cases - see each associated test function's docstring for more info. + + Finally prints the string "Light component tests completed" after completion + + Tests will fail immediately if any of these log lines are found: + 1. Trace::Assert + 2. Trace::Error + 3. Traceback (most recent call last): + + :return: None + """ + atom_component_helper.create_basic_atom_level(level_name=LEVEL_NAME) + + # Run tests. + area_light_test() + spot_light_test() + general.log("Light component tests completed.") + + +def area_light_test(): + """ + Basic test for the "Light" component attached to an "area_light" entity. + + Test Case - Light Component: Capsule, Spot (disk), and Point (sphere): + 1. Creates "area_light" entity w/ a Light component that has a Capsule Light type w/ the color set to 255, 0, 0 + 2. Enters game mode to take a screenshot for comparison, then exits game mode. + 3. Sets the Light component Intensity Mode to Lumens (default). + 4. Ensures the Light component Mode is Automatic (default). + 5. Sets the Intensity value of the Light component to 0.0 + 6. Enters game mode again, takes another screenshot for comparison, then exits game mode. + 7. Updates the Intensity value of the Light component to 1000.0 + 8. Enters game mode again, takes another screenshot for comparison, then exits game mode. + 9. Swaps the Capsule light type option to Spot (disk) light type on the Light component + 10. Updates "area_light" entity Transform rotate value to x: 90.0, y:0.0, z:0.0 + 11. Enters game mode again, takes another screenshot for comparison, then exits game mode. + 12. Swaps the Spot (disk) light type for the Point (sphere) light type in the Light component. + 13. Enters game mode again, takes another screenshot for comparison, then exits game mode. + 14. Deletes the Light component from the "area_light" entity and verifies its successful. + """ + # Create an "area_light" entity with "Light" component using Light type of "Capsule" + area_light_entity_name = "area_light" + area_light = hydra.Entity(area_light_entity_name) + area_light.create_entity(math.Vector3(-1.0, -2.0, 3.0), [LIGHT_COMPONENT]) + general.log( + f"{area_light_entity_name}_test: Component added to the entity: " + f"{hydra.has_components(area_light.id, [LIGHT_COMPONENT])}") + light_component_id_pair = hydra.attach_component_to_entity(area_light.id, LIGHT_COMPONENT) + + # Select the "Capsule" light type option. + azlmbr.editor.EditorComponentAPIBus( + azlmbr.bus.Broadcast, + 'SetComponentProperty', + light_component_id_pair, + LIGHT_TYPE_PROPERTY, + atom_component_helper.LIGHT_TYPES['capsule'] + ) + + # Update color and take screenshot in game mode + color = math.Color(255.0, 0.0, 0.0, 0.0) + area_light.get_set_test(0, "Controller|Configuration|Color", color) + general.idle_wait(1.0) + screenshot_utils.take_screenshot_game_mode("AreaLight_1", area_light_entity_name) + + # Update intensity value to 0.0 and take screenshot in game mode + area_light.get_set_test(0, "Controller|Configuration|Attenuation Radius|Mode", 1) + area_light.get_set_test(0, "Controller|Configuration|Intensity", 0.0) + general.idle_wait(1.0) + screenshot_utils.take_screenshot_game_mode("AreaLight_2", area_light_entity_name) + + # Update intensity value to 1000.0 and take screenshot in game mode + area_light.get_set_test(0, "Controller|Configuration|Intensity", 1000.0) + general.idle_wait(1.0) + screenshot_utils.take_screenshot_game_mode("AreaLight_3", area_light_entity_name) + + # Swap the "Capsule" light type option to "Spot (disk)" light type + azlmbr.editor.EditorComponentAPIBus( + azlmbr.bus.Broadcast, + 'SetComponentProperty', + light_component_id_pair, + LIGHT_TYPE_PROPERTY, + atom_component_helper.LIGHT_TYPES['spot_disk'] + ) + area_light_rotation = math.Vector3(DEGREE_RADIAN_FACTOR * 90.0, 0.0, 0.0) + azlmbr.components.TransformBus(azlmbr.bus.Event, "SetLocalRotation", area_light.id, area_light_rotation) + general.idle_wait(1.0) + screenshot_utils.take_screenshot_game_mode("AreaLight_4", area_light_entity_name) + + # Swap the "Spot (disk)" light type to the "Point (sphere)" light type and take screenshot. + azlmbr.editor.EditorComponentAPIBus( + azlmbr.bus.Broadcast, + 'SetComponentProperty', + light_component_id_pair, + LIGHT_TYPE_PROPERTY, + atom_component_helper.LIGHT_TYPES['sphere'] + ) + general.idle_wait(1.0) + screenshot_utils.take_screenshot_game_mode("AreaLight_5", area_light_entity_name) + + editor.ToolsApplicationRequestBus(bus.Broadcast, "DeleteEntityById", area_light.id) + + +def spot_light_test(): + """ + Basic test for the Light component attached to a "spot_light" entity. + + Test Case - Light Component: Spot (disk) with shadows & colors: + 1. Creates "spot_light" entity w/ a Light component attached to it. + 2. Selects the "directional_light" entity already present in the level and disables it. + 3. Selects the "global_skylight" entity already present in the level and disables the HDRi Skybox component, + as well as the Global Skylight (IBL) component. + 4. Enters game mode to take a screenshot for comparison, then exits game mode. + 5. Selects the "ground_plane" entity and changes updates the material to a new material. + 6. Enters game mode to take a screenshot for comparison, then exits game mode. + 7. Selects the "spot_light" entity and increases the Light component Intensity to 800 lm + 8. Enters game mode to take a screenshot for comparison, then exits game mode. + 9. Selects the "spot_light" entity and sets the Light component Color to 47, 75, 37 + 10. Enters game mode to take a screenshot for comparison, then exits game mode. + 11. Selects the "spot_light" entity and modifies the Shutter controls to the following values: + - Enable shutters: True + - Inner Angle: 60.0 + - Outer Angle: 75.0 + 12. Enters game mode to take a screenshot for comparison, then exits game mode. + 13. Selects the "spot_light" entity and modifies the Shadow controls to the following values: + - Enable Shadow: True + - ShadowmapSize: 256 + 14. Modifies the world translate position of the "spot_light" entity to 0.7, -2.0, 1.9 (for casting shadows better) + 15. Enters game mode to take a screenshot for comparison, then exits game mode. + """ + # Disable "Directional Light" component for the "directional_light" entity + # "directional_light" entity is created by the create_basic_atom_level() function by default. + directional_light_entity_id = hydra.find_entity_by_name("directional_light") + directional_light = hydra.Entity(name='directional_light', id=directional_light_entity_id) + directional_light_component_type = azlmbr.editor.EditorComponentAPIBus( + azlmbr.bus.Broadcast, 'FindComponentTypeIdsByEntityType', ["Directional Light"], 0)[0] + directional_light_component = azlmbr.editor.EditorComponentAPIBus( + azlmbr.bus.Broadcast, 'GetComponentOfType', directional_light.id, directional_light_component_type + ).GetValue() + editor.EditorComponentAPIBus(bus.Broadcast, "DisableComponents", [directional_light_component]) + general.idle_wait(0.5) + + # Disable "Global Skylight (IBL)" and "HDRi Skybox" components for the "global_skylight" entity + global_skylight_entity_id = hydra.find_entity_by_name("global_skylight") + global_skylight = hydra.Entity(name='global_skylight', id=global_skylight_entity_id) + global_skylight_component_type = azlmbr.editor.EditorComponentAPIBus( + azlmbr.bus.Broadcast, 'FindComponentTypeIdsByEntityType', ["Global Skylight (IBL)"], 0)[0] + global_skylight_component = azlmbr.editor.EditorComponentAPIBus( + azlmbr.bus.Broadcast, 'GetComponentOfType', global_skylight.id, global_skylight_component_type + ).GetValue() + editor.EditorComponentAPIBus(bus.Broadcast, "DisableComponents", [global_skylight_component]) + hdri_skybox_component_type = azlmbr.editor.EditorComponentAPIBus( + azlmbr.bus.Broadcast, 'FindComponentTypeIdsByEntityType', ["HDRi Skybox"], 0)[0] + hdri_skybox_component = azlmbr.editor.EditorComponentAPIBus( + azlmbr.bus.Broadcast, 'GetComponentOfType', global_skylight.id, hdri_skybox_component_type + ).GetValue() + editor.EditorComponentAPIBus(bus.Broadcast, "DisableComponents", [hdri_skybox_component]) + general.idle_wait(0.5) + + # Create a "spot_light" entity with "Light" component using Light Type of "Spot (disk)" + spot_light_entity_name = "spot_light" + spot_light = hydra.Entity(spot_light_entity_name) + spot_light.create_entity(math.Vector3(0.7, -2.0, 1.0), [LIGHT_COMPONENT]) + general.log( + f"{spot_light_entity_name}_test: Component added to the entity: " + f"{hydra.has_components(spot_light.id, [LIGHT_COMPONENT])}") + rotation = math.Vector3(DEGREE_RADIAN_FACTOR * 300.0, 0.0, 0.0) + azlmbr.components.TransformBus(azlmbr.bus.Event, "SetLocalRotation", spot_light.id, rotation) + light_component_type = hydra.attach_component_to_entity(spot_light.id, LIGHT_COMPONENT) + editor.EditorComponentAPIBus( + azlmbr.bus.Broadcast, + 'SetComponentProperty', + light_component_type, + LIGHT_TYPE_PROPERTY, + atom_component_helper.LIGHT_TYPES['spot_disk'] + ) + + general.idle_wait(1.0) + screenshot_utils.take_screenshot_game_mode("SpotLight_1", spot_light_entity_name) + + # Change default material of ground plane entity and take screenshot + ground_plane_entity_id = hydra.find_entity_by_name("ground_plane") + ground_plane = hydra.Entity(name='ground_plane', id=ground_plane_entity_id) + ground_plane_asset_path = os.path.join("Materials", "Presets", "MacBeth", "22_neutral_5-0_0-70d.azmaterial") + ground_plane_asset_value = asset.AssetCatalogRequestBus( + bus.Broadcast, "GetAssetIdByPath", ground_plane_asset_path, math.Uuid(), False) + material_property_path = "Default Material|Material Asset" + material_component_type = azlmbr.editor.EditorComponentAPIBus( + azlmbr.bus.Broadcast, 'FindComponentTypeIdsByEntityType', ["Material"], 0)[0] + material_component = azlmbr.editor.EditorComponentAPIBus( + azlmbr.bus.Broadcast, 'GetComponentOfType', ground_plane.id, material_component_type).GetValue() + editor.EditorComponentAPIBus( + azlmbr.bus.Broadcast, + 'SetComponentProperty', + material_component, + material_property_path, + ground_plane_asset_value + ) + general.idle_wait(1.0) + screenshot_utils.take_screenshot_game_mode("SpotLight_2", spot_light_entity_name) + + # Increase intensity value of the Spot light and take screenshot in game mode + spot_light.get_set_test(0, "Controller|Configuration|Intensity", 800.0) + general.idle_wait(1.0) + screenshot_utils.take_screenshot_game_mode("SpotLight_3", spot_light_entity_name) + + # Update the Spot light color and take screenshot in game mode + color_value = math.Color(47.0 / 255.0, 75.0 / 255.0, 37.0 / 255.0, 255.0 / 255.0) + spot_light.get_set_test(0, "Controller|Configuration|Color", color_value) + general.idle_wait(1.0) + screenshot_utils.take_screenshot_game_mode("SpotLight_4", spot_light_entity_name) + + # Update the Shutter controls of the Light component and take screenshot + spot_light.get_set_test(0, "Controller|Configuration|Shutters|Enable shutters", True) + spot_light.get_set_test(0, "Controller|Configuration|Shutters|Inner angle", 60.0) + spot_light.get_set_test(0, "Controller|Configuration|Shutters|Outer angle", 75.0) + general.idle_wait(1.0) + screenshot_utils.take_screenshot_game_mode("SpotLight_5", spot_light_entity_name) + + # Update the Shadow controls, move the spot_light entity world translate position and take screenshot + spot_light.get_set_test(0, "Controller|Configuration|Shadows|Enable shadow", True) + spot_light.get_set_test(0, "Controller|Configuration|Shadows|Shadowmap size", 256.0) + azlmbr.components.TransformBus( + azlmbr.bus.Event, "SetWorldTranslation", spot_light.id, math.Vector3(0.7, -2.0, 1.9)) + general.idle_wait(1.0) + screenshot_utils.take_screenshot_game_mode("SpotLight_6", spot_light_entity_name) + + +if __name__ == "__main__": + run() diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/atom_component_helper.py b/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/atom_component_helper.py index de4e28bb36..58b72ef01a 100644 --- a/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/atom_component_helper.py +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/atom_component_helper.py @@ -3,17 +3,184 @@ Copyright (c) Contributors to the Open 3D Engine Project. For complete copyright SPDX-License-Identifier: Apache-2.0 OR MIT -File to assist with common hydra component functions or constants used across various Atom tests. +File to assist with common hydra component functions used across various Atom tests. """ +import os -# Light type options for the Light component. -LIGHT_TYPES = { - 'unknown': 0, - 'sphere': 1, - 'spot_disk': 2, - 'capsule': 3, - 'quad': 4, - 'polygon': 5, - 'simple_point': 6, - 'simple_spot': 7, -} +from editor_python_test_tools.editor_test_helper import EditorTestHelper + +helper = EditorTestHelper(log_prefix="Atom_EditorTestHelper") + + +def create_basic_atom_level(level_name): + """ + Creates a new level inside the Editor matching level_name & adds the following: + 1. "default_level" entity to hold all other entities. + 2. Adds Grid, Global Skylight (IBL), ground Mesh, Directional Light, Sphere w/ material+mesh, & Camera components. + 3. Each of these components has its settings tweaked slightly to match the ideal scene to test Atom rendering. + :param level_name: name of the level to create and apply this basic setup to. + :return: None + """ + import azlmbr.asset as asset + import azlmbr.bus as bus + import azlmbr.camera as camera + import azlmbr.editor as editor + import azlmbr.entity as entity + import azlmbr.legacy.general as general + import azlmbr.math as math + import azlmbr.object + + import editor_python_test_tools.hydra_editor_utils as hydra + + # Create a new level. + new_level_name = level_name + heightmap_resolution = 512 + heightmap_meters_per_pixel = 1 + terrain_texture_resolution = 412 + use_terrain = False + + # Return codes are ECreateLevelResult defined in CryEdit.h + return_code = general.create_level_no_prompt( + new_level_name, heightmap_resolution, heightmap_meters_per_pixel, terrain_texture_resolution, use_terrain) + if return_code == 1: + general.log(f"{new_level_name} level already exists") + elif return_code == 2: + general.log("Failed to create directory") + elif return_code == 3: + general.log("Directory length is too long") + elif return_code != 0: + general.log("Unknown error, failed to create level") + else: + general.log(f"{new_level_name} level created successfully") + + # Enable idle and update viewport. + general.idle_enable(True) + general.idle_wait(1.0) + general.update_viewport() + general.idle_wait(0.5) # half a second is more than enough for updating the viewport. + + # Close out problematic windows, FPS meters, and anti-aliasing. + if general.is_helpers_shown(): # Turn off the helper gizmos if visible + general.toggle_helpers() + general.idle_wait(1.0) + if general.is_pane_visible("Error Report"): # Close Error Report windows that block focus. + general.close_pane("Error Report") + if general.is_pane_visible("Error Log"): # Close Error Log windows that block focus. + general.close_pane("Error Log") + general.idle_wait(1.0) + general.run_console("r_displayInfo=0") + general.run_console("r_antialiasingmode=0") + general.idle_wait(1.0) + + # Delete all existing entities & create default_level entity + search_filter = azlmbr.entity.SearchFilter() + all_entities = entity.SearchBus(azlmbr.bus.Broadcast, "SearchEntities", search_filter) + editor.ToolsApplicationRequestBus(bus.Broadcast, "DeleteEntities", all_entities) + default_level = hydra.Entity("default_level") + default_position = math.Vector3(0.0, 0.0, 0.0) + default_level.create_entity(default_position, ["Grid"]) + default_level.get_set_test(0, "Controller|Configuration|Secondary Grid Spacing", 1.0) + + # Set the viewport up correctly after adding the parent default_level entity. + screen_width = 1280 + screen_height = 720 + degree_radian_factor = 0.0174533 # Used by "Rotation" property for the Transform component. + general.set_viewport_size(screen_width, screen_height) + general.update_viewport() + helper.wait_for_condition( + function=lambda: helper.isclose(a=general.get_viewport_size().x, b=screen_width, rel_tol=0.1) + and helper.isclose(a=general.get_viewport_size().y, b=screen_height, rel_tol=0.1), + timeout_in_seconds=4.0 + ) + result = helper.isclose(a=general.get_viewport_size().x, b=screen_width, rel_tol=0.1) and helper.isclose( + a=general.get_viewport_size().y, b=screen_height, rel_tol=0.1) + general.log(general.get_viewport_size().x) + general.log(general.get_viewport_size().y) + general.log(general.get_viewport_size().z) + general.log(f"Viewport is set to the expected size: {result}") + general.log("Basic level created") + general.run_console("r_DisplayInfo = 0") + + # Create global_skylight entity and set the properties + global_skylight = hydra.Entity("global_skylight") + global_skylight.create_entity( + entity_position=default_position, + components=["HDRi Skybox", "Global Skylight (IBL)"], + parent_id=default_level.id) + global_skylight_asset_path = os.path.join( + "LightingPresets", "greenwich_park_02_4k_iblskyboxcm_iblspecular.exr.streamingimage") + global_skylight_asset_value = asset.AssetCatalogRequestBus( + bus.Broadcast, "GetAssetIdByPath", global_skylight_asset_path, math.Uuid(), False) + global_skylight.get_set_test(0, "Controller|Configuration|Cubemap Texture", global_skylight_asset_value) + global_skylight.get_set_test(1, "Controller|Configuration|Diffuse Image", global_skylight_asset_value) + global_skylight.get_set_test(1, "Controller|Configuration|Specular Image", global_skylight_asset_value) + + # Create ground_plane entity and set the properties + ground_plane = hydra.Entity("ground_plane") + ground_plane.create_entity( + entity_position=default_position, + components=["Material"], + parent_id=default_level.id) + azlmbr.components.TransformBus(azlmbr.bus.Event, "SetLocalUniformScale", ground_plane.id, 32.0) + ground_plane_material_asset_path = os.path.join( + "Materials", "Presets", "PBR", "metal_chrome.azmaterial") + ground_plane_material_asset_value = asset.AssetCatalogRequestBus( + bus.Broadcast, "GetAssetIdByPath", ground_plane_material_asset_path, math.Uuid(), False) + ground_plane.get_set_test(0, "Default Material|Material Asset", ground_plane_material_asset_value) + + # Work around to add the correct Atom Mesh component + mesh_type_id = azlmbr.globals.property.EditorMeshComponentTypeId + ground_plane.components.append( + editor.EditorComponentAPIBus( + bus.Broadcast, "AddComponentsOfType", ground_plane.id, [mesh_type_id] + ).GetValue()[0] + ) + ground_plane_mesh_asset_path = os.path.join("Models", "plane.azmodel") + ground_plane_mesh_asset_value = asset.AssetCatalogRequestBus( + bus.Broadcast, "GetAssetIdByPath", ground_plane_mesh_asset_path, math.Uuid(), False) + ground_plane.get_set_test(1, "Controller|Configuration|Mesh Asset", ground_plane_mesh_asset_value) + + # Create directional_light entity and set the properties + directional_light = hydra.Entity("directional_light") + directional_light.create_entity( + entity_position=math.Vector3(0.0, 0.0, 10.0), + components=["Directional Light"], + parent_id=default_level.id) + directional_light_rotation = math.Vector3(degree_radian_factor * -90.0, 0.0, 0.0) + azlmbr.components.TransformBus( + azlmbr.bus.Event, "SetLocalRotation", directional_light.id, directional_light_rotation) + + # Create sphere entity and set the properties + sphere_entity = hydra.Entity("sphere") + sphere_entity.create_entity( + entity_position=math.Vector3(0.0, 0.0, 1.0), + components=["Material"], + parent_id=default_level.id) + sphere_material_asset_path = os.path.join("Materials", "Presets", "PBR", "metal_brass_polished.azmaterial") + sphere_material_asset_value = asset.AssetCatalogRequestBus( + bus.Broadcast, "GetAssetIdByPath", sphere_material_asset_path, math.Uuid(), False) + sphere_entity.get_set_test(0, "Default Material|Material Asset", sphere_material_asset_value) + + # Work around to add the correct Atom Mesh component + sphere_entity.components.append( + editor.EditorComponentAPIBus( + bus.Broadcast, "AddComponentsOfType", sphere_entity.id, [mesh_type_id] + ).GetValue()[0] + ) + sphere_mesh_asset_path = os.path.join("Models", "sphere.azmodel") + sphere_mesh_asset_value = asset.AssetCatalogRequestBus( + bus.Broadcast, "GetAssetIdByPath", sphere_mesh_asset_path, math.Uuid(), False) + sphere_entity.get_set_test(1, "Controller|Configuration|Mesh Asset", sphere_mesh_asset_value) + + # Create camera component and set the properties + camera_entity = hydra.Entity("camera") + camera_entity.create_entity( + entity_position=math.Vector3(5.5, -12.0, 9.0), + components=["Camera"], + parent_id=default_level.id) + rotation = math.Vector3( + degree_radian_factor * -27.0, degree_radian_factor * -12.0, degree_radian_factor * 25.0 + ) + azlmbr.components.TransformBus(azlmbr.bus.Event, "SetLocalRotation", camera_entity.id, rotation) + camera_entity.get_set_test(0, "Controller|Configuration|Field of view", 60.0) + camera.EditorCameraViewRequestBus(azlmbr.bus.Event, "ToggleCameraAsActiveView", camera_entity.id) diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/atom_constants.py b/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/atom_constants.py new file mode 100644 index 0000000000..88a959f198 --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/atom_constants.py @@ -0,0 +1,19 @@ +""" +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 + +Hold constants used across both hydra and non-hydra scripts. +""" + +# Light type options for the Light component. +LIGHT_TYPES = { + 'unknown': 0, + 'sphere': 1, + 'spot_disk': 2, + 'capsule': 3, + 'quad': 4, + 'polygon': 5, + 'simple_point': 6, + 'simple_spot': 7, +} diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/screenshot_utils.py b/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/screenshot_utils.py index 28a4037dcc..a7a02816ac 100644 --- a/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/screenshot_utils.py +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/screenshot_utils.py @@ -91,3 +91,20 @@ class ScreenshotHelper(object): else: frames_waited = frames_waited + 1 general.log(f"(waited {frames_waited} frames)") + + +def take_screenshot_game_mode(screenshot_name, entity_name=None): + """ + Enters game mode & takes a screenshot, then exits game mode after. + :param screenshot_name: name to give the captured screenshot .ppm file. + :param entity_name: name of the entity being tested (for generating unique log lines). + :return: None + """ + general.enter_game_mode() + helper.wait_for_condition(lambda: general.is_in_game_mode(), 2.0) + general.log(f"{entity_name}_test: Entered game mode: {general.is_in_game_mode()}") + ScreenshotHelper(general.idle_wait_frames).capture_screenshot_blocking(f"{screenshot_name}.ppm") + general.idle_wait(1.0) + general.exit_game_mode() + helper.wait_for_condition(lambda: not general.is_in_game_mode(), 2.0) + general.log(f"{entity_name}_test: Exit game mode: {not general.is_in_game_mode()}") diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_1.ppm b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_1.ppm new file mode 100644 index 0000000000..0725999dcf --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_1.ppm @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:954d7d0df47c840a24e313893800eb3126d0c0d47c3380926776b51833778db7 +size 6220817 diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_2.ppm b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_2.ppm new file mode 100644 index 0000000000..3a45bd31e3 --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_2.ppm @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:e81c19128f42ba362a2d5f3ccf159dfbc942d67ceeb1ac8c21f295a6fd9d2ce5 +size 6220817 diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_3.ppm b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_3.ppm new file mode 100644 index 0000000000..15d679b784 --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_3.ppm @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:5e20801213e065b6ea8c95ede81c23faa9b6dc70a2002dc5bced293e1bed989f +size 6220817 diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_4.ppm b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_4.ppm new file mode 100644 index 0000000000..85c083a386 --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_4.ppm @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:e250f812e594e5152bf2d6f23caa8b53b78276bfdf344d7a8d355dd96cb995c0 +size 6220817 diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_5.ppm b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_5.ppm new file mode 100644 index 0000000000..d575de761e --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/AreaLight_5.ppm @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:95be359041f8291c74b335297a4dfe9902a180510f24a181b15e1a5ba4d3b024 +size 6220817 diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_1.ppm b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_1.ppm new file mode 100644 index 0000000000..bbbd127929 --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_1.ppm @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:118e43e4b915e262726183467cc4b82f244565213fea5b6bfe02be07f0851ab1 +size 6220817 diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_2.ppm b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_2.ppm new file mode 100644 index 0000000000..8e716fabcc --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_2.ppm @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:dc2ce3256a6552975962c9e113c52c1a22bf3817d417151f6f60640dd568e0fa +size 6220817 diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_3.ppm b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_3.ppm new file mode 100644 index 0000000000..6b6a5a5d6e --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_3.ppm @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:287d98890b35427688999760f9d066bcbff1a3bc9001534241dc212b32edabd8 +size 6220817 diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_4.ppm b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_4.ppm new file mode 100644 index 0000000000..eb05228cc2 --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_4.ppm @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:66e91c92c868167c850078cd91714db47e10a96e23cc30191994486bd79c353f +size 6220817 diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_5.ppm b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_5.ppm new file mode 100644 index 0000000000..5e12edc46d --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_5.ppm @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d950d173f5101820c5e18205401ca08ce5feeff2302ac2920b292750d86a8fa4 +size 6220817 diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_6.ppm b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_6.ppm new file mode 100644 index 0000000000..d442d90287 --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/golden_images/SpotLight_6.ppm @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:72eddb7126eae0c839b933886e0fb69d78229f72d49ef13199de28df2b7879db +size 6220817 diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/test_Atom_GPUTests.py b/AutomatedTesting/Gem/PythonTests/atom_renderer/test_Atom_GPUTests.py index 9165bced90..7be1ed1b12 100644 --- a/AutomatedTesting/Gem/PythonTests/atom_renderer/test_Atom_GPUTests.py +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/test_Atom_GPUTests.py @@ -15,11 +15,11 @@ import pytest import ly_test_tools.environment.file_system as file_system from ly_test_tools.image.screenshot_compare_qssim import qssim as compare_screenshots from ly_test_tools.benchmark.data_aggregator import BenchmarkDataAggregator + import editor_python_test_tools.hydra_test_utils as hydra logger = logging.getLogger(__name__) DEFAULT_SUBFOLDER_PATH = 'user/PythonTests/Automated/Screenshots' -EDITOR_TIMEOUT = 600 TEST_DIRECTORY = os.path.join(os.path.dirname(__file__), "atom_hydra_scripts") @@ -67,6 +67,7 @@ class TestAllComponentsIndepthTests(object): "Trace::Assert", "Trace::Error", "Traceback (most recent call last):", + "Screenshot failed" ] hydra.launch_and_validate_results( @@ -74,7 +75,7 @@ class TestAllComponentsIndepthTests(object): TEST_DIRECTORY, editor, "hydra_GPUTest_BasicLevelSetup.py", - timeout=EDITOR_TIMEOUT, + timeout=180, expected_lines=level_creation_expected_lines, unexpected_lines=unexpected_lines, halt_on_unexpected=True, @@ -85,6 +86,60 @@ class TestAllComponentsIndepthTests(object): for test_screenshot, golden_screenshot in zip(test_screenshots, golden_images): compare_screenshots(test_screenshot, golden_screenshot) + def test_LightComponent_ScreenshotMatchesGoldenImage( + self, request, editor, workspace, project, launcher_platform, level): + """ + Please review the hydra script run by this test for more specific test info. + Tests that the Light component screenshots in a rendered level appear the same as the golden images. + """ + screenshot_names = [ + "AreaLight_1.ppm", + "AreaLight_2.ppm", + "AreaLight_3.ppm", + "AreaLight_4.ppm", + "AreaLight_5.ppm", + "SpotLight_1.ppm", + "SpotLight_2.ppm", + "SpotLight_3.ppm", + "SpotLight_4.ppm", + "SpotLight_5.ppm", + "SpotLight_6.ppm", + ] + test_screenshots = [] + for screenshot in screenshot_names: + screenshot_path = os.path.join(workspace.paths.project(), DEFAULT_SUBFOLDER_PATH, screenshot) + test_screenshots.append(screenshot_path) + file_system.delete(test_screenshots, True, True) + + golden_images = [] + for golden_image in screenshot_names: + golden_image_path = os.path.join(golden_images_directory(), golden_image) + golden_images.append(golden_image_path) + + expected_lines = ["Light component tests completed."] + unexpected_lines = [ + "Trace::Assert", + "Trace::Error", + "Traceback (most recent call last):", + "Screenshot failed", + ] + hydra.launch_and_validate_results( + request, + TEST_DIRECTORY, + editor, + "hydra_GPUTest_LightComponent.py", + timeout=180, + expected_lines=expected_lines, + unexpected_lines=unexpected_lines, + halt_on_unexpected=True, + cfg_args=[level], + null_renderer=False, + ) + + for test_screenshot, golden_screenshot in zip(test_screenshots, golden_images): + compare_screenshots(test_screenshot, golden_screenshot) + + @pytest.mark.parametrize('rhi', ['dx12', 'vulkan']) @pytest.mark.parametrize("project", ["AutomatedTesting"]) @pytest.mark.parametrize("launcher_platform", ["windows_editor"]) @@ -116,7 +171,7 @@ class TestPerformanceBenchmarkSuite(object): TEST_DIRECTORY, editor, "hydra_GPUTest_AtomFeatureIntegrationBenchmark.py", - timeout=EDITOR_TIMEOUT, + timeout=600, expected_lines=expected_lines, unexpected_lines=unexpected_lines, halt_on_unexpected=True, diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/test_Atom_MainSuite.py b/AutomatedTesting/Gem/PythonTests/atom_renderer/test_Atom_MainSuite.py index 98d2ba0632..39689816b8 100644 --- a/AutomatedTesting/Gem/PythonTests/atom_renderer/test_Atom_MainSuite.py +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/test_Atom_MainSuite.py @@ -12,7 +12,7 @@ import os import pytest import editor_python_test_tools.hydra_test_utils as hydra -from atom_renderer.atom_utils.atom_component_helper import LIGHT_TYPES +from atom_renderer.atom_utils.atom_constants import LIGHT_TYPES logger = logging.getLogger(__name__) EDITOR_TIMEOUT = 120 From 2564e8f8dce6fc65804500dee166f72a574477f9 Mon Sep 17 00:00:00 2001 From: smurly Date: Wed, 11 Aug 2021 12:15:31 -0700 Subject: [PATCH 7/7] MaterialEditor BasicTests added to AutomatedTesting for AR (#3022) * MaterialEditor BasicTests added to AutomatedTesting for AR Signed-off-by: Scott Murray * launch_and_validate_results adding a waiter.wait_for to the log monitor so the log file exists Signed-off-by: Scott Murray --- .../AssetProcessorGamePlatformConfig.setreg | 19 ++ .../hydra_test_utils.py | 15 +- .../hydra_AtomMaterialEditor_BasicTests.py | 183 ++++++++++++ .../atom_utils/material_editor_utils.py | 274 ++++++++++++++++++ .../atom_renderer/test_Atom_MainSuite.py | 63 ++++ 5 files changed, 552 insertions(+), 2 deletions(-) create mode 100644 AutomatedTesting/AssetProcessorGamePlatformConfig.setreg create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/hydra_AtomMaterialEditor_BasicTests.py create mode 100644 AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/material_editor_utils.py diff --git a/AutomatedTesting/AssetProcessorGamePlatformConfig.setreg b/AutomatedTesting/AssetProcessorGamePlatformConfig.setreg new file mode 100644 index 0000000000..5457b3f1ca --- /dev/null +++ b/AutomatedTesting/AssetProcessorGamePlatformConfig.setreg @@ -0,0 +1,19 @@ +{ + "Amazon": { + "AssetProcessor": { + "Settings": { + "RC cgf": { + "ignore": true + }, + "RC fbx": { + "ignore": true + }, + "ScanFolder AtomTestData": { + "watch": "@ENGINEROOT@/Gems/Atom/TestData", + "recursive": 1, + "order": 1000 + } + } + } + } +} diff --git a/AutomatedTesting/Gem/PythonTests/EditorPythonTestTools/editor_python_test_tools/hydra_test_utils.py b/AutomatedTesting/Gem/PythonTests/EditorPythonTestTools/editor_python_test_tools/hydra_test_utils.py index 3004b9ec7d..3d4d9ea419 100644 --- a/AutomatedTesting/Gem/PythonTests/EditorPythonTestTools/editor_python_test_tools/hydra_test_utils.py +++ b/AutomatedTesting/Gem/PythonTests/EditorPythonTestTools/editor_python_test_tools/hydra_test_utils.py @@ -29,7 +29,7 @@ def teardown_editor(editor): def launch_and_validate_results(request, test_directory, editor, editor_script, expected_lines, unexpected_lines=[], halt_on_unexpected=False, run_python="--runpythontest", auto_test_mode=True, null_renderer=True, cfg_args=[], - timeout=300): + timeout=300, log_file_name="Editor.log"): """ Runs the Editor with the specified script, and monitors for expected log lines. :param request: Special fixture providing information of the requesting test function. @@ -44,6 +44,7 @@ def launch_and_validate_results(request, test_directory, editor, editor_script, :param null_renderer: Specifies the test does not require the renderer. Defaults to True. :param cfg_args: Additional arguments for CFG, such as LevelName. :param timeout: Length of time for test to run. Default is 60. + :param log_file_name: Name of the log file created by the editor. Defaults to 'Editor.log' """ test_case = os.path.join(test_directory, editor_script) request.addfinalizer(lambda: teardown_editor(editor)) @@ -58,7 +59,17 @@ def launch_and_validate_results(request, test_directory, editor, editor_script, with editor.start(): - editorlog_file = os.path.join(editor.workspace.paths.project_log(), 'Editor.log') + editorlog_file = os.path.join(editor.workspace.paths.project_log(), log_file_name) + + # Log monitor requires the file to exist. + logger.debug(f"Waiting until log file <{editorlog_file}> exists...") + waiter.wait_for( + lambda: os.path.exists(editorlog_file), + timeout=60, + exc=f"Log file '{editorlog_file}' was never created by another process.", + interval=1, + ) + logger.debug(f"Done! log file <{editorlog_file}> exists.") # Initialize the log monitor and set time to wait for log creation log_monitor = ly_test_tools.log.log_monitor.LogMonitor(launcher=editor, log_file_path=editorlog_file) diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/hydra_AtomMaterialEditor_BasicTests.py b/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/hydra_AtomMaterialEditor_BasicTests.py new file mode 100644 index 0000000000..b3c51ca912 --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_hydra_scripts/hydra_AtomMaterialEditor_BasicTests.py @@ -0,0 +1,183 @@ +""" +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 + +import azlmbr.materialeditor will fail with a ModuleNotFound error when using this script with Editor.exe +This is because azlmbr.materialeditor only binds to MaterialEditor.exe and not Editor.exe +You need to launch this script with MaterialEditor.exe in order for azlmbr.materialeditor to appear. +""" + +import os +import sys +import time + +import azlmbr.math as math +import azlmbr.paths + +sys.path.append(os.path.join(azlmbr.paths.devassets, "Gem", "PythonTests")) + +import atom_renderer.atom_utils.material_editor_utils as material_editor + +NEW_MATERIAL = "test_material.material" +NEW_MATERIAL_1 = "test_material_1.material" +NEW_MATERIAL_2 = "test_material_2.material" +TEST_MATERIAL_1 = "001_DefaultWhite.material" +TEST_MATERIAL_2 = "002_BaseColorLerp.material" +TEST_MATERIAL_3 = "003_MetalMatte.material" +TEST_DATA_PATH = os.path.join( + azlmbr.paths.devroot, "Gems", "Atom", "TestData", "TestData", "Materials", "StandardPbrTestCases" +) +MATERIAL_TYPE_PATH = os.path.join( + azlmbr.paths.devroot, "Gems", "Atom", "Feature", "Common", "Assets", + "Materials", "Types", "StandardPBR.materialtype", +) + + +def run(): + """ + Summary: + Material Editor basic tests including the below + 1. Opening an Existing Asset + 2. Creating a New Asset + 3. Closing Selected Material + 4. Closing All Materials + 5. Closing all but Selected Material + 6. Saving Material + 7. Saving as a New Material + 8. Saving as a Child Material + 9. Saving all Open Materials + + Expected Result: + All the above functions work as expected in Material Editor. + + :return: None + """ + + # 1) Test Case: Opening an Existing Asset + document_id = material_editor.open_material(MATERIAL_TYPE_PATH) + print(f"Material opened: {material_editor.is_open(document_id)}") + + # Verify if the test material exists initially + target_path = os.path.join(azlmbr.paths.devroot, "AutomatedTesting", "Materials", NEW_MATERIAL) + print(f"Test asset doesn't exist initially: {not os.path.exists(target_path)}") + + # 2) Test Case: Creating a New Material Using Existing One + material_editor.save_document_as_child(document_id, target_path) + material_editor.wait_for_condition(lambda: os.path.exists(target_path), 2.0) + print(f"New asset created: {os.path.exists(target_path)}") + + # Verify if the newly created document is open + new_document_id = material_editor.open_material(target_path) + material_editor.wait_for_condition(lambda: material_editor.is_open(new_document_id)) + print(f"New Material opened: {material_editor.is_open(new_document_id)}") + + # 3) Test Case: Closing Selected Material + print(f"Material closed: {material_editor.close_document(new_document_id)}") + + # Open materials initially + document1_id, document2_id, document3_id = ( + material_editor.open_material(os.path.join(TEST_DATA_PATH, material)) + for material in [TEST_MATERIAL_1, TEST_MATERIAL_2, TEST_MATERIAL_3] + ) + + # 4) Test Case: Closing All Materials + print(f"All documents closed: {material_editor.close_all_documents()}") + + # 5) Test Case: Closing all but Selected Material + document1_id, document2_id, document3_id = ( + material_editor.open_material(os.path.join(TEST_DATA_PATH, material)) + for material in [TEST_MATERIAL_1, TEST_MATERIAL_2, TEST_MATERIAL_3] + ) + result = material_editor.close_all_except_selected(document1_id) + print(f"Close All Except Selected worked as expected: {result and material_editor.is_open(document1_id)}") + + # 6) Test Case: Saving Material + document_id = material_editor.open_material(os.path.join(TEST_DATA_PATH, TEST_MATERIAL_1)) + property_name = azlmbr.name.Name("baseColor.color") + initial_color = material_editor.get_property(document_id, property_name) + # Assign new color to the material file and save the actual material + expected_color = math.Color(0.25, 0.25, 0.25, 1.0) + material_editor.set_property(document_id, property_name, expected_color) + material_editor.save_document(document_id) + + # 7) Test Case: Saving as a New Material + # Assign new color to the material file and save the document as copy + expected_color_1 = math.Color(0.5, 0.5, 0.5, 1.0) + material_editor.set_property(document_id, property_name, expected_color_1) + target_path_1 = os.path.join(azlmbr.paths.devroot, "AutomatedTesting", "Materials", NEW_MATERIAL_1) + material_editor.save_document_as_copy(document_id, target_path_1) + time.sleep(2.0) + + # 8) Test Case: Saving as a Child Material + # Assign new color to the material file save the document as child + expected_color_2 = math.Color(0.75, 0.75, 0.75, 1.0) + material_editor.set_property(document_id, property_name, expected_color_2) + target_path_2 = os.path.join(azlmbr.paths.devroot, "AutomatedTesting", "Materials", NEW_MATERIAL_2) + material_editor.save_document_as_child(document_id, target_path_2) + time.sleep(2.0) + + # Close/Reopen documents + material_editor.close_all_documents() + document_id = material_editor.open_material(os.path.join(TEST_DATA_PATH, TEST_MATERIAL_1)) + document1_id = material_editor.open_material(target_path_1) + document2_id = material_editor.open_material(target_path_2) + + # Verify if the changes are saved in the actual document + actual_color = material_editor.get_property(document_id, property_name) + print(f"Actual Document saved with changes: {material_editor.compare_colors(actual_color, expected_color)}") + + # Verify if the changes are saved in the document saved as copy + actual_color = material_editor.get_property(document1_id, property_name) + result_copy = material_editor.compare_colors(actual_color, expected_color_1) + print(f"Document saved as copy is saved with changes: {result_copy}") + + # Verify if the changes are saved in the document saved as child + actual_color = material_editor.get_property(document2_id, property_name) + result_child = material_editor.compare_colors(actual_color, expected_color_2) + print(f"Document saved as child is saved with changes: {result_child}") + + # Revert back the changes in the actual document + material_editor.set_property(document_id, property_name, initial_color) + material_editor.save_document(document_id) + material_editor.close_all_documents() + + # 9) Test Case: Saving all Open Materials + # Open first material and make change to the values + document1_id = material_editor.open_material(os.path.join(TEST_DATA_PATH, TEST_MATERIAL_1)) + property1_name = azlmbr.name.Name("metallic.factor") + initial_metallic_factor = material_editor.get_property(document1_id, property1_name) + expected_metallic_factor = 0.444 + material_editor.set_property(document1_id, property1_name, expected_metallic_factor) + + # Open second material and make change to the values + document2_id = material_editor.open_material(os.path.join(TEST_DATA_PATH, TEST_MATERIAL_2)) + property2_name = azlmbr.name.Name("baseColor.color") + initial_color = material_editor.get_property(document2_id, property2_name) + expected_color = math.Color(0.4156, 0.0196, 0.6862, 1.0) + material_editor.set_property(document2_id, property2_name, expected_color) + + # Save all and close all documents + material_editor.save_all() + material_editor.close_all_documents() + + # Reopen materials and verify values + document1_id = material_editor.open_material(os.path.join(TEST_DATA_PATH, TEST_MATERIAL_1)) + result = material_editor.is_close( + material_editor.get_property(document1_id, property1_name), expected_metallic_factor, 0.00001 + ) + document2_id = material_editor.open_material(os.path.join(TEST_DATA_PATH, TEST_MATERIAL_2)) + result = result and material_editor.compare_colors( + expected_color, material_editor.get_property(document2_id, property2_name)) + print(f"Save All worked as expected: {result}") + + # Revert the changes made + material_editor.set_property(document1_id, property1_name, initial_metallic_factor) + material_editor.set_property(document2_id, property2_name, initial_color) + material_editor.save_all() + material_editor.close_all_documents() + + +if __name__ == "__main__": + run() diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/material_editor_utils.py b/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/material_editor_utils.py new file mode 100644 index 0000000000..77d1285188 --- /dev/null +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/atom_utils/material_editor_utils.py @@ -0,0 +1,274 @@ +""" +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 + +import azlmbr.materialeditor will fail with a ModuleNotFound error when using this script with Editor.exe +This is because azlmbr.materialeditor only binds to MaterialEditor.exe and not Editor.exe +You need to launch this script with MaterialEditor.exe in order for azlmbr.materialeditor to appear. +""" + +import os +import sys +import time +import azlmbr.atom +import azlmbr.materialeditor as materialeditor +import azlmbr.bus as bus +import azlmbr.atomtools.general as general + + +def is_close(actual, expected, buffer=sys.float_info.min): + """ + :param actual: actual value + :param expected: expected value + :param buffer: acceptable variation from expected + :return: bool + """ + return abs(actual - expected) < buffer + + +def compare_colors(color1, color2, buffer=0.00001): + """ + Compares the red, green and blue properties of a color allowing a slight variance of buffer + :param color1: first color to compare + :param color2: second color + :param buffer: allowed variance in individual color value + :return: bool + """ + return ( + is_close(color1.r, color2.r, buffer) + and is_close(color1.g, color2.g, buffer) + and is_close(color1.b, color2.b, buffer) + ) + + +def open_material(file_path): + """ + :return: uuid of material document opened + """ + return materialeditor.MaterialDocumentSystemRequestBus(bus.Broadcast, "OpenDocument", file_path) + + +def is_open(document_id): + """ + :return: bool + """ + return materialeditor.MaterialDocumentRequestBus(bus.Event, "IsOpen", document_id) + + +def save_document(document_id): + """ + :return: bool success + """ + return materialeditor.MaterialDocumentSystemRequestBus(bus.Broadcast, "SaveDocument", document_id) + + +def save_document_as_copy(document_id, target_path): + """ + :return: bool success + """ + return materialeditor.MaterialDocumentSystemRequestBus( + bus.Broadcast, "SaveDocumentAsCopy", document_id, target_path + ) + + +def save_document_as_child(document_id, target_path): + """ + :return: bool success + """ + return materialeditor.MaterialDocumentSystemRequestBus( + bus.Broadcast, "SaveDocumentAsChild", document_id, target_path + ) + + +def save_all(): + """ + :return: bool success + """ + return materialeditor.MaterialDocumentSystemRequestBus(bus.Broadcast, "SaveAllDocuments") + + +def close_document(document_id): + """ + :return: bool success + """ + return materialeditor.MaterialDocumentSystemRequestBus(bus.Broadcast, "CloseDocument", document_id) + + +def close_all_documents(): + """ + :return: bool success + """ + return materialeditor.MaterialDocumentSystemRequestBus(bus.Broadcast, "CloseAllDocuments") + + +def close_all_except_selected(document_id): + """ + :return: bool success + """ + return materialeditor.MaterialDocumentSystemRequestBus(bus.Broadcast, "CloseAllDocumentsExcept", document_id) + + +def get_property(document_id, property_name): + """ + :return: property value or invalid value if the document is not open or the property_name can't be found + """ + return materialeditor.MaterialDocumentRequestBus(bus.Event, "GetPropertyValue", document_id, property_name) + + +def set_property(document_id, property_name, value): + materialeditor.MaterialDocumentRequestBus(bus.Event, "SetPropertyValue", document_id, property_name, value) + + +def is_pane_visible(pane_name): + """ + :return: bool + """ + return materialeditor.MaterialEditorWindowRequestBus(bus.Broadcast, "IsDockWidgetVisible", pane_name) + + +def set_pane_visibility(pane_name, value): + materialeditor.MaterialEditorWindowRequestBus(bus.Broadcast, "SetDockWidgetVisible", pane_name, value) + + +def select_lighting_config(config_name): + azlmbr.materialeditor.MaterialViewportRequestBus(azlmbr.bus.Broadcast, "SelectLightingPresetByName", config_name) + + +def set_grid_enable_disable(value): + azlmbr.materialeditor.MaterialViewportRequestBus(azlmbr.bus.Broadcast, "SetGridEnabled", value) + + +def get_grid_enable_disable(): + """ + :return: bool + """ + return azlmbr.materialeditor.MaterialViewportRequestBus(azlmbr.bus.Broadcast, "GetGridEnabled") + + +def set_shadowcatcher_enable_disable(value): + azlmbr.materialeditor.MaterialViewportRequestBus(azlmbr.bus.Broadcast, "SetShadowCatcherEnabled", value) + + +def get_shadowcatcher_enable_disable(): + """ + :return: bool + """ + return azlmbr.materialeditor.MaterialViewportRequestBus(azlmbr.bus.Broadcast, "GetShadowCatcherEnabled") + + +def select_model_config(configname): + azlmbr.materialeditor.MaterialViewportRequestBus(azlmbr.bus.Broadcast, "SelectModelPresetByName", configname) + + +def wait_for_condition(function, timeout_in_seconds=1.0): + # type: (function, float) -> bool + """ + Function to run until it returns True or timeout is reached + the function can have no parameters and + waiting idle__wait_* is handled here not in the function + + :param function: a function that returns a boolean indicating a desired condition is achieved + :param timeout_in_seconds: when reached, function execution is abandoned and False is returned + """ + with Timeout(timeout_in_seconds) as t: + while True: + try: + general.idle_wait_frames(1) + except Exception: + print("WARNING: Couldn't wait for frame") + + if t.timed_out: + return False + + ret = function() + if not isinstance(ret, bool): + raise TypeError("return value for wait_for_condition function must be a bool") + if ret: + return True + + +class Timeout: + # type: (float) -> None + """ + contextual timeout + :param seconds: float seconds to allow before timed_out is True + """ + + def __init__(self, seconds): + self.seconds = seconds + + def __enter__(self): + self.die_after = time.time() + self.seconds + return self + + def __exit__(self, type, value, traceback): + pass + + @property + def timed_out(self): + return time.time() > self.die_after + + +screenshotsFolder = os.path.join(azlmbr.paths.devroot, "AtomTest", "Cache" "pc", "Screenshots") + + +class ScreenshotHelper: + """ + A helper to capture screenshots and wait for them. + """ + + def __init__(self, idle_wait_frames_callback): + super().__init__() + self.done = False + self.capturedScreenshot = False + self.max_frames_to_wait = 60 + + self.idle_wait_frames_callback = idle_wait_frames_callback + + def capture_screenshot_blocking(self, filename): + """ + Capture a screenshot and block the execution until the screenshot has been written to the disk. + """ + self.handler = azlmbr.atom.FrameCaptureNotificationBusHandler() + self.handler.connect() + self.handler.add_callback("OnCaptureFinished", self.on_screenshot_captured) + + self.done = False + self.capturedScreenshot = False + success = azlmbr.atom.FrameCaptureRequestBus(azlmbr.bus.Broadcast, "CaptureScreenshot", filename) + if success: + self.wait_until_screenshot() + print("Screenshot taken.") + else: + print("screenshot failed") + return self.capturedScreenshot + + def on_screenshot_captured(self, parameters): + # the parameters come in as a tuple + if parameters[0]: + print("screenshot saved: {}".format(parameters[1])) + self.capturedScreenshot = True + else: + print("screenshot failed: {}".format(parameters[1])) + self.done = True + self.handler.disconnect() + + def wait_until_screenshot(self): + frames_waited = 0 + while self.done == False: + self.idle_wait_frames_callback(1) + if frames_waited > self.max_frames_to_wait: + print("timeout while waiting for the screenshot to be written") + self.handler.disconnect() + break + else: + frames_waited = frames_waited + 1 + print("(waited {} frames)".format(frames_waited)) + + +def capture_screenshot(file_path): + return ScreenshotHelper(azlmbr.atomtools.general.idle_wait_frames).capture_screenshot_blocking( + os.path.join(file_path) + ) diff --git a/AutomatedTesting/Gem/PythonTests/atom_renderer/test_Atom_MainSuite.py b/AutomatedTesting/Gem/PythonTests/atom_renderer/test_Atom_MainSuite.py index 39689816b8..be75801b63 100644 --- a/AutomatedTesting/Gem/PythonTests/atom_renderer/test_Atom_MainSuite.py +++ b/AutomatedTesting/Gem/PythonTests/atom_renderer/test_Atom_MainSuite.py @@ -11,6 +11,7 @@ import os import pytest +import ly_test_tools.environment.file_system as file_system import editor_python_test_tools.hydra_test_utils as hydra from atom_renderer.atom_utils.atom_constants import LIGHT_TYPES @@ -242,3 +243,65 @@ class TestAtomEditorComponentsMain(object): null_renderer=True, cfg_args=cfg_args, ) + + +@pytest.mark.parametrize("project", ["AutomatedTesting"]) +@pytest.mark.parametrize("launcher_platform", ['windows_generic']) +@pytest.mark.system +class TestMaterialEditorBasicTests(object): + @pytest.fixture(autouse=True) + def setup_teardown(self, request, workspace, project): + def delete_files(): + file_system.delete( + [ + os.path.join(workspace.paths.project(), "Materials", "test_material.material"), + os.path.join(workspace.paths.project(), "Materials", "test_material_1.material"), + os.path.join(workspace.paths.project(), "Materials", "test_material_2.material"), + ], + True, + True, + ) + # Cleanup our newly created materials + delete_files() + + def teardown(): + # Cleanup our newly created materials + delete_files() + + request.addfinalizer(teardown) + + @pytest.mark.parametrize("exe_file_name", ["MaterialEditor"]) + def test_MaterialEditorBasicTests( + self, request, workspace, project, launcher_platform, generic_launcher, exe_file_name): + + expected_lines = [ + "Material opened: True", + "Test asset doesn't exist initially: True", + "New asset created: True", + "New Material opened: True", + "Material closed: True", + "All documents closed: True", + "Close All Except Selected worked as expected: True", + "Actual Document saved with changes: True", + "Document saved as copy is saved with changes: True", + "Document saved as child is saved with changes: True", + "Save All worked as expected: True", + ] + unexpected_lines = [ + # "Trace::Assert", + # "Trace::Error", + "Traceback (most recent call last):" + ] + + hydra.launch_and_validate_results( + request, + TEST_DIRECTORY, + generic_launcher, + "hydra_AtomMaterialEditor_BasicTests.py", + run_python="--runpython", + timeout=80, + expected_lines=expected_lines, + unexpected_lines=unexpected_lines, + halt_on_unexpected=True, + log_file_name="MaterialEditor.log", + )