From f6f7ee06a6b1a96746a391556833951af4c9af73 Mon Sep 17 00:00:00 2001 From: AMZN-Phil Date: Fri, 28 Jan 2022 10:10:17 -0800 Subject: [PATCH 1/8] Catch URLError so a message can be printed. Signed-off-by: AMZN-Phil --- scripts/o3de/o3de/utils.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/o3de/o3de/utils.py b/scripts/o3de/o3de/utils.py index 6628e6c946..5a3c807dd1 100644 --- a/scripts/o3de/o3de/utils.py +++ b/scripts/o3de/o3de/utils.py @@ -203,6 +203,9 @@ def download_file(parsed_uri, download_path: pathlib.Path, force_overwrite: bool except urllib.error.HTTPError as e: logger.error(f'HTTP Error {e.code} opening {parsed_uri.geturl()}') return 1 + except urllib.error.URLError as e: + logger.error(f'URL Error {e.reason} opening {parsed_uri.geturl()}') + return 1 else: origin_file = pathlib.Path(parsed_uri.geturl()).resolve() if not origin_file.is_file(): From c9c172794ef08aa007f60f5ff80acdbd7666ec58 Mon Sep 17 00:00:00 2001 From: Guthrie Adams Date: Thu, 3 Feb 2022 20:15:40 -0600 Subject: [PATCH 2/8] =?UTF-8?q?Atom=20Tools:=20Document=20system=20exposes?= =?UTF-8?q?=20reflected=20object=20data=20=E2=80=A2=20This=20removes=20a?= =?UTF-8?q?=20direct=20dependency=20on=20dynamic=20property=20groups=20and?= =?UTF-8?q?=20data=20from=20the=20document=20system.=20=E2=80=A2=20Added?= =?UTF-8?q?=20support=20for=20names,=20descriptions,=20and=20nesting=20to?= =?UTF-8?q?=20dynamic=20property=20groups.=20=E2=80=A2=20Moved=20property?= =?UTF-8?q?=20related=20functions=20from=20base=20document=20classes=20int?= =?UTF-8?q?o=20material=20editor=20document=20classes=20because=20dynamic?= =?UTF-8?q?=20property=20groups=20are=20an=20implementation=20detail=20of?= =?UTF-8?q?=20the=20material=20editor=20document=20to=20support=20material?= =?UTF-8?q?=20type=20flexible=20data=20format.=20=E2=80=A2=20Change=20mate?= =?UTF-8?q?rial=20document=20to=20use=20a=20table=20of=20dynamic=20propert?= =?UTF-8?q?y=20groups=20instead=20of=20a=20map=20of=20properties.=20?= =?UTF-8?q?=E2=80=A2=20Added=20functions=20to=20traverse=20groups=20and=20?= =?UTF-8?q?properties.=20=E2=80=A2=20This=20keeps=20groups=20and=20propert?= =?UTF-8?q?ies=20organized=20consistently=20with=20the=20material=20type?= =?UTF-8?q?=20file=20as=20well=20as=20what=E2=80=99s=20expected=20in=20the?= =?UTF-8?q?=20UI.=20=E2=80=A2=20Document=20data=20can=20now=20be=20maps=20?= =?UTF-8?q?directly=20to=20the=20inspector=20reflective=20property=20edito?= =?UTF-8?q?rs=20instead=20of=20copying=20one=20property=20at=20a=20time=20?= =?UTF-8?q?out=20of=20the=20document=20and=20keeping=20those=20synchronize?= =?UTF-8?q?d.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Guthrie Adams --- .../Atom/atom_utils/material_editor_utils.py | 4 +- .../Document/AtomToolsDocument.h | 9 +- .../AtomToolsDocumentNotificationBus.h | 23 +- .../Document/AtomToolsDocumentRequestBus.h | 30 +- .../AtomToolsDocumentSystemRequestBus.h | 4 +- .../DynamicProperty/DynamicProperty.h | 4 +- .../DynamicProperty/DynamicPropertyGroup.h | 6 + .../Source/Document/AtomToolsDocument.cpp | 32 +- .../AtomToolsDocumentSystemComponent.cpp | 2 - .../DynamicProperty/DynamicProperty.cpp | 10 +- .../DynamicProperty/DynamicPropertyGroup.cpp | 8 + .../Code/Source/Document/MaterialDocument.cpp | 656 +++++++++++------- .../Code/Source/Document/MaterialDocument.h | 69 +- .../Document/MaterialDocumentRequestBus.h | 9 +- .../Code/Source/MaterialEditorApplication.cpp | 3 +- .../MaterialInspector/MaterialInspector.cpp | 227 ++---- .../MaterialInspector/MaterialInspector.h | 18 +- .../ShaderManagementConsoleDocument.cpp | 22 + .../ShaderManagementConsoleDocument.h | 1 + 19 files changed, 565 insertions(+), 572 deletions(-) diff --git a/AutomatedTesting/Gem/PythonTests/Atom/atom_utils/material_editor_utils.py b/AutomatedTesting/Gem/PythonTests/Atom/atom_utils/material_editor_utils.py index 96eeb4279e..c3c1c76611 100644 --- a/AutomatedTesting/Gem/PythonTests/Atom/atom_utils/material_editor_utils.py +++ b/AutomatedTesting/Gem/PythonTests/Atom/atom_utils/material_editor_utils.py @@ -114,11 +114,11 @@ 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 azlmbr.atomtools.AtomToolsDocumentRequestBus(bus.Event, "GetPropertyValue", document_id, property_name) + return azlmbr.materialeditor.MaterialDocumentRequestBus(bus.Event, "GetPropertyValue", document_id, property_name) def set_property(document_id, property_name, value): - azlmbr.atomtools.AtomToolsDocumentRequestBus(bus.Event, "SetPropertyValue", document_id, property_name, value) + azlmbr.materialeditor.MaterialDocumentRequestBus(bus.Event, "SetPropertyValue", document_id, property_name, value) def is_pane_visible(pane_name): diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h index cb64d23cd0..2c9497df8e 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocument.h @@ -32,10 +32,7 @@ namespace AtomToolsFramework // AtomToolsDocumentRequestBus::Handler overrides... AZStd::string_view GetAbsolutePath() const override; - const AZStd::any& GetPropertyValue(const AZ::Name& propertyId) const override; - const AtomToolsFramework::DynamicProperty& GetProperty(const AZ::Name& propertyId) const override; - bool IsPropertyGroupVisible(const AZ::Name& propertyGroupFullName) const override; - void SetPropertyValue(const AZ::Name& propertyId, const AZStd::any& value) override; + AZStd::vector GetObjectInfo() const override; bool Open(AZStd::string_view loadPath) override; bool Reopen() override; bool Save() override; @@ -78,10 +75,6 @@ namespace AtomToolsFramework //! The normalized, absolute path where the document will be saved. AZStd::string m_savePathNormalized; - AZStd::any m_invalidValue; - - AtomToolsFramework::DynamicProperty m_invalidProperty; - //! This contains absolute paths of other source files that affect this document. //! If any of the source files in this container are modified, the document system is notified to reload this document. AZStd::unordered_set m_sourceDependencies; diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentNotificationBus.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentNotificationBus.h index 9c3a536333..b9af219b9e 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentNotificationBus.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentNotificationBus.h @@ -10,11 +10,10 @@ #include #include -#include -#include - namespace AtomToolsFramework { + struct DocumentObjectInfo; + class AtomToolsDocumentNotifications : public AZ::EBusTraits { @@ -61,22 +60,12 @@ namespace AtomToolsFramework //! Signal that a document undo state was updated //! @param documentId unique id of document for which the notification is sent virtual void OnDocumentUndoStateChanged([[maybe_unused]] const AZ::Uuid& documentId) {} - - //! Signal that a property changed - //! @param documentId unique id of document for which the notification is sent - //! @param property object containing the property value and configuration that was modified - virtual void OnDocumentPropertyValueModified([[maybe_unused]] const AZ::Uuid& documentId, [[maybe_unused]] const AtomToolsFramework::DynamicProperty& property) {} - - //! Signal that the property configuration has been changed. - //! @param documentId unique id of document for which the notification is sent - //! @param property object containing the property value and configuration that was modified - virtual void OnDocumentPropertyConfigModified([[maybe_unused]] const AZ::Uuid& documentId, [[maybe_unused]] const AtomToolsFramework::DynamicProperty& property) {} - //! Signal that the property group visibility has been changed. + //! Signal that the group has been changed. //! @param documentId unique id of document for which the notification is sent - //! @param groupId id of the group that changed - //! @param visible whether the property group is visible - virtual void OnDocumentPropertyGroupVisibilityChanged([[maybe_unused]] const AZ::Uuid& documentId, [[maybe_unused]] const AZ::Name& groupId, [[maybe_unused]] bool visible) {} + //! @param objectInfo description of the reflected object that's been modified + //! @param rebuilt signifies if it was a structural change that might require ui to be rebuilt + virtual void OnDocumentObjectInfoChanged([[maybe_unused]] const AZ::Uuid& documentId, [[maybe_unused]] const DocumentObjectInfo& objectInfo, [[maybe_unused]] bool rebuilt) {} }; using AtomToolsDocumentNotificationBus = AZ::EBus; diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentRequestBus.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentRequestBus.h index b6b3f65110..29cd8bede7 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentRequestBus.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentRequestBus.h @@ -14,6 +14,20 @@ namespace AtomToolsFramework { + //! Structure used as an opaque description of objects reflected inside of a document. + //! An example use would be semi automatic handling of serialization or reflection to a property editor. + struct DocumentObjectInfo + { + bool m_visible = true; + AZStd::string m_name; + AZStd::string m_displayName; + AZStd::string m_description; + AZ::Uuid m_objectType = AZ::Uuid::CreateNull(); + void* m_objectPtr = {}; + }; + + //! This bus provides the most basic interface for implementing a document that works with the document system. + //! Any extensions or application specific functionality should be added using a domain specific buses. class AtomToolsDocumentRequests : public AZ::EBusTraits { @@ -25,20 +39,8 @@ namespace AtomToolsFramework //! Get absolute path of document virtual AZStd::string_view GetAbsolutePath() const = 0; - //! Return property value - //! If the document is not open or the id can't be found, an invalid value is returned instead. - virtual const AZStd::any& GetPropertyValue(const AZ::Name& propertyFullName) const = 0; - - //! Returns a property object - //! If the document is not open or the id can't be found, an invalid property is returned. - virtual const AtomToolsFramework::DynamicProperty& GetProperty(const AZ::Name& propertyFullName) const = 0; - - //! Returns whether a property group is visible - //! If the document is not open or the id can't be found, returns false. - virtual bool IsPropertyGroupVisible(const AZ::Name& propertyGroupFullName) const = 0; - - //! Modify document property value - virtual void SetPropertyValue(const AZ::Name& propertyFullName, const AZStd::any& value) = 0; + //! Returns a container describing all reflected objects contained in a document + virtual AZStd::vector GetObjectInfo() const = 0; //! Load document and related data //! @param loadPath absolute path of document to load diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentSystemRequestBus.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentSystemRequestBus.h index b12cf5e580..88b5acd3be 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentSystemRequestBus.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsDocumentSystemRequestBus.h @@ -25,11 +25,11 @@ namespace AtomToolsFramework //! Register a document factory function used to create specific document types virtual void RegisterDocumentType(AZStd::function documentCreator) = 0; - //! Create a document object + //! Create a document //! @return Uuid of new document, or null Uuid if failed virtual AZ::Uuid CreateDocument() = 0; - //! Destroy a document object with the specified id + //! Destroy a document with the specified id //! @return true if Uuid was found and removed, otherwise false virtual bool DestroyDocument(const AZ::Uuid& documentId) = 0; diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicProperty.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicProperty.h index 5af0fcc845..2612b3be1b 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicProperty.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicProperty.h @@ -46,6 +46,7 @@ namespace AtomToolsFramework AZStd::string m_name; AZStd::string m_displayName; AZStd::string m_groupName; + AZStd::string m_groupDisplayName; AZStd::string m_description; AZStd::any m_defaultValue; AZStd::any m_parentValue; @@ -60,6 +61,7 @@ namespace AtomToolsFramework bool m_visible = true; bool m_readOnly = false; bool m_showThumbnail = false; + AZStd::function m_dataChangeCallback; }; //! Wraps an AZStd::any value and configuration so that it can be displayed and edited in a ReflectedPropertyEditor. @@ -106,7 +108,7 @@ namespace AtomToolsFramework private: // Functions used to configure edit data attributes. AZStd::string GetDisplayName() const; - AZStd::string GetGroupName() const; + AZStd::string GetGroupDisplayName() const; AZStd::string GetAssetPickerTitle() const; AZStd::string GetDescription() const; AZStd::vector> GetEnumValues() const; diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicPropertyGroup.h b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicPropertyGroup.h index 91705f3428..7d5e25b95f 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicPropertyGroup.h +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/DynamicProperty/DynamicPropertyGroup.h @@ -9,6 +9,7 @@ #pragma once #include +#include #include namespace AtomToolsFramework @@ -22,6 +23,11 @@ namespace AtomToolsFramework static void Reflect(AZ::ReflectContext* context); + bool m_visible = true; + AZStd::string m_name; + AZStd::string m_displayName; + AZStd::string m_description; AZStd::vector m_properties; + AZStd::vector> m_groups; }; } // namespace AtomToolsFramework diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp index 84638eefc4..2660a1e8fb 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp @@ -36,32 +36,10 @@ namespace AtomToolsFramework return m_absolutePath; } - const AZStd::any& AtomToolsDocument::GetPropertyValue([[maybe_unused]] const AZ::Name& propertyId) const + AZStd::vector AtomToolsDocument::GetObjectInfo() const { - AZ_UNUSED(propertyId); - AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); - return m_invalidValue; - } - - const AtomToolsFramework::DynamicProperty& AtomToolsDocument::GetProperty([[maybe_unused]] const AZ::Name& propertyId) const - { - AZ_UNUSED(propertyId); - AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); - return m_invalidProperty; - } - - bool AtomToolsDocument::IsPropertyGroupVisible([[maybe_unused]] const AZ::Name& propertyGroupFullName) const - { - AZ_UNUSED(propertyGroupFullName); - AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); - return false; - } - - void AtomToolsDocument::SetPropertyValue([[maybe_unused]] const AZ::Name& propertyId, [[maybe_unused]] const AZStd::any& value) - { - AZ_UNUSED(propertyId); - AZ_UNUSED(value); - AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); + AZ_Warning("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); + return AZStd::vector(); } bool AtomToolsDocument::Open(AZStd::string_view loadPath) @@ -256,13 +234,13 @@ namespace AtomToolsFramework bool AtomToolsDocument::BeginEdit() { - AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); + AZ_Warning("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); return false; } bool AtomToolsDocument::EndEdit() { - AZ_Error("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); + AZ_Warning("AtomToolsDocument", false, "%s not implemented.", __FUNCTION__); return false; } diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.cpp index b5a3f5322c..c05bb56391 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystemComponent.cpp @@ -77,8 +77,6 @@ namespace AtomToolsFramework ->Attribute(AZ::Script::Attributes::Category, "Editor") ->Attribute(AZ::Script::Attributes::Module, "atomtools") ->Event("GetAbsolutePath", &AtomToolsDocumentRequestBus::Events::GetAbsolutePath) - ->Event("GetPropertyValue", &AtomToolsDocumentRequestBus::Events::GetPropertyValue) - ->Event("SetPropertyValue", &AtomToolsDocumentRequestBus::Events::SetPropertyValue) ->Event("Open", &AtomToolsDocumentRequestBus::Events::Open) ->Event("Reopen", &AtomToolsDocumentRequestBus::Events::Reopen) ->Event("Close", &AtomToolsDocumentRequestBus::Events::Close) diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/DynamicProperty/DynamicProperty.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/DynamicProperty/DynamicProperty.cpp index 830f71933c..bd83fef320 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/DynamicProperty/DynamicProperty.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/DynamicProperty/DynamicProperty.cpp @@ -195,14 +195,14 @@ namespace AtomToolsFramework return !m_config.m_displayName.empty() ? m_config.m_displayName : m_config.m_name; } - AZStd::string DynamicProperty::GetGroupName() const + AZStd::string DynamicProperty::GetGroupDisplayName() const { - return m_config.m_groupName; + return m_config.m_groupDisplayName; } AZStd::string DynamicProperty::GetAssetPickerTitle() const { - return GetGroupName().empty() ? GetDisplayName() : GetGroupName() + " " + GetDisplayName(); + return GetGroupDisplayName().empty() ? GetDisplayName() : GetGroupDisplayName() + " " + GetDisplayName(); } AZStd::string DynamicProperty::GetDescription() const @@ -235,6 +235,10 @@ namespace AtomToolsFramework AZ::u32 DynamicProperty::OnDataChanged() const { + if (m_config.m_dataChangeCallback) + { + return m_config.m_dataChangeCallback(GetValue()); + } return AZ::Edit::PropertyRefreshLevels::AttributesAndValues; } diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/DynamicProperty/DynamicPropertyGroup.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/DynamicProperty/DynamicPropertyGroup.cpp index 41e96976e6..2647eb180a 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/DynamicProperty/DynamicPropertyGroup.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/DynamicProperty/DynamicPropertyGroup.cpp @@ -17,7 +17,12 @@ namespace AtomToolsFramework if (auto serializeContext = azrtti_cast(context)) { serializeContext->Class() + ->Field("visible", &DynamicPropertyGroup::m_visible) + ->Field("name", &DynamicPropertyGroup::m_name) + ->Field("displayName", &DynamicPropertyGroup::m_displayName) + ->Field("description", &DynamicPropertyGroup::m_description) ->Field("properties", &DynamicPropertyGroup::m_properties) + ->Field("groups", &DynamicPropertyGroup::m_groups) ; if (auto editContext = serializeContext->GetEditContext()) @@ -28,6 +33,9 @@ namespace AtomToolsFramework ->DataElement(AZ::Edit::UIHandlers::Default, &DynamicPropertyGroup::m_properties, "properties", "") ->Attribute(AZ::Edit::Attributes::Visibility, AZ::Edit::PropertyVisibility::ShowChildrenOnly) // hides the m_properties row ->Attribute(AZ::Edit::Attributes::ContainerCanBeModified, false) // probably not necessary since Visibility is children-only + ->DataElement(AZ::Edit::UIHandlers::Default, &DynamicPropertyGroup::m_groups, "groups", "") + ->Attribute(AZ::Edit::Attributes::Visibility, AZ::Edit::PropertyVisibility::ShowChildrenOnly) // hides the m_groups row + ->Attribute(AZ::Edit::Attributes::ContainerCanBeModified, false) // probably not necessary since Visibility is children-only ; } } diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp index 283b7a8a7f..2c1ff5cad3 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp @@ -53,112 +53,93 @@ namespace MaterialEditor return &m_materialTypeSourceData; } - const AZStd::any& MaterialDocument::GetPropertyValue(const AZ::Name& propertyId) const + void MaterialDocument::SetPropertyValue(const AZ::Name& propertyId, const AZStd::any& value) { if (!IsOpen()) { AZ_Error("MaterialDocument", false, "Document is not open."); - return m_invalidValue; + return; } - const auto it = m_properties.find(propertyId); - if (it == m_properties.end()) - { - AZ_Error("MaterialDocument", false, "Document property could not be found: '%s'.", propertyId.GetCStr()); - return m_invalidValue; - } + AtomToolsFramework::DynamicProperty* foundProperty = {}; + TraverseGroups(m_groups, [&, this](auto& group) { + for (auto& property : group->m_properties) + { + if (property.GetId() == propertyId) + { + foundProperty = &property; - const AtomToolsFramework::DynamicProperty& property = it->second; - return property.GetValue(); - } + // This first converts to an acceptable runtime type in case the value came from script + const AZ::RPI::MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(value); - const AtomToolsFramework::DynamicProperty& MaterialDocument::GetProperty(const AZ::Name& propertyId) const - { - if (!IsOpen()) - { - AZ_Error("MaterialDocument", false, "Document is not open."); - return m_invalidProperty; - } + property.SetValue(AtomToolsFramework::ConvertToEditableType(propertyValue)); + + const auto propertyIndex = m_materialInstance->FindPropertyIndex(propertyId); + if (!propertyIndex.IsNull()) + { + if (m_materialInstance->SetPropertyValue(propertyIndex, propertyValue)) + { + AZ::RPI::MaterialPropertyFlags dirtyFlags = m_materialInstance->GetPropertyDirtyFlags(); - const auto it = m_properties.find(propertyId); - if (it == m_properties.end()) + Recompile(); + RunEditorMaterialFunctors(dirtyFlags); + } + } + + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentObjectInfoChanged, m_id, + GetObjectInfoFromDynamicPropertyGroup(group.get()), false); + + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentModified, m_id); + return false; + } + } + return true; + }); + + if (!foundProperty) { AZ_Error("MaterialDocument", false, "Document property could not be found: '%s'.", propertyId.GetCStr()); - return m_invalidProperty; } - - const AtomToolsFramework::DynamicProperty& property = it->second; - return property; } - - bool MaterialDocument::IsPropertyGroupVisible(const AZ::Name& propertyGroupFullName) const + + const AZStd::any& MaterialDocument::GetPropertyValue(const AZ::Name& propertyId) const { if (!IsOpen()) { AZ_Error("MaterialDocument", false, "Document is not open."); - return false; + return m_invalidValue; } - const auto it = m_propertyGroupVisibility.find(propertyGroupFullName); - if (it == m_propertyGroupVisibility.end()) + auto property = FindProperty(propertyId); + if (!property) { - AZ_Error("MaterialDocument", false, "Document property group could not be found: '%s'.", propertyGroupFullName.GetCStr()); - return false; + AZ_Error("MaterialDocument", false, "Document property could not be found: '%s'.", propertyId.GetCStr()); + return m_invalidValue; } - return it->second; + return property->GetValue(); } - void MaterialDocument::SetPropertyValue(const AZ::Name& propertyId, const AZStd::any& value) + AZStd::vector MaterialDocument::GetObjectInfo() const { if (!IsOpen()) { AZ_Error("MaterialDocument", false, "Document is not open."); - return; + return {}; } - const auto it = m_properties.find(propertyId); - if (it == m_properties.end()) - { - AZ_Error("MaterialDocument", false, "Document property could not be found: '%s'.", propertyId.GetCStr()); - return; - } - - // This first converts to an acceptable runtime type in case the value came from script - const AZ::RPI::MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(value); - - AtomToolsFramework::DynamicProperty& property = it->second; - property.SetValue(AtomToolsFramework::ConvertToEditableType(propertyValue)); + AZStd::vector objects; + objects.reserve(m_groups.size()); - const auto propertyIndex = m_materialInstance->FindPropertyIndex(propertyId); - if (!propertyIndex.IsNull()) + AtomToolsFramework::DocumentObjectInfo objectInfo; + for (const auto& group : m_groups) { - if (m_materialInstance->SetPropertyValue(propertyIndex, propertyValue)) - { - AZ::RPI::MaterialPropertyFlags dirtyFlags = m_materialInstance->GetPropertyDirtyFlags(); - - Recompile(); - - EditorMaterialFunctorResult result = RunEditorMaterialFunctors(dirtyFlags); - for (const AZ::Name& changedPropertyGroupName : result.m_updatedPropertyGroups) - { - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( - &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentPropertyGroupVisibilityChanged, m_id, - changedPropertyGroupName, IsPropertyGroupVisible(changedPropertyGroupName)); - } - for (const AZ::Name& changedPropertyName : result.m_updatedProperties) - { - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( - &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentPropertyConfigModified, m_id, - GetProperty(changedPropertyName)); - } - } + objects.push_back(GetObjectInfoFromDynamicPropertyGroup(group.get())); } - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( - &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentPropertyValueModified, m_id, property); - AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( - &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentModified, m_id); + return objects; } bool MaterialDocument::Save() @@ -185,14 +166,15 @@ namespace MaterialEditor } // after saving, reset to a clean state - for (auto& propertyPair : m_properties) - { - AtomToolsFramework::DynamicProperty& property = propertyPair.second; - auto propertyConfig = property.GetConfig(); - propertyConfig.m_originalValue = property.GetValue(); - property.SetConfig(propertyConfig); - } - + TraverseGroups(m_groups, [&](auto& group) { + for (auto& property : group->m_properties) + { + auto propertyConfig = property.GetConfig(); + propertyConfig.m_originalValue = property.GetValue(); + property.SetConfig(propertyConfig); + } + return true; + }); return SaveSucceeded(); } @@ -273,12 +255,19 @@ namespace MaterialEditor bool MaterialDocument::IsModified() const { - return AZStd::any_of(m_properties.begin(), m_properties.end(), - [](const auto& propertyPair) - { - const AtomToolsFramework::DynamicProperty& property = propertyPair.second; - return !AtomToolsFramework::ArePropertyValuesEqual(property.GetValue(), property.GetConfig().m_originalValue); + bool result = false; + TraverseGroups(m_groups, [&](auto& group) { + for (auto& property : group->m_properties) + { + if (!AtomToolsFramework::ArePropertyValuesEqual(property.GetValue(), property.GetConfig().m_originalValue)) + { + result = true; + return false; + } + } + return true; }); + return result; } bool MaterialDocument::IsSavable() const @@ -290,11 +279,13 @@ namespace MaterialEditor { // Save the current properties as a momento for undo before any changes are applied m_propertyValuesBeforeEdit.clear(); - for (const auto& propertyPair : m_properties) - { - const AtomToolsFramework::DynamicProperty& property = propertyPair.second; - m_propertyValuesBeforeEdit[property.GetId()] = property.GetValue(); - } + TraverseGroups(m_groups, [this](auto& group) { + for (auto& property : group->m_properties) + { + m_propertyValuesBeforeEdit[property.GetId()] = property.GetValue(); + } + return true; + }); return true; } @@ -348,10 +339,10 @@ namespace MaterialEditor AZ::Name propertyId{propertyIdContext + propertyDefinition->GetName()}; - const auto it = m_properties.find(propertyId); - if (it != m_properties.end() && propertyFilter(it->second)) + const auto property = FindProperty(propertyId); + if (property && propertyFilter(*property)) { - AZ::RPI::MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(it->second.GetValue()); + AZ::RPI::MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(property->GetValue()); if (propertyValue.IsValid()) { if (!AtomToolsFramework::ConvertToExportFormat(m_savePathNormalized, propertyId, *propertyDefinition, propertyValue)) @@ -394,52 +385,17 @@ namespace MaterialEditor // The material document and inspector are constructed from source data if (AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), AZ::RPI::MaterialSourceData::Extension)) { - // Load the material source data so that we can check properties and create a material asset from it - if (!AZ::RPI::JsonUtils::LoadObjectFromFile(m_absolutePath, m_materialSourceData)) + if (!LoadMaterialSourceData()) { - AZ_Error("MaterialDocument", false, "Material source data could not be loaded: '%s'.", m_absolutePath.c_str()); return OpenFailed(); } - - // We always need the absolute path for the material type and parent material to load source data and resolving - // relative paths when saving. This will convert and store them as absolute paths for use within the document. - if (!m_materialSourceData.m_parentMaterial.empty()) - { - m_materialSourceData.m_parentMaterial = - AZ::RPI::AssetUtils::ResolvePathReference(m_absolutePath, m_materialSourceData.m_parentMaterial); - } - - if (!m_materialSourceData.m_materialType.empty()) - { - m_materialSourceData.m_materialType = - AZ::RPI::AssetUtils::ResolvePathReference(m_absolutePath, m_materialSourceData.m_materialType); - } - - // Load the material type source data which provides the layout and default values of all of the properties - auto materialTypeOutcome = AZ::RPI::MaterialUtils::LoadMaterialTypeSourceData(m_materialSourceData.m_materialType); - if (!materialTypeOutcome.IsSuccess()) - { - AZ_Error("MaterialDocument", false, "Material type source data could not be loaded: '%s'.", m_materialSourceData.m_materialType.c_str()); - return OpenFailed(); - } - m_materialTypeSourceData = materialTypeOutcome.TakeValue(); } else if (AzFramework::StringFunc::Path::IsExtension(m_absolutePath.c_str(), AZ::RPI::MaterialTypeSourceData::Extension)) { - // A material document can be created or loaded from material or material type source data. If we are attempting to load - // material type source data then the material source data object can be created just by referencing the document path as the - // material type path. - auto materialTypeOutcome = AZ::RPI::MaterialUtils::LoadMaterialTypeSourceData(m_absolutePath); - if (!materialTypeOutcome.IsSuccess()) + if (!LoadMaterialTypeSourceData()) { - AZ_Error("MaterialDocument", false, "Material type source data could not be loaded: '%s'.", m_absolutePath.c_str()); return OpenFailed(); } - m_materialTypeSourceData = materialTypeOutcome.TakeValue(); - - // We are storing absolute paths in the loaded version of the source data so that the files can be resolved at all times. - m_materialSourceData.m_materialType = m_absolutePath; - m_materialSourceData.m_parentMaterial.clear(); } else { @@ -519,58 +475,21 @@ namespace MaterialEditor // where such changes are supported at runtime. m_materialInstance->SetPsoHandlingOverride(AZ::RPI::MaterialPropertyPsoHandling::Allowed); - // Populate the property map from a combination of source data and assets - // Assets must still be used for now because they contain the final accumulated value after all other materials - // in the hierarchy are applied - m_materialTypeSourceData.EnumeratePropertyGroups([this, &parentPropertyValues](const AZStd::string& propertyIdContext, const AZ::RPI::MaterialTypeSourceData::PropertyGroup* propertyGroup) - { - AtomToolsFramework::DynamicPropertyConfig propertyConfig; + // Adding properties for material type and parent as part of making dynamic properties and the inspector more general purpose. This + // allows the read only properties to appear in the inspector like any other property. This may change or be removed once support + // for changing the material parent is implemented. + m_groups.emplace_back(aznew AtomToolsFramework::DynamicPropertyGroup); + m_groups.back()->m_name = "overview"; + m_groups.back()->m_displayName = "Overview"; + m_groups.back()->m_description = m_materialSourceData.m_description; - for (const auto& propertyDefinition : propertyGroup->GetProperties()) - { - // Assign id before conversion so it can be used in dynamic description - propertyConfig.m_id = propertyIdContext + propertyGroup->GetName() + "." + propertyDefinition->GetName(); - - const auto& propertyIndex = m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyConfig.m_id); - const bool propertyIndexInBounds = propertyIndex.IsValid() && propertyIndex.GetIndex() < m_materialAsset->GetPropertyValues().size(); - AZ_Warning("MaterialDocument", propertyIndexInBounds, "Failed to add material property '%s' to document '%s'.", propertyConfig.m_id.GetCStr(), m_absolutePath.c_str()); - - if (propertyIndexInBounds) - { - AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, *propertyDefinition); - propertyConfig.m_showThumbnail = true; - propertyConfig.m_originalValue = AtomToolsFramework::ConvertToEditableType(m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()]); - propertyConfig.m_parentValue = AtomToolsFramework::ConvertToEditableType(parentPropertyValues[propertyIndex.GetIndex()]); - - // TODO: Support populating the Material Editor with nested property groups, not just the top level. - // (Does DynamicPropertyConfig really even need m_groupName?) - propertyConfig.m_groupName = propertyGroup->GetDisplayName(); - m_properties[propertyConfig.m_id] = AtomToolsFramework::DynamicProperty(propertyConfig); - } - } - - return true; - }); - - // Populate the property group visibility map - // TODO: Support populating the Material Editor with nested property groups, not just the top level. - for (const AZStd::unique_ptr& propertyGroup : m_materialTypeSourceData.GetPropertyLayout().m_propertyGroups) - { - m_propertyGroupVisibility[AZ::Name{propertyGroup->GetName()}] = true; - } - - // Adding properties for material type and parent as part of making dynamic - // properties and the inspector more general purpose. - // This allows the read only properties to appear in the inspector like any - // other property. - // This may change or be removed once support for changing the material parent - // is implemented. AtomToolsFramework::DynamicPropertyConfig propertyConfig; propertyConfig.m_dataType = AtomToolsFramework::DynamicPropertyType::Asset; propertyConfig.m_id = "overview.materialType"; propertyConfig.m_name = "materialType"; propertyConfig.m_displayName = "Material Type"; - propertyConfig.m_groupName = "Overview"; + propertyConfig.m_groupName = "overview"; + propertyConfig.m_groupDisplayName = "Overview"; propertyConfig.m_description = "The material type defines the layout, properties, default values, shader connections, and other " "data needed to create and edit a derived material."; propertyConfig.m_defaultValue = AZStd::any(materialTypeAsset); @@ -578,14 +497,15 @@ namespace MaterialEditor propertyConfig.m_parentValue = propertyConfig.m_defaultValue; propertyConfig.m_readOnly = true; - m_properties[propertyConfig.m_id] = AtomToolsFramework::DynamicProperty(propertyConfig); + m_groups.back()->m_properties.push_back(AtomToolsFramework::DynamicProperty(propertyConfig)); propertyConfig = {}; propertyConfig.m_dataType = AtomToolsFramework::DynamicPropertyType::Asset; propertyConfig.m_id = "overview.parentMaterial"; propertyConfig.m_name = "parentMaterial"; propertyConfig.m_displayName = "Parent Material"; - propertyConfig.m_groupName = "Overview"; + propertyConfig.m_groupName = "overview"; + propertyConfig.m_groupDisplayName = "Overview"; propertyConfig.m_description = "The parent material provides an initial configuration whose properties are inherited and overriden by a derived material."; propertyConfig.m_defaultValue = AZStd::any(parentMaterialAsset); @@ -594,9 +514,14 @@ namespace MaterialEditor propertyConfig.m_readOnly = true; propertyConfig.m_showThumbnail = true; - m_properties[propertyConfig.m_id] = AtomToolsFramework::DynamicProperty(propertyConfig); + m_groups.back()->m_properties.push_back(AtomToolsFramework::DynamicProperty(propertyConfig)); + + m_groups.emplace_back(aznew AtomToolsFramework::DynamicPropertyGroup); + m_groups.back()->m_name = UvGroupName; + m_groups.back()->m_displayName = "UV Sets"; + m_groups.back()->m_description = "UV set names in this material, which can be renamed to match those in the model."; - //Add UV name customization properties + // Add UV name customization properties const AZ::RPI::MaterialUvNameMap& uvNameMap = materialTypeAsset->GetUvNameMap(); for (const AZ::RPI::UvNamePair& uvNamePair : uvNameMap) { @@ -608,61 +533,69 @@ namespace MaterialEditor propertyConfig.m_id = AZ::RPI::MaterialPropertyId(UvGroupName, shaderInput); propertyConfig.m_name = shaderInput; propertyConfig.m_displayName = shaderInput; - propertyConfig.m_groupName = "UV Sets"; + propertyConfig.m_groupName = UvGroupName; + propertyConfig.m_groupDisplayName = "UV Sets"; propertyConfig.m_description = shaderInput; propertyConfig.m_defaultValue = uvName; propertyConfig.m_originalValue = uvName; propertyConfig.m_parentValue = uvName; propertyConfig.m_readOnly = true; - m_properties[propertyConfig.m_id] = AtomToolsFramework::DynamicProperty(propertyConfig); + m_groups.back()->m_properties.push_back(AtomToolsFramework::DynamicProperty(propertyConfig)); } - // Add material functors that are in the top-level functors list. - const AZ::RPI::MaterialFunctorSourceData::EditorContext editorContext = - AZ::RPI::MaterialFunctorSourceData::EditorContext(m_materialSourceData.m_materialType, m_materialAsset->GetMaterialPropertiesLayout()); - for (AZ::RPI::Ptr functorData : m_materialTypeSourceData.m_materialFunctorSourceData) - { - AZ::RPI::MaterialFunctorSourceData::FunctorResult result2 = functorData->CreateFunctor(editorContext); - - if (result2.IsSuccess()) + // Populate the property map from a combination of source data and assets + // Assets must still be used for now because they contain the final accumulated value after all other materials + // in the hierarchy are applied + bool enumerateResult = m_materialTypeSourceData.EnumeratePropertyGroups( + [this, &parentPropertyValues]( + const AZStd::string& propertyIdContext, const AZ::RPI::MaterialTypeSourceData::PropertyGroup* propertyGroup) { - AZ::RPI::Ptr& functor = result2.GetValue(); - if (functor != nullptr) + // Add any material functors that are located inside each property group. + if (!AddEditorMaterialFunctors(propertyGroup->GetFunctors())) { - m_editorFunctors.push_back(functor); + return false; } - } - else - { - AZ_Error("MaterialDocument", false, "Material functors were not created: '%s'.", m_absolutePath.c_str()); - return OpenFailed(); - } - } - - // Add any material functors that are located inside each property group. - bool enumerateResult = m_materialTypeSourceData.EnumeratePropertyGroups( - [this](const AZStd::string&, const AZ::RPI::MaterialTypeSourceData::PropertyGroup* propertyGroup) - { - const AZ::RPI::MaterialFunctorSourceData::EditorContext editorContext = AZ::RPI::MaterialFunctorSourceData::EditorContext( - m_materialSourceData.m_materialType, m_materialAsset->GetMaterialPropertiesLayout()); - for (AZ::RPI::Ptr functorData : propertyGroup->GetFunctors()) + m_groups.emplace_back(aznew AtomToolsFramework::DynamicPropertyGroup); + m_groups.back()->m_name = propertyIdContext + propertyGroup->GetName(); + m_groups.back()->m_displayName = propertyGroup->GetDisplayName(); + m_groups.back()->m_description = propertyGroup->GetDescription(); + + for (const auto& propertyDefinition : propertyGroup->GetProperties()) { - AZ::RPI::MaterialFunctorSourceData::FunctorResult result = functorData->CreateFunctor(editorContext); + // Assign id before conversion so it can be used in dynamic description + AtomToolsFramework::DynamicPropertyConfig propertyConfig; + propertyConfig.m_id = m_groups.back()->m_name + "." + propertyDefinition->GetName(); + + const auto& propertyIndex = m_materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyConfig.m_id); + const bool propertyIndexInBounds = + propertyIndex.IsValid() && propertyIndex.GetIndex() < m_materialAsset->GetPropertyValues().size(); + AZ_Warning( + "MaterialDocument", propertyIndexInBounds, "Failed to add material property '%s' to document '%s'.", + propertyConfig.m_id.GetCStr(), m_absolutePath.c_str()); - if (result.IsSuccess()) + if (propertyIndexInBounds) { - AZ::RPI::Ptr& functor = result.GetValue(); - if (functor != nullptr) + AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, *propertyDefinition); + + // TODO: Support populating the Material Editor with nested property groups, not just the top level. + // (Does DynamicPropertyConfig really even need m_groupDisplayName?) + propertyConfig.m_groupName = m_groups.back()->m_name; + propertyConfig.m_groupDisplayName = m_groups.back()->m_displayName; + propertyConfig.m_showThumbnail = true; + propertyConfig.m_originalValue = + AtomToolsFramework::ConvertToEditableType(m_materialAsset->GetPropertyValues()[propertyIndex.GetIndex()]); + propertyConfig.m_parentValue = + AtomToolsFramework::ConvertToEditableType(parentPropertyValues[propertyIndex.GetIndex()]); + propertyConfig.m_dataChangeCallback = [documentId = m_id, propertyId = propertyConfig.m_id](const AZStd::any& value) { - m_editorFunctors.push_back(functor); - } - } - else - { - AZ_Error("MaterialDocument", false, "Material functors were not created: '%s'.", m_absolutePath.c_str()); - return false; + MaterialDocumentRequestBus::Event( + documentId, &MaterialDocumentRequestBus::Events::SetPropertyValue, propertyId, value); + return AZ::Edit::PropertyRefreshLevels::AttributesAndValues; + }; + + m_groups.back()->m_properties.push_back(AtomToolsFramework::DynamicProperty(propertyConfig)); } } @@ -674,6 +607,12 @@ namespace MaterialEditor return OpenFailed(); } + // Add material functors that are in the top-level functors list. + if (!AddEditorMaterialFunctors(m_materialTypeSourceData.m_materialFunctorSourceData)) + { + return OpenFailed(); + } + AZ::RPI::MaterialPropertyFlags dirtyFlags; dirtyFlags.set(); // Mark all properties as dirty since we just loaded the material and need to initialize property visibility RunEditorMaterialFunctors(dirtyFlags); @@ -681,17 +620,35 @@ namespace MaterialEditor return OpenSucceeded(); } + void MaterialDocument::Clear() + { + AtomToolsFramework::AtomToolsDocument::Clear(); + + AZ::TickBus::Handler::BusDisconnect(); + + m_materialAsset = {}; + m_materialInstance = {}; + m_compilePending = {}; + m_groups.clear(); + m_editorFunctors.clear(); + m_materialTypeSourceData = AZ::RPI::MaterialTypeSourceData(); + m_materialSourceData = AZ::RPI::MaterialSourceData(); + m_propertyValuesBeforeEdit.clear(); + } + bool MaterialDocument::ReopenRecordState() { m_propertyValuesBeforeReopen.clear(); - for (const auto& propertyPair : m_properties) - { - const AtomToolsFramework::DynamicProperty& property = propertyPair.second; - if (!AtomToolsFramework::ArePropertyValuesEqual(property.GetValue(), property.GetConfig().m_parentValue)) + TraverseGroups(m_groups, [this](auto& group) { + for (auto& property : group->m_properties) { - m_propertyValuesBeforeReopen[property.GetId()] = property.GetValue(); + if (!AtomToolsFramework::ArePropertyValuesEqual(property.GetValue(), property.GetConfig().m_parentValue)) + { + m_propertyValuesBeforeReopen[property.GetId()] = property.GetValue(); + } } - } + return true; + }); return AtomToolsDocument::ReopenRecordState(); } @@ -711,20 +668,59 @@ namespace MaterialEditor } } - void MaterialDocument::Clear() + bool MaterialDocument::LoadMaterialSourceData() { - AtomToolsFramework::AtomToolsDocument::Clear(); + // Load the material source data so that we can check properties and create a material asset from it + if (!AZ::RPI::JsonUtils::LoadObjectFromFile(m_absolutePath, m_materialSourceData)) + { + AZ_Error("MaterialDocument", false, "Material source data could not be loaded: '%s'.", m_absolutePath.c_str()); + return false; + } - AZ::TickBus::Handler::BusDisconnect(); + // We always need the absolute path for the material type and parent material to load source data and resolving + // relative paths when saving. This will convert and store them as absolute paths for use within the document. + if (!m_materialSourceData.m_parentMaterial.empty()) + { + m_materialSourceData.m_parentMaterial = + AZ::RPI::AssetUtils::ResolvePathReference(m_absolutePath, m_materialSourceData.m_parentMaterial); + } - m_materialAsset = {}; - m_materialInstance = {}; - m_compilePending = {}; - m_properties.clear(); - m_editorFunctors.clear(); - m_materialTypeSourceData = AZ::RPI::MaterialTypeSourceData(); - m_materialSourceData = AZ::RPI::MaterialSourceData(); - m_propertyValuesBeforeEdit.clear(); + if (!m_materialSourceData.m_materialType.empty()) + { + m_materialSourceData.m_materialType = + AZ::RPI::AssetUtils::ResolvePathReference(m_absolutePath, m_materialSourceData.m_materialType); + } + + // Load the material type source data which provides the layout and default values of all of the properties + auto materialTypeOutcome = AZ::RPI::MaterialUtils::LoadMaterialTypeSourceData(m_materialSourceData.m_materialType); + if (!materialTypeOutcome.IsSuccess()) + { + AZ_Error("MaterialDocument", false, "Material type source data could not be loaded: '%s'.", m_materialSourceData.m_materialType.c_str()); + return false; + } + + m_materialTypeSourceData = materialTypeOutcome.TakeValue(); + return true; + } + + bool MaterialDocument::LoadMaterialTypeSourceData() + { + // A material document can be created or loaded from material or material type source data. If we are attempting to load + // material type source data then the material source data object can be created just by referencing the document path as the + // material type path. + auto materialTypeOutcome = AZ::RPI::MaterialUtils::LoadMaterialTypeSourceData(m_absolutePath); + if (!materialTypeOutcome.IsSuccess()) + { + AZ_Error("MaterialDocument", false, "Material type source data could not be loaded: '%s'.", m_absolutePath.c_str()); + return false; + } + + m_materialTypeSourceData = materialTypeOutcome.TakeValue(); + + // We are storing absolute paths in the loaded version of the source data so that the files can be resolved at all times. + m_materialSourceData.m_materialType = m_absolutePath; + m_materialSourceData.m_parentMaterial.clear(); + return true; } void MaterialDocument::RestorePropertyValues(const PropertyValueMap& propertyValues) @@ -737,60 +733,186 @@ namespace MaterialEditor } } - MaterialDocument::EditorMaterialFunctorResult MaterialDocument::RunEditorMaterialFunctors(AZ::RPI::MaterialPropertyFlags dirtyFlags) + bool MaterialDocument::AddEditorMaterialFunctors( + const AZStd::vector>& functorSourceDataHolders) { - EditorMaterialFunctorResult result; + const AZ::RPI::MaterialFunctorSourceData::EditorContext editorContext = AZ::RPI::MaterialFunctorSourceData::EditorContext( + m_materialSourceData.m_materialType, m_materialAsset->GetMaterialPropertiesLayout()); - AZStd::unordered_map propertyDynamicMetadata; - AZStd::unordered_map propertyGroupDynamicMetadata; - for (auto& propertyPair : m_properties) - { - AtomToolsFramework::DynamicProperty& property = propertyPair.second; - AtomToolsFramework::ConvertToPropertyMetaData(propertyDynamicMetadata[property.GetId()], property.GetConfig()); - } - for (auto& groupPair : m_propertyGroupVisibility) + for (AZ::RPI::Ptr functorData : functorSourceDataHolders) { - AZ::RPI::MaterialPropertyGroupDynamicMetadata& metadata = propertyGroupDynamicMetadata[AZ::Name{groupPair.first}]; + AZ::RPI::MaterialFunctorSourceData::FunctorResult result = functorData->CreateFunctor(editorContext); - bool visible = groupPair.second; - metadata.m_visibility = visible ? - AZ::RPI::MaterialPropertyGroupVisibility::Enabled : AZ::RPI::MaterialPropertyGroupVisibility::Hidden; + if (result.IsSuccess()) + { + AZ::RPI::Ptr& functor = result.GetValue(); + if (functor != nullptr) + { + m_editorFunctors.push_back(functor); + } + } + else + { + AZ_Error("MaterialDocument", false, "Material functors were not created: '%s'.", m_absolutePath.c_str()); + return false; + } } - + + return true; + } + + void MaterialDocument::RunEditorMaterialFunctors(AZ::RPI::MaterialPropertyFlags dirtyFlags) + { + AZStd::unordered_map propertyDynamicMetadata; + AZStd::unordered_map propertyGroupDynamicMetadata; + + TraverseGroups(m_groups, [&](auto& group) { + AZ::RPI::MaterialPropertyGroupDynamicMetadata& metadata = propertyGroupDynamicMetadata[AZ::Name{ group->m_name }]; + metadata.m_visibility = group->m_visible ? AZ::RPI::MaterialPropertyGroupVisibility::Enabled : AZ::RPI::MaterialPropertyGroupVisibility::Hidden; + + for (auto& property : group->m_properties) + { + AtomToolsFramework::ConvertToPropertyMetaData(propertyDynamicMetadata[property.GetId()], property.GetConfig()); + } + return true; + }); + + AZStd::unordered_set updatedProperties; + AZStd::unordered_set updatedPropertyGroups; + for (AZ::RPI::Ptr& functor : m_editorFunctors) { const AZ::RPI::MaterialPropertyFlags& materialPropertyDependencies = functor->GetMaterialPropertyDependencies(); + // None also covers case that the client code doesn't register material properties to dependencies, // which will later get caught in Process() when trying to access a property. if (materialPropertyDependencies.none() || functor->NeedsProcess(dirtyFlags)) { AZ::RPI::MaterialFunctor::EditorContext context = AZ::RPI::MaterialFunctor::EditorContext( - m_materialInstance->GetPropertyValues(), - m_materialInstance->GetMaterialPropertiesLayout(), - propertyDynamicMetadata, - propertyGroupDynamicMetadata, - result.m_updatedProperties, - result.m_updatedPropertyGroups, - &materialPropertyDependencies - ); + m_materialInstance->GetPropertyValues(), m_materialInstance->GetMaterialPropertiesLayout(), propertyDynamicMetadata, + propertyGroupDynamicMetadata, updatedProperties, updatedPropertyGroups, + &materialPropertyDependencies); functor->Process(context); } } - for (auto& propertyPair : m_properties) + TraverseGroups(m_groups, [&](auto& group) { + bool groupChange = false; + bool groupRebuilt = false; + if (updatedPropertyGroups.find(AZ::Name(group->m_name)) != updatedPropertyGroups.end()) + { + AZ::RPI::MaterialPropertyGroupDynamicMetadata& metadata = propertyGroupDynamicMetadata[AZ::Name{ group->m_name }]; + group->m_visible = metadata.m_visibility != AZ::RPI::MaterialPropertyGroupVisibility::Hidden; + groupChange = true; + } + + for (auto& property : group->m_properties) + { + if (updatedProperties.find(AZ::Name(property.GetId())) != updatedProperties.end()) + { + const bool visibleBefore = property.GetConfig().m_visible; + AtomToolsFramework::DynamicPropertyConfig propertyConfig = property.GetConfig(); + AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, propertyDynamicMetadata[property.GetId()]); + property.SetConfig(propertyConfig); + groupChange = true; + groupRebuilt |= visibleBefore != property.GetConfig().m_visible; + } + } + + if (groupChange || groupRebuilt) + { + AtomToolsFramework::AtomToolsDocumentNotificationBus::Broadcast( + &AtomToolsFramework::AtomToolsDocumentNotificationBus::Events::OnDocumentObjectInfoChanged, m_id, + GetObjectInfoFromDynamicPropertyGroup(group.get()), groupRebuilt); + } + return true; + }); + } + + AtomToolsFramework::DocumentObjectInfo MaterialDocument::GetObjectInfoFromDynamicPropertyGroup( + const AtomToolsFramework::DynamicPropertyGroup* group) const + { + AtomToolsFramework::DocumentObjectInfo objectInfo; + objectInfo.m_visible = group->m_visible; + objectInfo.m_name = group->m_name; + objectInfo.m_displayName = group->m_displayName; + objectInfo.m_description = group->m_description; + objectInfo.m_objectType = azrtti_typeid(); + objectInfo.m_objectPtr = const_cast(group); + return objectInfo; + } + + bool MaterialDocument::TraverseGroups( + AZStd::vector>& groups, + AZStd::function&)> callback) + { + if (!callback) + { + return false; + } + + for (auto& group : groups) + { + if (!callback(group) || !TraverseGroups(group->m_groups, callback)) + { + return false; + } + } + + return true; + } + + bool MaterialDocument::TraverseGroups( + const AZStd::vector>& groups, + AZStd::function&)> callback) const + { + if (!callback) { - AtomToolsFramework::DynamicProperty& property = propertyPair.second; - AtomToolsFramework::DynamicPropertyConfig propertyConfig = property.GetConfig(); - AtomToolsFramework::ConvertToPropertyConfig(propertyConfig, propertyDynamicMetadata[property.GetId()]); - property.SetConfig(propertyConfig); + return false; } - for (auto& updatedPropertyGroup : result.m_updatedPropertyGroups) + for (auto& group : groups) { - bool visible = propertyGroupDynamicMetadata[updatedPropertyGroup].m_visibility == AZ::RPI::MaterialPropertyGroupVisibility::Enabled; - m_propertyGroupVisibility[updatedPropertyGroup] = visible; + if (!callback(group) || !TraverseGroups(group->m_groups, callback)) + { + return false; + } } + return true; + } + + AtomToolsFramework::DynamicProperty* MaterialDocument::FindProperty(const AZ::Name& propertyId) + { + AtomToolsFramework::DynamicProperty* result = nullptr; + TraverseGroups(m_groups, [&](auto& group) { + for (auto& property : group->m_properties) + { + if (property.GetId() == propertyId) + { + result = &property; + return false; + } + } + return true; + }); + return result; + } + + const AtomToolsFramework::DynamicProperty* MaterialDocument::FindProperty(const AZ::Name& propertyId) const + { + AtomToolsFramework::DynamicProperty* result = nullptr; + TraverseGroups(m_groups, [&](auto& group) { + for (auto& property : group->m_properties) + { + if (property.GetId() == propertyId) + { + result = &property; + return false; + } + } + return true; + }); return result; } } // namespace MaterialEditor diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h index b03d0943fa..37b3e1922f 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h @@ -37,10 +37,7 @@ namespace MaterialEditor virtual ~MaterialDocument(); // AtomToolsFramework::AtomToolsDocument overrides... - const AZStd::any& GetPropertyValue(const AZ::Name& propertyId) const override; - const AtomToolsFramework::DynamicProperty& GetProperty(const AZ::Name& propertyId) const override; - bool IsPropertyGroupVisible(const AZ::Name& propertyGroupFullName) const override; - void SetPropertyValue(const AZ::Name& propertyId, const AZStd::any& value) override; + AZStd::vector GetObjectInfo() const override; bool Open(AZStd::string_view loadPath) override; bool Save() override; bool SaveAsCopy(AZStd::string_view savePath) override; @@ -56,20 +53,16 @@ namespace MaterialEditor AZ::Data::Instance GetInstance() const override; const AZ::RPI::MaterialSourceData* GetMaterialSourceData() const override; const AZ::RPI::MaterialTypeSourceData* GetMaterialTypeSourceData() const override; + void SetPropertyValue(const AZ::Name& propertyId, const AZStd::any& value) override; + const AZStd::any& GetPropertyValue(const AZ::Name& propertyId) const override; private: // Predicate for evaluating properties using PropertyFilterFunction = AZStd::function; - // Map of document's properties - using PropertyMap = AZStd::unordered_map; - // Map of raw property values for undo/redo comparison and storage using PropertyValueMap = AZStd::unordered_map; - - // Map of document's property group visibility flags - using PropertyGroupVisibilityMap = AZStd::unordered_map; // AZ::TickBus overrides... void OnTick(float deltaTime, AZ::ScriptTimePoint time) override; @@ -78,47 +71,60 @@ namespace MaterialEditor // AtomToolsFramework::AtomToolsDocument overrides... void Clear() override; - bool ReopenRecordState() override; bool ReopenRestoreState() override; void Recompile(); + bool LoadMaterialSourceData(); + bool LoadMaterialTypeSourceData(); + void RestorePropertyValues(const PropertyValueMap& propertyValues); - struct EditorMaterialFunctorResult - { - AZStd::unordered_set m_updatedProperties; - AZStd::unordered_set m_updatedPropertyGroups; - }; + bool AddEditorMaterialFunctors( + const AZStd::vector>& functorSourceDataHolders); // Run editor material functor to update editor metadata. // @param dirtyFlags indicates which properties have changed, and thus which MaterialFunctors need to be run. - // @return names for the set of properties and groups that have been changed or need update. - EditorMaterialFunctorResult RunEditorMaterialFunctors(AZ::RPI::MaterialPropertyFlags dirtyFlags); + void RunEditorMaterialFunctors(AZ::RPI::MaterialPropertyFlags dirtyFlags); + + // Convert a dynamic property group pointer into generic document object info used to populate the inspector + AtomToolsFramework::DocumentObjectInfo GetObjectInfoFromDynamicPropertyGroup( + const AtomToolsFramework::DynamicPropertyGroup* group) const; - // Underlying material asset + // In order traversal of dynamic property groups + bool TraverseGroups( + AZStd::vector>& groups, + AZStd::function&)> callback); + + // In order traversal of dynamic property groups + bool TraverseGroups( + const AZStd::vector>& groups, + AZStd::function&)> callback) const; + + // Traverses dynamic property groups to find a property with a specific ID + AtomToolsFramework::DynamicProperty* FindProperty(const AZ::Name& propertyId); + + // Traverses dynamic property groups to find a property with a specific ID + const AtomToolsFramework::DynamicProperty* FindProperty(const AZ::Name& propertyId) const; + + // Material asset generated from source data, used to get the final values for properties to be assigned to the document AZ::Data::Asset m_materialAsset; - // Material instance being edited + // Material instance is only needed to run editor functors and is assigned directly to the viewport model to reflect real time + // changes to material property values AZ::Data::Instance m_materialInstance; // If material instance value(s) were modified, do we need to recompile on next tick? bool m_compilePending = false; - // Collection of all material's properties - PropertyMap m_properties; - - // Collection of all material's property groups - PropertyGroupVisibilityMap m_propertyGroupVisibility; - // Material functors that run in editor. See MaterialFunctor.h for details. AZStd::vector> m_editorFunctors; - // Source data for material type + // Material type source data used to enumerate all properties and populate the document AZ::RPI::MaterialTypeSourceData m_materialTypeSourceData; - // Source data for material + // Material source data with property values that override the material type AZ::RPI::MaterialSourceData m_materialSourceData; // State of property values prior to an edit, used for restoration during undo @@ -126,5 +132,12 @@ namespace MaterialEditor // State of property values prior to reopen PropertyValueMap m_propertyValuesBeforeReopen; + + // A container of root level dynamic property groups that represents the reflected, editable data within the document. + // These groups will be mapped to document object info so they can populate and be edited directly in the inspector. + AZStd::vector> m_groups; + + // Dummy default value returned whenever a property cannot be located + AZStd::any m_invalidValue; }; } // namespace MaterialEditor diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocumentRequestBus.h b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocumentRequestBus.h index c95aa4f215..da47ace48e 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocumentRequestBus.h +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocumentRequestBus.h @@ -45,7 +45,14 @@ namespace MaterialEditor //! Get the internal material type source data virtual const AZ::RPI::MaterialTypeSourceData* GetMaterialTypeSourceData() const = 0; - }; + + //! Modify property value + virtual void SetPropertyValue(const AZ::Name& propertyFullName, const AZStd::any& value) = 0; + + //! Return property value + //! If the document is not open or the id can't be found, an invalid value is returned instead. + virtual const AZStd::any& GetPropertyValue(const AZ::Name& propertyFullName) const = 0; + }; using MaterialDocumentRequestBus = AZ::EBus; } // namespace MaterialEditor diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/MaterialEditorApplication.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/MaterialEditorApplication.cpp index 2e64740057..686715017c 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/MaterialEditorApplication.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/MaterialEditorApplication.cpp @@ -65,7 +65,8 @@ namespace MaterialEditor ->Attribute(AZ::Script::Attributes::Scope, AZ::Script::Attributes::ScopeFlags::Common) ->Attribute(AZ::Script::Attributes::Category, "Editor") ->Attribute(AZ::Script::Attributes::Module, "materialeditor") - ; + ->Event("SetPropertyValue", &MaterialDocumentRequestBus::Events::SetPropertyValue) + ->Event("GetPropertyValue", &MaterialDocumentRequestBus::Events::GetPropertyValue); } } diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp index 1402ad9f24..e6e768a7fd 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.cpp @@ -7,14 +7,10 @@ */ #include -#include -#include -#include #include #include #include #include -#include #include namespace MaterialEditor @@ -38,7 +34,7 @@ namespace MaterialEditor { m_documentPath.clear(); m_documentId = AZ::Uuid::CreateNull(); - m_groups = {}; + m_activeProperty = {}; AtomToolsFramework::InspectorRequestBus::Handler::BusDisconnect(); AtomToolsFramework::InspectorWidget::Reset(); @@ -67,18 +63,31 @@ namespace MaterialEditor m_documentId = documentId; bool isOpen = false; - AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult(isOpen, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::IsOpen); + AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult( + isOpen, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::IsOpen); - AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult(m_documentPath, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::GetAbsolutePath); + AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult( + m_documentPath, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::GetAbsolutePath); if (!m_documentId.IsNull() && isOpen) { - // Create the top group for displaying overview info about the material - AddOverviewGroup(); - // Create groups for displaying editable UV names - AddUvNamesGroup(); - // Create groups for displaying editable properties - AddPropertiesGroup(); + // This will automatically expose all document contents to an inspector with a collapsible group per object. In the case of the + // material editor, this will be one inspector group per property group. + AZStd::vector objects; + AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult( + objects, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::GetObjectInfo); + + for (auto& objectInfo : objects) + { + // Passing in same main and comparison instance to enable custom value comparison for highlighting modified properties + auto propertyGroupWidget = new AtomToolsFramework::InspectorPropertyGroupWidget( + objectInfo.m_objectPtr, objectInfo.m_objectPtr, objectInfo.m_objectType, this, this, + GetGroupSaveStateKey(objectInfo.m_name), {}, + [this](const auto node) { return GetInstanceNodePropertyIndicator(node); }, 0); + + AddGroup(objectInfo.m_name, objectInfo.m_displayName, objectInfo.m_description, propertyGroupWidget); + SetGroupVisible(objectInfo.m_name, objectInfo.m_visible); + } AtomToolsFramework::InspectorRequestBus::Handler::BusConnect(m_documentId); } @@ -106,198 +115,42 @@ namespace MaterialEditor return ":/Icons/blank.png"; } - void MaterialInspector::AddOverviewGroup() - { - const AZ::RPI::MaterialTypeSourceData* materialTypeSourceData = nullptr; - MaterialDocumentRequestBus::EventResult( - materialTypeSourceData, m_documentId, &MaterialDocumentRequestBus::Events::GetMaterialTypeSourceData); - - const AZStd::string groupName = "overview"; - const AZStd::string groupDisplayName = "Overview"; - const AZStd::string groupDescription = materialTypeSourceData->m_description; - auto& group = m_groups[groupName]; - - AtomToolsFramework::DynamicProperty property; - AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult( - property, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::GetProperty, AZ::Name("overview.materialType")); - group.m_properties.push_back(property); - - property = {}; - AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult( - property, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::GetProperty, AZ::Name("overview.parentMaterial")); - group.m_properties.push_back(property); - - // Passing in same group as main and comparison instance to enable custom value comparison for highlighting modified properties - auto propertyGroupWidget = new AtomToolsFramework::InspectorPropertyGroupWidget( - &group, &group, group.TYPEINFO_Uuid(), this, this, GetGroupSaveStateKey(groupName), {}, - [this](const auto node) { return GetInstanceNodePropertyIndicator(node); }, 0); - AddGroup(groupName, groupDisplayName, groupDescription, propertyGroupWidget); - } - - void MaterialInspector::AddUvNamesGroup() - { - AZ::Data::Asset materialAsset; - MaterialDocumentRequestBus::EventResult(materialAsset, m_documentId, &MaterialDocumentRequestBus::Events::GetAsset); - - const AZStd::string groupName = UvGroupName; - const AZStd::string groupDisplayName = "UV Sets"; - const AZStd::string groupDescription = "UV set names in this material, which can be renamed to match those in the model."; - auto& group = m_groups[groupName]; - - const auto& uvNameMap = materialAsset->GetMaterialTypeAsset()->GetUvNameMap(); - group.m_properties.reserve(uvNameMap.size()); - - for (const auto& uvNamePair : uvNameMap) - { - AtomToolsFramework::DynamicProperty property; - AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult( - property, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::GetProperty, - AZ::RPI::MaterialPropertyId(groupName, uvNamePair.m_shaderInput.ToString())); - group.m_properties.push_back(property); - - property.SetValue(property.GetConfig().m_parentValue); - } - - // Passing in same group as main and comparison instance to enable custom value comparison for highlighting modified properties - auto propertyGroupWidget = new AtomToolsFramework::InspectorPropertyGroupWidget( - &group, &group, group.TYPEINFO_Uuid(), this, this, GetGroupSaveStateKey(groupName), {}, - [this](const auto node) { return GetInstanceNodePropertyIndicator(node); }, 0); - AddGroup(groupName, groupDisplayName, groupDescription, propertyGroupWidget); - } - - void MaterialInspector::AddPropertiesGroup() - { - const AZ::RPI::MaterialTypeSourceData* materialTypeSourceData = nullptr; - MaterialDocumentRequestBus::EventResult( - materialTypeSourceData, m_documentId, &MaterialDocumentRequestBus::Events::GetMaterialTypeSourceData); - - // TODO: Support populating the Material Editor with nested property groups, not just the top level. - for (const AZStd::unique_ptr& propertyGroup : materialTypeSourceData->GetPropertyLayout().m_propertyGroups) - { - const AZStd::string& groupName = propertyGroup->GetName(); - const AZStd::string& groupDisplayName = !propertyGroup->GetDisplayName().empty() ? propertyGroup->GetDisplayName() : groupName; - const AZStd::string& groupDescription = !propertyGroup->GetDescription().empty() ? propertyGroup->GetDescription() : groupDisplayName; - auto& group = m_groups[groupName]; - - group.m_properties.reserve(propertyGroup->GetProperties().size()); - for (const auto& propertyDefinition : propertyGroup->GetProperties()) - { - AtomToolsFramework::DynamicProperty property; - AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult( - property, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::GetProperty, - AZ::RPI::MaterialPropertyId(groupName, propertyDefinition->GetName())); - group.m_properties.push_back(property); - } - - // Passing in same group as main and comparison instance to enable custom value comparison for highlighting modified properties - auto propertyGroupWidget = new AtomToolsFramework::InspectorPropertyGroupWidget( - &group, &group, group.TYPEINFO_Uuid(), this, this, GetGroupSaveStateKey(groupName), {}, - [this](const auto node) { return GetInstanceNodePropertyIndicator(node); }, 0); - AddGroup(groupName, groupDisplayName, groupDescription, propertyGroupWidget); - - bool isGroupVisible = false; - AtomToolsFramework::AtomToolsDocumentRequestBus::EventResult( - isGroupVisible, m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::IsPropertyGroupVisible, AZ::Name{groupName}); - SetGroupVisible(groupName, isGroupVisible); - } - } - - void MaterialInspector::OnDocumentPropertyValueModified(const AZ::Uuid& documentId, const AtomToolsFramework::DynamicProperty& property) + void MaterialInspector::OnDocumentObjectInfoChanged( + [[maybe_unused]] const AZ::Uuid& documentId, const AtomToolsFramework::DocumentObjectInfo& objectInfo, bool rebuilt) { - for (auto& groupPair : m_groups) + SetGroupVisible(objectInfo.m_name, objectInfo.m_visible); + if (rebuilt) { - for (auto& reflectedProperty : groupPair.second.m_properties) - { - if (reflectedProperty.GetId() == property.GetId()) - { - if (!AtomToolsFramework::ArePropertyValuesEqual(reflectedProperty.GetValue(), property.GetValue())) - { - reflectedProperty.SetValue(property.GetValue()); - AtomToolsFramework::InspectorRequestBus::Event( - documentId, &AtomToolsFramework::InspectorRequestBus::Events::RefreshGroup, groupPair.first); - } - return; - } - } + RebuildGroup(objectInfo.m_name); } - } - - void MaterialInspector::OnDocumentPropertyConfigModified(const AZ::Uuid&, const AtomToolsFramework::DynamicProperty& property) - { - for (auto& groupPair : m_groups) + else { - for (auto& reflectedProperty : groupPair.second.m_properties) - { - if (reflectedProperty.GetId() == property.GetId()) - { - // Visibility changes require the entire reflected property editor tree for this group to be rebuilt - if (reflectedProperty.GetVisibility() != property.GetVisibility()) - { - reflectedProperty.SetConfig(property.GetConfig()); - RebuildGroup(groupPair.first); - } - else - { - reflectedProperty.SetConfig(property.GetConfig()); - RefreshGroup(groupPair.first); - } - return; - } - } + RefreshGroup(objectInfo.m_name); } } - - void MaterialInspector::OnDocumentPropertyGroupVisibilityChanged(const AZ::Uuid&, const AZ::Name& groupId, bool visible) - { - SetGroupVisible(groupId.GetStringView(), visible); - } void MaterialInspector::BeforePropertyModified(AzToolsFramework::InstanceDataNode* pNode) { - // For some reason the reflected property editor notifications are not symmetrical - // This function is called continuously anytime a property changes until the edit has completed - // Because of that, we have to track whether or not we are continuing to edit the same property to know when editing has started and - // ended - const AtomToolsFramework::DynamicProperty* property = AtomToolsFramework::FindDynamicPropertyForInstanceDataNode(pNode); - if (property) - { - if (m_activeProperty != property) - { - m_activeProperty = property; - AtomToolsFramework::AtomToolsDocumentRequestBus::Event(m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::BeginEdit); - } - } - } - - void MaterialInspector::AfterPropertyModified(AzToolsFramework::InstanceDataNode* pNode) - { + // This function is called before every single property change whether it's a button click or dragging a slider. We only want to + // begin tracking undo state for the first change in the sequence, when the user begins to drag the slider. const AtomToolsFramework::DynamicProperty* property = AtomToolsFramework::FindDynamicPropertyForInstanceDataNode(pNode); - if (property) + if (!m_activeProperty && property) { - if (m_activeProperty == property) - { - AtomToolsFramework::AtomToolsDocumentRequestBus::Event( - m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::SetPropertyValue, property->GetId(), property->GetValue()); - } + m_activeProperty = property; + AtomToolsFramework::AtomToolsDocumentRequestBus::Event( + m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::BeginEdit); } } void MaterialInspector::SetPropertyEditingComplete(AzToolsFramework::InstanceDataNode* pNode) { - // As above, there are symmetrical functions on the notification interface for when editing begins and ends and has been completed - // but they are not being called following that pattern. when this function executes the changes to the property are ready to be - // committed or reverted + // If tracking has started and editing has completed then we can stop tracking undue state for this sequence of changes. const AtomToolsFramework::DynamicProperty* property = AtomToolsFramework::FindDynamicPropertyForInstanceDataNode(pNode); - if (property) + if (m_activeProperty && m_activeProperty == property) { - if (m_activeProperty == property) - { - AtomToolsFramework::AtomToolsDocumentRequestBus::Event( - m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::SetPropertyValue, property->GetId(), property->GetValue()); - - AtomToolsFramework::AtomToolsDocumentRequestBus::Event(m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::EndEdit); - m_activeProperty = nullptr; - } + AtomToolsFramework::AtomToolsDocumentRequestBus::Event( + m_documentId, &AtomToolsFramework::AtomToolsDocumentRequestBus::Events::EndEdit); + m_activeProperty = {}; } } } // namespace MaterialEditor diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.h b/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.h index 690e8add86..fe688e3ad5 100644 --- a/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.h +++ b/Gems/Atom/Tools/MaterialEditor/Code/Source/Window/MaterialInspector/MaterialInspector.h @@ -45,31 +45,25 @@ namespace MaterialEditor bool IsInstanceNodePropertyModifed(const AzToolsFramework::InstanceDataNode* node) const; const char* GetInstanceNodePropertyIndicator(const AzToolsFramework::InstanceDataNode* node) const; - void AddOverviewGroup(); - void AddUvNamesGroup(); - void AddPropertiesGroup(); - // AtomToolsDocumentNotificationBus::Handler implementation void OnDocumentOpened(const AZ::Uuid& documentId) override; - void OnDocumentPropertyValueModified(const AZ::Uuid& documentId, const AtomToolsFramework::DynamicProperty& property) override; - void OnDocumentPropertyConfigModified(const AZ::Uuid& documentId, const AtomToolsFramework::DynamicProperty& property) override; - void OnDocumentPropertyGroupVisibilityChanged(const AZ::Uuid& documentId, const AZ::Name& groupId, bool visible) override; + void OnDocumentObjectInfoChanged( + const AZ::Uuid& documentId, const AtomToolsFramework::DocumentObjectInfo& objectInfo, bool rebuilt) override; // AzToolsFramework::IPropertyEditorNotify overrides... void BeforePropertyModified(AzToolsFramework::InstanceDataNode* pNode) override; - void AfterPropertyModified(AzToolsFramework::InstanceDataNode* pNode) override; + void AfterPropertyModified([[maybe_unused]] AzToolsFramework::InstanceDataNode* pNode) override {} void SetPropertyEditingActive([[maybe_unused]] AzToolsFramework::InstanceDataNode* pNode) override {} void SetPropertyEditingComplete(AzToolsFramework::InstanceDataNode* pNode) override; void SealUndoStack() override {} - void RequestPropertyContextMenu(AzToolsFramework::InstanceDataNode*, const QPoint&) override {} - void PropertySelectionChanged(AzToolsFramework::InstanceDataNode*, bool) override {} + void RequestPropertyContextMenu([[maybe_unused]] AzToolsFramework::InstanceDataNode* pNode, const QPoint&) override {} + void PropertySelectionChanged([[maybe_unused]] AzToolsFramework::InstanceDataNode* pNode, bool) override {} // Tracking the property that is activiley being edited in the inspector - const AtomToolsFramework::DynamicProperty* m_activeProperty = nullptr; + const AtomToolsFramework::DynamicProperty* m_activeProperty = {}; AZ::Uuid m_documentId = AZ::Uuid::CreateNull(); AZStd::string m_documentPath; - AZStd::unordered_map m_groups; AZStd::intrusive_ptr m_windowSettings; }; } // namespace MaterialEditor diff --git a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.cpp b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.cpp index 99a3aea347..5698e33cec 100644 --- a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.cpp +++ b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.cpp @@ -78,6 +78,28 @@ namespace ShaderManagementConsole return m_invalidDescriptor; } + AZStd::vector ShaderManagementConsoleDocument::GetObjectInfo() const + { + if (!IsOpen()) + { + AZ_Error("ShaderManagementConsoleDocument", false, "Document is not open."); + return {}; + } + + AZStd::vector objects; + + AtomToolsFramework::DocumentObjectInfo objectInfo; + objectInfo.m_visible = true; + objectInfo.m_name = "Shader Variant List"; + objectInfo.m_displayName = "Shader Variant List"; + objectInfo.m_description = "Shader Variant List"; + objectInfo.m_objectType = azrtti_typeid(); + objectInfo.m_objectPtr = const_cast(&m_shaderVariantListSourceData); + objects.push_back(AZStd::move(objectInfo)); + + return objects; + } + bool ShaderManagementConsoleDocument::Open(AZStd::string_view loadPath) { if (!AtomToolsDocument::Open(loadPath)) diff --git a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.h b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.h index 4ad951e463..5c3d66b0a9 100644 --- a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.h +++ b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.h @@ -32,6 +32,7 @@ namespace ShaderManagementConsole ~ShaderManagementConsoleDocument(); // AtomToolsFramework::AtomToolsDocument overrides... + AZStd::vector GetObjectInfo() const override; bool Open(AZStd::string_view loadPath) override; bool Save() override; bool SaveAsCopy(AZStd::string_view savePath) override; From 76b2932cae3a801ac117610e6dab0e3f6ca675ab Mon Sep 17 00:00:00 2001 From: Allen Jackson <23512001+jackalbe@users.noreply.github.com> Date: Fri, 4 Feb 2022 08:50:14 -0600 Subject: [PATCH 3/8] {lyn7307} disable saving procedural prefabs in the Editor (#4877) disable saving procedural prefabs in the Editor since the templates are backed by asset files that will change outside the Editor Signed-off-by: jackalbe <23512001+jackalbe@users.noreply.github.com> --- .../AzToolsFramework/Prefab/PrefabSystemComponent.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp index 9648a9af09..38ffb98fcb 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabSystemComponent.cpp @@ -815,7 +815,8 @@ namespace AzToolsFramework if (templateRef.has_value()) { - return templateRef->get().IsDirty(); + return !templateRef->get().IsProcedural() && // all procedural prefabs are read-only + templateRef->get().IsDirty(); } return false; From d77403f9df988acf99dd5f0f843f6dbecebfd9bb Mon Sep 17 00:00:00 2001 From: galibzon <66021303+galibzon@users.noreply.github.com> Date: Fri, 4 Feb 2022 10:58:42 -0600 Subject: [PATCH 4/8] Upgrade AZSLc to version 1.7.35 (#7411) * Upgrade AZSLc to version 1.7.35 for Windows, Linux & Mac. Signed-off-by: galibzon <66021303+galibzon@users.noreply.github.com> --- cmake/3rdParty/Platform/Linux/BuiltInPackages_linux.cmake | 2 +- cmake/3rdParty/Platform/Mac/BuiltInPackages_mac.cmake | 2 +- cmake/3rdParty/Platform/Windows/BuiltInPackages_windows.cmake | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmake/3rdParty/Platform/Linux/BuiltInPackages_linux.cmake b/cmake/3rdParty/Platform/Linux/BuiltInPackages_linux.cmake index 742b4a0555..f20b67307b 100644 --- a/cmake/3rdParty/Platform/Linux/BuiltInPackages_linux.cmake +++ b/cmake/3rdParty/Platform/Linux/BuiltInPackages_linux.cmake @@ -38,7 +38,7 @@ ly_associate_package(PACKAGE_NAME OpenMesh-8.1-rev3-linux ly_associate_package(PACKAGE_NAME OpenSSL-1.1.1b-rev2-linux TARGETS OpenSSL PACKAGE_HASH b779426d1e9c5ddf71160d5ae2e639c3b956e0fb5e9fcaf9ce97c4526024e3bc) ly_associate_package(PACKAGE_NAME DirectXShaderCompilerDxc-1.6.2104-o3de-rev3-linux TARGETS DirectXShaderCompilerDxc PACKAGE_HASH 88c4a359325d749bc34090b9ac466424847f3b71ba0de15045cf355c17c07099) ly_associate_package(PACKAGE_NAME SPIRVCross-2021.04.29-rev1-linux TARGETS SPIRVCross PACKAGE_HASH 7889ee5460a688e9b910c0168b31445c0079d363affa07b25d4c8aeb608a0b80) -ly_associate_package(PACKAGE_NAME azslc-1.7.34-rev1-linux TARGETS azslc PACKAGE_HASH 6d7dc671936c34ff70d2632196107ca1b8b2b41acdd021bfbc69a9fd56215c22) +ly_associate_package(PACKAGE_NAME azslc-1.7.35-rev1-linux TARGETS azslc PACKAGE_HASH 273484be06dfc25e8da6a6e17937ae69a2efdb0b4c5f105efa83d6ad54d756e5) ly_associate_package(PACKAGE_NAME zlib-1.2.11-rev5-linux TARGETS ZLIB PACKAGE_HASH 9be5ea85722fc27a8645a9c8a812669d107c68e6baa2ca0740872eaeb6a8b0fc) ly_associate_package(PACKAGE_NAME squish-ccr-deb557d-rev1-linux TARGETS squish-ccr PACKAGE_HASH 85fecafbddc6a41a27c5f59ed4a5dfb123a94cb4666782cf26e63c0a4724c530) ly_associate_package(PACKAGE_NAME astc-encoder-3.2-rev2-linux TARGETS astc-encoder PACKAGE_HASH 71549d1ca9e4d48391b92a89ea23656d3393810e6777879f6f8a9def2db1610c) diff --git a/cmake/3rdParty/Platform/Mac/BuiltInPackages_mac.cmake b/cmake/3rdParty/Platform/Mac/BuiltInPackages_mac.cmake index 2ac3c49480..fba8f73ba1 100644 --- a/cmake/3rdParty/Platform/Mac/BuiltInPackages_mac.cmake +++ b/cmake/3rdParty/Platform/Mac/BuiltInPackages_mac.cmake @@ -41,6 +41,6 @@ ly_associate_package(PACKAGE_NAME squish-ccr-deb557d-rev1-mac ly_associate_package(PACKAGE_NAME astc-encoder-3.2-rev5-mac TARGETS astc-encoder PACKAGE_HASH bdb1146cc6bbacc07901564fe884529d7cacc9bb44895597327341d3b9833ab0) ly_associate_package(PACKAGE_NAME ISPCTexComp-36b80aa-rev1-mac TARGETS ISPCTexComp PACKAGE_HASH 8a4e93277b8face6ea2fd57c6d017bdb55643ed3d6387110bc5f6b3b884dd169) ly_associate_package(PACKAGE_NAME lz4-1.9.3-vcpkg-rev4-mac TARGETS lz4 PACKAGE_HASH 891ff630bf34f7ab1d8eaee2ea0a8f1fca89dbdc63fca41ee592703dd488a73b) -ly_associate_package(PACKAGE_NAME azslc-1.7.34-rev1-mac TARGETS azslc PACKAGE_HASH a9d81946b42ffa55c0d14d6a9249b3340e59a8fb8835e7a96c31df80f14723bc) +ly_associate_package(PACKAGE_NAME azslc-1.7.35-rev1-mac TARGETS azslc PACKAGE_HASH 03cb1ea8c47d4c80c893e2e88767272d5d377838f5ba94b777a45902dd85052e) ly_associate_package(PACKAGE_NAME SQLite-3.37.2-rev1-mac TARGETS SQLite PACKAGE_HASH f9101023f99cf32fc5867284ceb28c0761c23d2c5a4b1748349c69f976a2fbea) ly_associate_package(PACKAGE_NAME AwsIotDeviceSdkCpp-1.15.2-rev2-mac TARGETS AwsIotDeviceSdkCpp PACKAGE_HASH 4854edb7b88fa6437b4e69e87d0ee111a25313ac2a2db5bb2f8b674ba0974f95) diff --git a/cmake/3rdParty/Platform/Windows/BuiltInPackages_windows.cmake b/cmake/3rdParty/Platform/Windows/BuiltInPackages_windows.cmake index fb7fb18235..e24582a453 100644 --- a/cmake/3rdParty/Platform/Windows/BuiltInPackages_windows.cmake +++ b/cmake/3rdParty/Platform/Windows/BuiltInPackages_windows.cmake @@ -47,6 +47,6 @@ ly_associate_package(PACKAGE_NAME squish-ccr-deb557d-rev1-windows ly_associate_package(PACKAGE_NAME astc-encoder-3.2-rev2-windows TARGETS astc-encoder PACKAGE_HASH 17249bfa438afb34e21449865d9c9297471174ae0cea9b2f9def2ee206038295) ly_associate_package(PACKAGE_NAME ISPCTexComp-36b80aa-rev1-windows TARGETS ISPCTexComp PACKAGE_HASH b6fa6ea28a2808a9a5524c72c37789c525925e435770f2d94eb2d387360fa2d0) ly_associate_package(PACKAGE_NAME lz4-1.9.3-vcpkg-rev4-windows TARGETS lz4 PACKAGE_HASH 4ea457b833cd8cfaf8e8e06ed6df601d3e6783b606bdbc44a677f77e19e0db16) -ly_associate_package(PACKAGE_NAME azslc-1.7.34-rev1-windows TARGETS azslc PACKAGE_HASH 44eb2e0fc4b0f1c75d0fb6f24c93a5753655b84dbc3e6ad45389ed3b9cf7a4b0) +ly_associate_package(PACKAGE_NAME azslc-1.7.35-rev1-windows TARGETS azslc PACKAGE_HASH 606aea611f2f20afcd8467ddabeecd3661e946eac3c843756c7df2871c1fb8a0) ly_associate_package(PACKAGE_NAME SQLite-3.37.2-rev1-windows TARGETS SQLite PACKAGE_HASH c1658c8ed5cf0e45d4a5da940c6a6d770b76e0f4f57313b70d0fd306885f015e) ly_associate_package(PACKAGE_NAME AwsIotDeviceSdkCpp-1.15.2-rev1-windows TARGETS AwsIotDeviceSdkCpp PACKAGE_HASH b03475a9f0f7a7e7c90619fba35f1a74fb2b8f4cd33fa07af99f2ae9e0c079dd) From ff4412db7ca9978eda06ebce089704c743ef2bb8 Mon Sep 17 00:00:00 2001 From: Ken Pruiksma Date: Fri, 4 Feb 2022 11:01:47 -0600 Subject: [PATCH 5/8] Calculating uv transforms for terrain detail materials (#7375) Calculating uv transforms for terrain detail materials This adds support for uv transforms in terrain detail materials. To support this work, the code for generating a matrix from material properties was pulled out of the material functor and put into a new MaterialUtils file in Atom Utils. --- .../Source/Material/Transform2DFunctor.cpp | 58 +++-------------- .../Code/Source/Material/Transform2DFunctor.h | 11 +--- .../Material/Transform2DFunctorSourceData.cpp | 4 +- .../Material/Transform2DFunctorSourceData.h | 3 +- .../Code/Include/Atom/Utils/MaterialUtils.h | 46 ++++++++++++++ Gems/Atom/Utils/Code/Source/MaterialUtils.cpp | 62 +++++++++++++++++++ Gems/Atom/Utils/Code/atom_utils_files.cmake | 2 + .../Terrain/TerrainDetailHelpers.azsli | 33 +++++++--- .../TerrainDetailMaterialManager.cpp | 39 ++++++++++++ .../TerrainDetailMaterialManager.h | 2 +- 10 files changed, 190 insertions(+), 70 deletions(-) create mode 100644 Gems/Atom/Utils/Code/Include/Atom/Utils/MaterialUtils.h create mode 100644 Gems/Atom/Utils/Code/Source/MaterialUtils.cpp diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctor.cpp b/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctor.cpp index e7baff7f93..e9fdd6e06e 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctor.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctor.cpp @@ -39,59 +39,21 @@ namespace AZ ; } } - + void Transform2DFunctor::Process(RuntimeContext& context) { using namespace RPI; - auto center = context.GetMaterialPropertyValue(m_center); - auto scale = context.GetMaterialPropertyValue(m_scale); - auto scaleX = context.GetMaterialPropertyValue(m_scaleX); - auto scaleY = context.GetMaterialPropertyValue(m_scaleY); - auto translateX = context.GetMaterialPropertyValue(m_translateX); - auto translateY = context.GetMaterialPropertyValue(m_translateY); - auto rotateDegrees = context.GetMaterialPropertyValue(m_rotateDegrees); - - if (scaleX != 0.0f) - { - translateX *= (1.0f / scaleX); - } - - if (scaleY != 0.0f) - { - translateY *= (1.0f / scaleY); - } - - Matrix3x3 translateCenter2D = Matrix3x3::CreateIdentity(); - translateCenter2D.SetBasisZ(-center.GetX(), -center.GetY(), 1.0f); - - Matrix3x3 translateCenterInv2D = Matrix3x3::CreateIdentity(); - translateCenterInv2D.SetBasisZ(center.GetX(), center.GetY(), 1.0f); - - Matrix3x3 scale2D = Matrix3x3::CreateDiagonal(AZ::Vector3(scaleX * scale, scaleY * scale, 1.0f)); - - Matrix3x3 translate2D = Matrix3x3::CreateIdentity(); - translate2D.SetBasisZ(translateX, translateY, 1.0f); + UvTransformDescriptor desc; + desc.m_center = context.GetMaterialPropertyValue(m_center); + desc.m_scale = context.GetMaterialPropertyValue(m_scale); + desc.m_scaleX = context.GetMaterialPropertyValue(m_scaleX); + desc.m_scaleY = context.GetMaterialPropertyValue(m_scaleY); + desc.m_translateX = context.GetMaterialPropertyValue(m_translateX); + desc.m_translateY = context.GetMaterialPropertyValue(m_translateY); + desc.m_rotateDegrees = context.GetMaterialPropertyValue(m_rotateDegrees); - Matrix3x3 rotate2D = Matrix3x3::CreateRotationZ(AZ::DegToRad(rotateDegrees)); - - Matrix3x3 transform = translateCenter2D; - for (auto transformType : m_transformOrder) - { - switch (transformType) - { - case TransformType::Scale: - transform = scale2D * transform; - break; - case TransformType::Rotate: - transform = rotate2D * transform; - break; - case TransformType::Translate: - transform = translate2D * transform; - break; - } - } - transform = translateCenterInv2D * transform; + Matrix3x3 transform = CreateUvTransformMatrix(desc, m_transformOrder); context.GetShaderResourceGroup()->SetConstant(m_transformMatrix, transform); diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctor.h b/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctor.h index 2ef5af6913..58dcd7ebb7 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctor.h +++ b/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctor.h @@ -11,6 +11,7 @@ #include #include #include +#include namespace AZ { @@ -24,14 +25,6 @@ namespace AZ public: AZ_RTTI(Transform2DFunctor, "{3E9C4357-6B2D-4A22-89DB-462441C9D8CD}", RPI::MaterialFunctor); - enum class TransformType - { - Invalid, - Scale, - Rotate, - Translate - }; - static void Reflect(ReflectContext* context); using RPI::MaterialFunctor::Process; @@ -57,6 +50,4 @@ namespace AZ } // namespace Render - AZ_TYPE_INFO_SPECIALIZE(Render::Transform2DFunctor::TransformType, "{D8C15D33-CE3D-4297-A646-030B0625BF84}"); - } // namespace AZ diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctorSourceData.cpp b/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctorSourceData.cpp index 26e7983a4b..7436cb06b7 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctorSourceData.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctorSourceData.cpp @@ -85,13 +85,13 @@ namespace AZ functor->m_transformOrder = m_transformOrder; - AZStd::set transformSet{m_transformOrder.begin(), m_transformOrder.end()}; + AZStd::set transformSet{m_transformOrder.begin(), m_transformOrder.end()}; if (m_transformOrder.size() != transformSet.size()) { AZ_Warning("Transform2DFunctor", false, "transformOrder field contains duplicate entries"); } - if (transformSet.find(Transform2DFunctor::TransformType::Invalid) != transformSet.end()) + if (transformSet.find(TransformType::Invalid) != transformSet.end()) { AZ_Warning("Transform2DFunctor", false, "transformOrder contains invalid entries"); } diff --git a/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctorSourceData.h b/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctorSourceData.h index db7e65fedc..ae97879884 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctorSourceData.h +++ b/Gems/Atom/Feature/Common/Code/Source/Material/Transform2DFunctorSourceData.h @@ -10,6 +10,7 @@ #include "./Transform2DFunctor.h" #include +#include namespace AZ { @@ -30,7 +31,7 @@ namespace AZ private: - AZStd::vector m_transformOrder; //!< Controls the order in which Scale, Translate, Rotate are performed + AZStd::vector m_transformOrder; //!< Controls the order in which Scale, Translate, Rotate are performed // Material property inputs... AZStd::string m_center; //!< material property for center of scaling and rotation diff --git a/Gems/Atom/Utils/Code/Include/Atom/Utils/MaterialUtils.h b/Gems/Atom/Utils/Code/Include/Atom/Utils/MaterialUtils.h new file mode 100644 index 0000000000..5c89b63182 --- /dev/null +++ b/Gems/Atom/Utils/Code/Include/Atom/Utils/MaterialUtils.h @@ -0,0 +1,46 @@ +/* + * 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 + +#include +#include +#include +#include + +//! This file holds useful material related utility functions. + +namespace AZ +{ + namespace Render + { + enum class TransformType + { + Invalid, + Scale, + Rotate, + Translate + }; + + struct UvTransformDescriptor + { + Vector2 m_center{ Vector2::CreateZero() }; + float m_scale{ 1.0f }; + float m_scaleX{ 1.0f }; + float m_scaleY{ 1.0f }; + float m_translateX{ 0.0f }; + float m_translateY{ 0.0f }; + float m_rotateDegrees{ 0.0f }; + }; + + // Create a 3x3 uv transform matrix from a set of input properties. + Matrix3x3 CreateUvTransformMatrix(const UvTransformDescriptor& desc, const AZStd::span transformOrder); + } + + AZ_TYPE_INFO_SPECIALIZE(Render::TransformType, "{D8C15D33-CE3D-4297-A646-030B0625BF84}"); +} diff --git a/Gems/Atom/Utils/Code/Source/MaterialUtils.cpp b/Gems/Atom/Utils/Code/Source/MaterialUtils.cpp new file mode 100644 index 0000000000..6ce9ed5629 --- /dev/null +++ b/Gems/Atom/Utils/Code/Source/MaterialUtils.cpp @@ -0,0 +1,62 @@ +/* + * 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 + +namespace AZ::Render +{ + + Matrix3x3 CreateUvTransformMatrix(const UvTransformDescriptor& desc, AZStd::span transformOrder) + { + float translateX = desc.m_translateX; + float translateY = desc.m_translateY; + + if (desc.m_scaleX != 0.0f) + { + translateX *= (1.0f / desc.m_scaleX); + } + + if (desc.m_scaleY != 0.0f) + { + translateY *= (1.0f / desc.m_scaleY); + } + + Matrix3x3 translateCenter2D = Matrix3x3::CreateIdentity(); + translateCenter2D.SetBasisZ(-desc.m_center.GetX(), -desc.m_center.GetY(), 1.0f); + + Matrix3x3 translateCenterInv2D = Matrix3x3::CreateIdentity(); + translateCenterInv2D.SetBasisZ(desc.m_center.GetX(), desc.m_center.GetY(), 1.0f); + + Matrix3x3 scale2D = Matrix3x3::CreateDiagonal(AZ::Vector3(desc.m_scaleX * desc.m_scale, desc.m_scaleY * desc.m_scale, 1.0f)); + + Matrix3x3 translate2D = Matrix3x3::CreateIdentity(); + translate2D.SetBasisZ(translateX, translateY, 1.0f); + + Matrix3x3 rotate2D = Matrix3x3::CreateRotationZ(AZ::DegToRad(desc.m_rotateDegrees)); + + Matrix3x3 transform = translateCenter2D; + for (auto transformType : transformOrder) + { + switch (transformType) + { + case TransformType::Scale: + transform = scale2D * transform; + break; + case TransformType::Rotate: + transform = rotate2D * transform; + break; + case TransformType::Translate: + transform = translate2D * transform; + break; + } + } + transform = translateCenterInv2D * transform; + return transform; + } + +} diff --git a/Gems/Atom/Utils/Code/atom_utils_files.cmake b/Gems/Atom/Utils/Code/atom_utils_files.cmake index c11c2e294d..31529c93be 100644 --- a/Gems/Atom/Utils/Code/atom_utils_files.cmake +++ b/Gems/Atom/Utils/Code/atom_utils_files.cmake @@ -21,6 +21,7 @@ set(FILES Include/Atom/Utils/ImGuiShaderMetrics.inl Include/Atom/Utils/ImGuiTransientAttachmentProfiler.h Include/Atom/Utils/ImGuiTransientAttachmentProfiler.inl + Include/Atom/Utils/MaterialUtils.h Include/Atom/Utils/PngFile.h Include/Atom/Utils/PpmFile.h Include/Atom/Utils/StableDynamicArray.h @@ -30,6 +31,7 @@ set(FILES Include/Atom/Utils/AssetCollectionAsyncLoader.h Source/DdsFile.cpp Source/ImageComparison.cpp + Source/MaterialUtils.cpp Source/PngFile.cpp Source/PpmFile.cpp Source/Utils.cpp diff --git a/Gems/Terrain/Assets/Shaders/Terrain/TerrainDetailHelpers.azsli b/Gems/Terrain/Assets/Shaders/Terrain/TerrainDetailHelpers.azsli index b566b727ff..9794eec4a3 100644 --- a/Gems/Terrain/Assets/Shaders/Terrain/TerrainDetailHelpers.azsli +++ b/Gems/Terrain/Assets/Shaders/Terrain/TerrainDetailHelpers.azsli @@ -204,16 +204,33 @@ void GetDetailSurfaceForMaterial(inout DetailSurface surface, uint materialId, f { TerrainSrg::DetailMaterialData detailMaterialData = TerrainSrg::m_detailMaterialData[materialId]; + float3x3 uvTransform = (float3x3)detailMaterialData.m_uvTransform; + float2 transformedUv = mul(uvTransform, float3(uv, 1.0)).xy; + + // With different materials in the quad, we can't rely on ddx/ddy of the transformed uv because + // the materials may have different uv transforms. This would create visible seams where the wrong + // mip was being used. Instead, manually calculate the transformed ddx/ddy using the ddx/ddy of the + // original uv. + float2 uvDdx = ddx(uv); float2 uvDdy = ddy(uv); - surface.m_color = GetDetailColor(detailMaterialData, uv, uvDdx, uvDdy); - surface.m_normal = GetDetailNormal(detailMaterialData, uv, uvDdx, uvDdy); - surface.m_roughness = GetDetailRoughness(detailMaterialData, uv, uvDdx, uvDdy); - surface.m_specularF0 = GetDetailSpecularF0(detailMaterialData, uv, uvDdx, uvDdy); - surface.m_metalness = GetDetailMetalness(detailMaterialData, uv, uvDdx, uvDdy); - surface.m_occlusion = GetDetailOcclusion(detailMaterialData, uv, uvDdx, uvDdy); - surface.m_height = GetDetailHeight(detailMaterialData, uv, uvDdx, uvDdy); + float2 uvX = uv + uvDdx; + float2 uvY = uv + uvDdy; + + float2 transformedUvX = mul(uvTransform, float3(uvX, 1.0)).xy; + float2 transformedUvY = mul(uvTransform, float3(uvY, 1.0)).xy; + + float2 transformedUvDdx = transformedUvX - transformedUv; + float2 transformedUvDdy = transformedUvY - transformedUv; + + surface.m_color = GetDetailColor(detailMaterialData, transformedUv, transformedUvDdx, transformedUvDdy); + surface.m_normal = GetDetailNormal(detailMaterialData, transformedUv, transformedUvDdx, transformedUvDdy); + surface.m_roughness = GetDetailRoughness(detailMaterialData, transformedUv, transformedUvDdx, transformedUvDdy); + surface.m_specularF0 = GetDetailSpecularF0(detailMaterialData, transformedUv, transformedUvDdx, transformedUvDdy); + surface.m_metalness = GetDetailMetalness(detailMaterialData, transformedUv, transformedUvDdx, transformedUvDdy); + surface.m_occlusion = GetDetailOcclusion(detailMaterialData, transformedUv, transformedUvDdx, transformedUvDdy); + surface.m_height = GetDetailHeight(detailMaterialData, transformedUv, transformedUvDdx, transformedUvDdy); } // Debugs the detail material by choosing a random color per material ID and rendering it without blending. @@ -279,7 +296,7 @@ bool GetDetailSurface(inout DetailSurface surface, float2 detailMaterialIdCoord, float2 textureSize; TerrainSrg::m_detailMaterialIdImage.GetDimensions(textureSize.x, textureSize.y); - // The detail material id texture wraps since the "center" point can be anywhere in the texture, so mod by texturesize + // The detail material id texture wraps since the "center" point can be anywhere in the texture, so mod by textureSize int2 detailMaterialIdTopLeft = ((int2(detailMaterialIdCoord) % textureSize) + textureSize) % textureSize; int2 detailMaterialIdBottomRight = (detailMaterialIdTopLeft + 1) % textureSize; diff --git a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainDetailMaterialManager.cpp b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainDetailMaterialManager.cpp index 96b363d8c6..a00d2efe59 100644 --- a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainDetailMaterialManager.cpp +++ b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainDetailMaterialManager.cpp @@ -15,6 +15,8 @@ #include #include +#include + #include namespace Terrain @@ -56,6 +58,13 @@ namespace Terrain static const char* const HeightFactor("parallax.factor"); static const char* const HeightOffset("parallax.offset"); static const char* const HeightBlendFactor("parallax.blendFactor"); + static const char* const UvCenter("uv.center"); + static const char* const UvScale("uv.scale"); + static const char* const UvTileU("uv.tileU"); + static const char* const UvTileV("uv.tileV"); + static const char* const UvOffsetU("uv.offsetU"); + static const char* const UvOffsetV("uv.offsetV"); + static const char* const UvRotateDegrees("uv.rotateDegrees"); } namespace TerrainSrgInputs @@ -548,6 +557,36 @@ namespace Terrain applyProperty(HeightOffset, shaderData.m_heightOffset); applyProperty(HeightBlendFactor, shaderData.m_heightBlendFactor); + AZ::Render::UvTransformDescriptor transformDescriptor; + applyProperty(UvCenter, transformDescriptor.m_center); + applyProperty(UvScale, transformDescriptor.m_scale); + applyProperty(UvTileU, transformDescriptor.m_scaleX); + applyProperty(UvTileV, transformDescriptor.m_scaleY); + applyProperty(UvOffsetU, transformDescriptor.m_translateX); + applyProperty(UvOffsetV, transformDescriptor.m_translateY); + applyProperty(UvRotateDegrees, transformDescriptor.m_rotateDegrees); + + AZStd::array order = + { + AZ::Render::TransformType::Rotate, + AZ::Render::TransformType::Translate, + AZ::Render::TransformType::Scale, + }; + + AZ::Matrix3x3 uvTransformMatrix = AZ::Render::CreateUvTransformMatrix(transformDescriptor, order); + uvTransformMatrix.GetRow(0).StoreToFloat3(&shaderData.m_uvTransform[0]); + uvTransformMatrix.GetRow(1).StoreToFloat3(&shaderData.m_uvTransform[4]); + uvTransformMatrix.GetRow(2).StoreToFloat3(&shaderData.m_uvTransform[8]); + + // Store a hash of the matrix in element in an unused portion for quick comparisons in the shader + size_t hash64 = 0; + for (float value : shaderData.m_uvTransform) + { + AZStd::hash_combine(hash64, value); + } + uint32_t hash32 = uint32_t((hash64 ^ (hash64 >> 32)) & 0xFFFFFFFF); + shaderData.m_uvTransform[3] = *reinterpret_cast(&hash32); + m_detailMaterialBufferNeedsUpdate = true; } diff --git a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainDetailMaterialManager.h b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainDetailMaterialManager.h index 667b1b2e85..3f63812e47 100644 --- a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainDetailMaterialManager.h +++ b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainDetailMaterialManager.h @@ -77,7 +77,7 @@ namespace Terrain struct DetailMaterialShaderData { - // Uv + // Uv (data is 3x3, padding each row for explicit alignment) AZStd::array m_uvTransform { 1.0, 0.0, 0.0, 0.0, From d9ba0af6454e80ba39e5d2d3da5443c49761de22 Mon Sep 17 00:00:00 2001 From: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> Date: Fri, 4 Feb 2022 11:27:59 -0600 Subject: [PATCH 6/8] SurfacePoint data structure encapsulations (#7413) * First pass at encapsulating SurfacePointList. The biggest challenge in optimizing SurfacePointList(s) usage is the overall memory management associated with it. There are M surface points with N surface mask entries created for every input point, which leads to a lot of container reallocation and memory shuffling when processing multiple input points. By encapsulating the list, it should become easier to preallocate the entries, as well as keep "helper data" around for managing the bookkeeping to associate the input points with the output points. Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> * Small fixes and TODO reminders. Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> * Encapsulate surface point creation and separate EnumeratePoints out from modifications. Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> * Start removing SurfacePoint from the exposed API. Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> * Changed SurfacePointList to split out the surface point storage to allow for span<> usage over time. Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> * Removed entity id Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> * Removed SurfacePoint from SurfaceData, changed all remaining uses to AzFramework::SurfaceData::SurfacePoint. Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> * Encapsulated SurfaceTagWeightMap and renamed to SurfaceTagWeights. Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> * Fixed make file. Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> * Better commenting and parameter naming. Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> * Renamed methods to be more descriptive. Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com> --- .../SurfaceData/SurfaceDataMeshComponent.cpp | 9 +- .../SurfaceData/SurfaceDataMeshComponent.h | 2 +- .../SurfaceAltitudeGradientComponent.h | 4 +- .../Components/SurfaceMaskGradientComponent.h | 17 +- .../SurfaceSlopeGradientComponent.h | 6 +- .../GradientSurfaceDataComponent.cpp | 27 +- .../SurfaceAltitudeGradientComponent.cpp | 2 +- .../SurfaceMaskGradientComponent.cpp | 2 +- .../SurfaceSlopeGradientComponent.cpp | 2 +- .../EditorGradientSurfaceDataComponent.cpp | 19 +- .../Tests/GradientSignalReferencesTests.cpp | 31 +- .../Code/Tests/GradientSignalSurfaceTests.cpp | 129 ++++--- .../Code/Tests/GradientSignalTestFixtures.cpp | 5 +- Gems/SurfaceData/Code/CMakeLists.txt | 2 +- .../Include/SurfaceData/SurfaceDataTypes.h | 202 ++++++++++- .../SurfaceData/Utility/SurfaceDataUtility.h | 107 +----- .../SurfaceDataColliderComponent.cpp | 33 +- .../Components/SurfaceDataColliderComponent.h | 2 +- .../Components/SurfaceDataShapeComponent.cpp | 27 +- .../Components/SurfaceDataShapeComponent.h | 2 +- .../Source/SurfaceDataSystemComponent.cpp | 104 +----- .../Code/Source/SurfaceDataSystemComponent.h | 3 - .../Code/Source/SurfaceDataTypes.cpp | 319 ++++++++++++++++++ .../Code/Tests/SurfaceDataBenchmarks.cpp | 2 +- .../SurfaceDataColliderComponentTest.cpp | 117 ++++--- .../Code/Tests/SurfaceDataTest.cpp | 157 +++++---- Gems/SurfaceData/Code/surfacedata_files.cmake | 1 + .../TerrainSurfaceDataSystemComponent.cpp | 18 +- .../Vegetation/Ebuses/AreaRequestBus.h | 4 +- .../Code/Include/Vegetation/InstanceData.h | 2 +- .../Code/Source/AreaSystemComponent.cpp | 25 +- .../Components/PositionModifierComponent.cpp | 41 ++- .../Source/Components/SpawnerComponent.cpp | 4 +- .../SurfaceMaskDepthFilterComponent.cpp | 34 +- .../Components/SurfaceMaskFilterComponent.cpp | 10 +- .../Code/Source/Debugger/DebugComponent.cpp | 2 +- .../Tests/VegetationComponentFilterTests.cpp | 5 +- .../VegetationComponentModifierTests.cpp | 2 +- Gems/Vegetation/Code/Tests/VegetationMocks.h | 8 +- 39 files changed, 947 insertions(+), 541 deletions(-) create mode 100644 Gems/SurfaceData/Code/Source/SurfaceDataTypes.cpp diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/SurfaceData/SurfaceDataMeshComponent.cpp b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/SurfaceData/SurfaceDataMeshComponent.cpp index 71e880fdef..2686ca8a16 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/SurfaceData/SurfaceDataMeshComponent.cpp +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/SurfaceData/SurfaceDataMeshComponent.cpp @@ -95,7 +95,7 @@ namespace SurfaceData m_refresh = false; // Update the cached mesh data and bounds, then register the surface data provider - AssignSurfaceTagWeights(m_configuration.m_tags, 1.0f, m_newPointWeights); + m_newPointWeights.AssignSurfaceTagWeights(m_configuration.m_tags, 1.0f); UpdateMeshData(); } @@ -178,12 +178,7 @@ namespace SurfaceData AZ::Vector3 hitNormal; if (DoRayTrace(inPosition, hitPosition, hitNormal)) { - SurfacePoint point; - point.m_entityId = GetEntityId(); - point.m_position = hitPosition; - point.m_normal = hitNormal; - point.m_masks = m_newPointWeights; - surfacePointList.push_back(AZStd::move(point)); + surfacePointList.AddSurfacePoint(GetEntityId(), hitPosition, hitNormal, m_newPointWeights); } } diff --git a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/SurfaceData/SurfaceDataMeshComponent.h b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/SurfaceData/SurfaceDataMeshComponent.h index 758bb1ee99..4a8d3cf427 100644 --- a/Gems/AtomLyIntegration/CommonFeatures/Code/Source/SurfaceData/SurfaceDataMeshComponent.h +++ b/Gems/AtomLyIntegration/CommonFeatures/Code/Source/SurfaceData/SurfaceDataMeshComponent.h @@ -103,6 +103,6 @@ namespace SurfaceData AZ::Transform m_meshWorldTMInverse = AZ::Transform::CreateIdentity(); AZ::Vector3 m_meshNonUniformScale = AZ::Vector3::CreateOne(); AZ::Aabb m_meshBounds = AZ::Aabb::CreateNull(); - SurfaceTagWeightMap m_newPointWeights; + SurfaceTagWeights m_newPointWeights; }; } diff --git a/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceAltitudeGradientComponent.h b/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceAltitudeGradientComponent.h index 6843be8a0e..d8e902a0d5 100644 --- a/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceAltitudeGradientComponent.h +++ b/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceAltitudeGradientComponent.h @@ -109,14 +109,14 @@ namespace GradientSignal private: static float CalculateAltitudeRatio(const SurfaceData::SurfacePointList& points, float altitudeMin, float altitudeMax) { - if (points.empty()) + if (points.IsEmpty()) { return 0.0f; } // GetSurfacePoints (which was used to populate the points list) always returns points in decreasing height order, so the // first point in the list contains the highest altitude. - const float highestAltitude = points.front().m_position.GetZ(); + const float highestAltitude = points.GetHighestSurfacePoint().m_position.GetZ(); // Turn the absolute altitude value into a 0-1 value by returning the % of the given altitude range that it falls at. return GetRatio(altitudeMin, altitudeMax, highestAltitude); diff --git a/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceMaskGradientComponent.h b/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceMaskGradientComponent.h index 2580da10a0..eb009e6b92 100644 --- a/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceMaskGradientComponent.h +++ b/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceMaskGradientComponent.h @@ -85,13 +85,18 @@ namespace GradientSignal { float result = 0.0f; - for (const auto& point : points) + points.EnumeratePoints([&result]( + [[maybe_unused]] const AZ::Vector3& position, [[maybe_unused]] const AZ::Vector3& normal, + const SurfaceData::SurfaceTagWeights& masks) -> bool { - for (const auto& [maskId, weight] : point.m_masks) - { - result = AZ::GetMax(AZ::GetClamp(weight, 0.0f, 1.0f), result); - } - } + masks.EnumerateWeights( + [&result]([[maybe_unused]] AZ::Crc32 surfaceType, float weight) -> bool + { + result = AZ::GetMax(AZ::GetClamp(weight, 0.0f, 1.0f), result); + return true; + }); + return true; + }); return result; } diff --git a/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceSlopeGradientComponent.h b/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceSlopeGradientComponent.h index bdfbcd2b7f..e7a55a2bbc 100644 --- a/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceSlopeGradientComponent.h +++ b/Gems/GradientSignal/Code/Include/GradientSignal/Components/SurfaceSlopeGradientComponent.h @@ -125,7 +125,7 @@ namespace GradientSignal private: float GetSlopeRatio(const SurfaceData::SurfacePointList& points, float angleMin, float angleMax) const { - if (points.empty()) + if (points.IsEmpty()) { return 0.0f; } @@ -133,9 +133,9 @@ namespace GradientSignal // Assuming our surface normal vector is actually normalized, we can get the slope // by just grabbing the Z value. It's the same thing as normal.Dot(AZ::Vector3::CreateAxisZ()). AZ_Assert( - points.front().m_normal.GetNormalized().IsClose(points.front().m_normal), + points.GetHighestSurfacePoint().m_normal.GetNormalized().IsClose(points.GetHighestSurfacePoint().m_normal), "Surface normals are expected to be normalized"); - const float slope = points.front().m_normal.GetZ(); + const float slope = points.GetHighestSurfacePoint().m_normal.GetZ(); // Convert slope back to an angle so that we can lerp in "angular space", not "slope value space". // (We want our 0-1 range to be linear across the range of angles) const float slopeAngle = acosf(slope); diff --git a/Gems/GradientSignal/Code/Source/Components/GradientSurfaceDataComponent.cpp b/Gems/GradientSignal/Code/Source/Components/GradientSurfaceDataComponent.cpp index 5a0555fe33..cd87e3044c 100644 --- a/Gems/GradientSignal/Code/Source/Components/GradientSurfaceDataComponent.cpp +++ b/Gems/GradientSignal/Code/Source/Components/GradientSurfaceDataComponent.cpp @@ -224,10 +224,9 @@ namespace GradientSignal validShapeBounds = m_cachedShapeConstraintBounds.IsValid(); } - const AZ::EntityId entityId = GetEntityId(); - for (auto& point : surfacePointList) - { - if (point.m_entityId != entityId) + surfacePointList.ModifySurfaceWeights( + GetEntityId(), + [this, validShapeBounds, shapeConstraintBounds](const AZ::Vector3& position, SurfaceData::SurfaceTagWeights& weights) { bool inBounds = true; @@ -236,28 +235,26 @@ namespace GradientSignal if (validShapeBounds) { inBounds = false; - if (shapeConstraintBounds.Contains(point.m_position)) + if (shapeConstraintBounds.Contains(position)) { - LmbrCentral::ShapeComponentRequestsBus::EventResult(inBounds, m_configuration.m_shapeConstraintEntityId, - &LmbrCentral::ShapeComponentRequestsBus::Events::IsPointInside, point.m_position); + LmbrCentral::ShapeComponentRequestsBus::EventResult( + inBounds, m_configuration.m_shapeConstraintEntityId, + &LmbrCentral::ShapeComponentRequestsBus::Events::IsPointInside, position); } } // If the point is within our allowed shape bounds, verify that it meets the gradient thresholds. - // If so, then add the value to the surface tags. + // If so, then return the value to add to the surface tags. if (inBounds) { - const GradientSampleParams sampleParams = { point.m_position }; + const GradientSampleParams sampleParams = { position }; const float value = m_gradientSampler.GetValue(sampleParams); - if (value >= m_configuration.m_thresholdMin && - value <= m_configuration.m_thresholdMax) + if (value >= m_configuration.m_thresholdMin && value <= m_configuration.m_thresholdMax) { - SurfaceData::AddMaxValueForMasks(point.m_masks, m_configuration.m_modifierTags, value); + weights.AddSurfaceWeightsIfGreater(m_configuration.m_modifierTags, value); } } - - } - } + }); } } diff --git a/Gems/GradientSignal/Code/Source/Components/SurfaceAltitudeGradientComponent.cpp b/Gems/GradientSignal/Code/Source/Components/SurfaceAltitudeGradientComponent.cpp index 1670483131..b79606818b 100644 --- a/Gems/GradientSignal/Code/Source/Components/SurfaceAltitudeGradientComponent.cpp +++ b/Gems/GradientSignal/Code/Source/Components/SurfaceAltitudeGradientComponent.cpp @@ -234,7 +234,7 @@ namespace GradientSignal // For each position, call GetSurfacePoints() and turn the height into a 0-1 value based on our min/max altitudes. for (size_t index = 0; index < positions.size(); index++) { - points.clear(); + points.Clear(); surfaceDataRequests->GetSurfacePoints(positions[index], m_configuration.m_surfaceTagsToSample, points); outValues[index] = CalculateAltitudeRatio(points, m_configuration.m_altitudeMin, m_configuration.m_altitudeMax); } diff --git a/Gems/GradientSignal/Code/Source/Components/SurfaceMaskGradientComponent.cpp b/Gems/GradientSignal/Code/Source/Components/SurfaceMaskGradientComponent.cpp index ea72971818..56bb846ca4 100644 --- a/Gems/GradientSignal/Code/Source/Components/SurfaceMaskGradientComponent.cpp +++ b/Gems/GradientSignal/Code/Source/Components/SurfaceMaskGradientComponent.cpp @@ -198,7 +198,7 @@ namespace GradientSignal for (size_t index = 0; index < positions.size(); index++) { - points.clear(); + points.Clear(); surfaceDataRequests->GetSurfacePoints(positions[index], m_configuration.m_surfaceTagList, points); outValues[index] = GetMaxSurfaceWeight(points); } diff --git a/Gems/GradientSignal/Code/Source/Components/SurfaceSlopeGradientComponent.cpp b/Gems/GradientSignal/Code/Source/Components/SurfaceSlopeGradientComponent.cpp index e557785509..fdfa9dd5f8 100644 --- a/Gems/GradientSignal/Code/Source/Components/SurfaceSlopeGradientComponent.cpp +++ b/Gems/GradientSignal/Code/Source/Components/SurfaceSlopeGradientComponent.cpp @@ -239,7 +239,7 @@ namespace GradientSignal for (size_t index = 0; index < positions.size(); index++) { - points.clear(); + points.Clear(); surfaceDataRequests->GetSurfacePoints(positions[index], m_configuration.m_surfaceTagsToSample, points); outValues[index] = GetSlopeRatio(points, angleMin, angleMax); } diff --git a/Gems/GradientSignal/Code/Source/Editor/EditorGradientSurfaceDataComponent.cpp b/Gems/GradientSignal/Code/Source/Editor/EditorGradientSurfaceDataComponent.cpp index b715df1f92..e318e0fdf6 100644 --- a/Gems/GradientSignal/Code/Source/Editor/EditorGradientSurfaceDataComponent.cpp +++ b/Gems/GradientSignal/Code/Source/Editor/EditorGradientSurfaceDataComponent.cpp @@ -98,10 +98,9 @@ namespace GradientSignal return [this]([[maybe_unused]] float sampleValue, const GradientSampleParams& params) { // Create a fake surface point with the position we're sampling. - SurfaceData::SurfacePoint point; + AzFramework::SurfaceData::SurfacePoint point; point.m_position = params.m_position; - SurfaceData::SurfacePointList pointList; - pointList.emplace_back(point); + SurfaceData::SurfacePointList pointList = { { point } }; // Send it into the component, see what emerges m_component.ModifySurfacePoints(pointList); @@ -110,10 +109,18 @@ namespace GradientSignal // Technically, they should all have the same value, but we'll grab the max from all of them in case // the underlying logic ever changes to allow separate ranges per tag. float result = 0.0f; - for (auto& mask : pointList[0].m_masks) + pointList.EnumeratePoints([&result]( + [[maybe_unused]] const AZ::Vector3& position, [[maybe_unused]] const AZ::Vector3& normal, + const SurfaceData::SurfaceTagWeights& masks) -> bool { - result = AZ::GetMax(result, mask.second); - } + masks.EnumerateWeights( + [&result]([[maybe_unused]] AZ::Crc32 surfaceType, float weight) -> bool + { + result = AZ::GetMax(result, weight); + return true; + }); + return true; + }); return result; }; } diff --git a/Gems/GradientSignal/Code/Tests/GradientSignalReferencesTests.cpp b/Gems/GradientSignal/Code/Tests/GradientSignalReferencesTests.cpp index 72bdfa202e..6f90c5022f 100644 --- a/Gems/GradientSignal/Code/Tests/GradientSignalReferencesTests.cpp +++ b/Gems/GradientSignal/Code/Tests/GradientSignalReferencesTests.cpp @@ -66,7 +66,7 @@ namespace UnitTest float falloffMidpoint, float falloffRange, float falloffStrength) { MockSurfaceDataSystem mockSurfaceDataSystem; - SurfaceData::SurfacePoint point; + AzFramework::SurfaceData::SurfacePoint point; // Fill our mock surface with the correct normal value for each point based on our test angle set. for (int y = 0; y < dataSize; y++) @@ -539,10 +539,10 @@ namespace UnitTest // Set a different altitude for each point we're going to test. We'll use 0, 2, 5, 10 to test various points along the range. MockSurfaceDataSystem mockSurfaceDataSystem; - mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(0.0f, 0.0f)] = { { entityShape->GetId(), AZ::Vector3(0.0f, 0.0f, 0.0f), AZ::Vector3::CreateZero() } }; - mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(1.0f, 0.0f)] = { { entityShape->GetId(), AZ::Vector3(0.0f, 0.0f, 2.0f), AZ::Vector3::CreateZero() } }; - mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(0.0f, 1.0f)] = { { entityShape->GetId(), AZ::Vector3(0.0f, 0.0f, 5.0f), AZ::Vector3::CreateZero() } }; - mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(1.0f, 1.0f)] = { { entityShape->GetId(), AZ::Vector3(0.0f, 0.0f, 10.0f), AZ::Vector3::CreateZero() } }; + mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(0.0f, 0.0f)] = { { AZ::Vector3(0.0f, 0.0f, 0.0f), AZ::Vector3::CreateZero() } }; + mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(1.0f, 0.0f)] = { { AZ::Vector3(0.0f, 0.0f, 2.0f), AZ::Vector3::CreateZero() } }; + mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(0.0f, 1.0f)] = { { AZ::Vector3(0.0f, 0.0f, 5.0f), AZ::Vector3::CreateZero() } }; + mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(1.0f, 1.0f)] = { { AZ::Vector3(0.0f, 0.0f, 10.0f), AZ::Vector3::CreateZero() } }; // We set the min/max to values other than 0-10 to help validate that they aren't used in the case of the pinned shape. GradientSignal::SurfaceAltitudeGradientConfig config; @@ -573,10 +573,10 @@ namespace UnitTest // Set a different altitude for each point we're going to test. We'll use 0, 2, 5, 10 to test various points along the range. MockSurfaceDataSystem mockSurfaceDataSystem; - mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(0.0f, 0.0f)] = { { entityShape->GetId(), AZ::Vector3(0.0f, 0.0f, 0.0f), AZ::Vector3::CreateZero() } }; - mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(1.0f, 0.0f)] = { { entityShape->GetId(), AZ::Vector3(0.0f, 0.0f, 2.0f), AZ::Vector3::CreateZero() } }; - mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(0.0f, 1.0f)] = { { entityShape->GetId(), AZ::Vector3(0.0f, 0.0f, 5.0f), AZ::Vector3::CreateZero() } }; - mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(1.0f, 1.0f)] = { { entityShape->GetId(), AZ::Vector3(0.0f, 0.0f, 10.0f), AZ::Vector3::CreateZero() } }; + mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(0.0f, 0.0f)] = { { AZ::Vector3(0.0f, 0.0f, 0.0f), AZ::Vector3::CreateZero() } }; + mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(1.0f, 0.0f)] = { { AZ::Vector3(0.0f, 0.0f, 2.0f), AZ::Vector3::CreateZero() } }; + mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(0.0f, 1.0f)] = { { AZ::Vector3(0.0f, 0.0f, 5.0f), AZ::Vector3::CreateZero() } }; + mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(1.0f, 1.0f)] = { { AZ::Vector3(0.0f, 0.0f, 10.0f), AZ::Vector3::CreateZero() } }; // We set the min/max to 0-10, but don't set a shape. GradientSignal::SurfaceAltitudeGradientConfig config; @@ -634,13 +634,13 @@ namespace UnitTest MockSurfaceDataSystem mockSurfaceDataSystem; // Altitude value below min - should result in 0.0f. - mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(0.0f, 0.0f)] = { { entityShape->GetId(), AZ::Vector3(0.0f, 0.0f, -10.0f), AZ::Vector3::CreateZero() } }; + mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(0.0f, 0.0f)] = { { AZ::Vector3(0.0f, 0.0f, -10.0f), AZ::Vector3::CreateZero() } }; // Altitude value at exactly min - should result in 0.0f. - mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(1.0f, 0.0f)] = { { entityShape->GetId(), AZ::Vector3(0.0f, 0.0f, -5.0f), AZ::Vector3::CreateZero() } }; + mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(1.0f, 0.0f)] = { { AZ::Vector3(0.0f, 0.0f, -5.0f), AZ::Vector3::CreateZero() } }; // Altitude value at exactly max - should result in 1.0f. - mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(0.0f, 1.0f)] = { { entityShape->GetId(), AZ::Vector3(0.0f, 0.0f, 15.0f), AZ::Vector3::CreateZero() } }; + mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(0.0f, 1.0f)] = { { AZ::Vector3(0.0f, 0.0f, 15.0f), AZ::Vector3::CreateZero() } }; // Altitude value above max - should result in 1.0f. - mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(1.0f, 1.0f)] = { { entityShape->GetId(), AZ::Vector3(0.0f, 0.0f, 20.0f), AZ::Vector3::CreateZero() } }; + mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(1.0f, 1.0f)] = { { AZ::Vector3(0.0f, 0.0f, 20.0f), AZ::Vector3::CreateZero() } }; // We set the min/max to -5 - 15. By using a range without 0 at either end, and not having 0 as the midpoint, // it should be easier to verify that we're successfully clamping to 0 and 1. @@ -668,14 +668,15 @@ namespace UnitTest }; MockSurfaceDataSystem mockSurfaceDataSystem; - SurfaceData::SurfacePoint point; + AzFramework::SurfaceData::SurfacePoint point; // Fill our mock surface with the test_mask set and the expected gradient value at each point. for (int y = 0; y < dataSize; y++) { for (int x = 0; x < dataSize; x++) { - point.m_masks[AZ_CRC("test_mask", 0x7a16e9ff)] = expectedOutput[(y * dataSize) + x]; + point.m_surfaceTags.clear(); + point.m_surfaceTags.emplace_back(AZ_CRC_CE("test_mask"), expectedOutput[(y * dataSize) + x]); mockSurfaceDataSystem.m_GetSurfacePoints[AZStd::make_pair(static_cast(x), static_cast(y))] = { { point } }; } } diff --git a/Gems/GradientSignal/Code/Tests/GradientSignalSurfaceTests.cpp b/Gems/GradientSignal/Code/Tests/GradientSignalSurfaceTests.cpp index a716fc79b8..9f4346e09e 100644 --- a/Gems/GradientSignal/Code/Tests/GradientSignalSurfaceTests.cpp +++ b/Gems/GradientSignal/Code/Tests/GradientSignalSurfaceTests.cpp @@ -19,27 +19,53 @@ namespace UnitTest struct GradientSignalSurfaceTestsFixture : public GradientSignalTest { - void SetSurfacePoint(SurfaceData::SurfacePoint& point, AZ::EntityId id, AZ::Vector3 position, AZ::Vector3 normal, AZStd::vector> tags) + void SetSurfacePoint(AzFramework::SurfaceData::SurfacePoint& point, AZ::Vector3 position, AZ::Vector3 normal, AZStd::vector> tags) { - point.m_entityId = id; point.m_position = position; point.m_normal = normal; for (auto& tag : tags) { - point.m_masks[SurfaceData::SurfaceTag(tag.first)] = tag.second; + point.m_surfaceTags.emplace_back(SurfaceData::SurfaceTag(tag.first), tag.second); } } - bool SurfacePointsAreEqual(const SurfaceData::SurfacePoint& lhs, const SurfaceData::SurfacePoint& rhs) + bool SurfacePointsAreEqual(const AzFramework::SurfaceData::SurfacePoint& lhs, const AzFramework::SurfaceData::SurfacePoint& rhs) { - return (lhs.m_entityId == rhs.m_entityId) - && (lhs.m_position == rhs.m_position) - && (lhs.m_normal == rhs.m_normal) - && (lhs.m_masks == rhs.m_masks); + if ((lhs.m_position != rhs.m_position) || (lhs.m_normal != rhs.m_normal) + || (lhs.m_surfaceTags.size() != rhs.m_surfaceTags.size())) + { + return false; + } + + for (auto& mask : lhs.m_surfaceTags) + { + auto maskEntry = AZStd::find_if( + rhs.m_surfaceTags.begin(), rhs.m_surfaceTags.end(), + [mask](const AzFramework::SurfaceData::SurfaceTagWeight& weight) -> bool + { + return (mask.m_surfaceType == weight.m_surfaceType) && (mask.m_weight == weight.m_weight); + }); + if (maskEntry == rhs.m_surfaceTags.end()) + { + return false; + } + } + + return true; + } + + bool SurfacePointsAreEqual( + const AZ::Vector3& lhsPosition, const AZ::Vector3& lhsNormal, const SurfaceData::SurfaceTagWeights& lhsWeights, + const AzFramework::SurfaceData::SurfacePoint& rhs) + { + return ((lhsPosition == rhs.m_position) + && (lhsNormal == rhs.m_normal) + && (lhsWeights.SurfaceWeightsAreEqual(rhs.m_surfaceTags))); } void TestGradientSurfaceDataComponent(float gradientValue, float thresholdMin, float thresholdMax, AZStd::vector tags, bool usesShape, - const SurfaceData::SurfacePoint& input, const SurfaceData::SurfacePoint& expectedOutput) + const AzFramework::SurfaceData::SurfacePoint& input, + const AzFramework::SurfaceData::SurfacePoint& expectedOutput) { // This lets our component register with surfaceData successfully. MockSurfaceDataSystem mockSurfaceDataSystem; @@ -83,10 +109,17 @@ namespace UnitTest EXPECT_TRUE(modifierHandle != SurfaceData::InvalidSurfaceDataRegistryHandle); // Call ModifySurfacePoints and verify the results - SurfaceData::SurfacePointList pointList; - pointList.emplace_back(input); + SurfaceData::SurfacePointList pointList = { { input } }; SurfaceData::SurfaceDataModifierRequestBus::Event(modifierHandle, &SurfaceData::SurfaceDataModifierRequestBus::Events::ModifySurfacePoints, pointList); - EXPECT_TRUE(SurfacePointsAreEqual(pointList[0],expectedOutput)); + ASSERT_EQ(pointList.GetSize(), 1); + pointList.EnumeratePoints( + [this, expectedOutput]( + const AZ::Vector3& position, const AZ::Vector3& normal, + const SurfaceData::SurfaceTagWeights& masks) + { + EXPECT_TRUE(SurfacePointsAreEqual(position, normal, masks, expectedOutput)); + return true; + }); } @@ -97,17 +130,17 @@ namespace UnitTest // Verify that for a gradient value within the threshold, the output point contains the // correct tag and gradient value. - SurfaceData::SurfacePoint input; - SurfaceData::SurfacePoint expectedOutput; + AzFramework::SurfaceData::SurfacePoint input; + AzFramework::SurfaceData::SurfacePoint expectedOutput; const char* tag = "test_mask"; // Select a gradient value within the threshold range below float gradientValue = 0.5f; // Set arbitrary input data - SetSurfacePoint(input, AZ::EntityId(0x12345678), AZ::Vector3(1.0f), AZ::Vector3(0.0f), {}); + SetSurfacePoint(input, AZ::Vector3(1.0f), AZ::Vector3(0.0f), {}); // Output should match the input, but with an added tag / value - SetSurfacePoint(expectedOutput, input.m_entityId, input.m_position, input.m_normal, { AZStd::make_pair(tag, gradientValue) }); + SetSurfacePoint(expectedOutput, input.m_position, input.m_normal, { AZStd::make_pair(tag, gradientValue) }); TestGradientSurfaceDataComponent( gradientValue, // constant gradient value @@ -123,17 +156,17 @@ namespace UnitTest { // Verify that for a gradient value outside the threshold, the output point contains no tags / values. - SurfaceData::SurfacePoint input; - SurfaceData::SurfacePoint expectedOutput; + AzFramework::SurfaceData::SurfacePoint input; + AzFramework::SurfaceData::SurfacePoint expectedOutput; const char* tag = "test_mask"; // Choose a value outside the threshold range float gradientValue = 0.05f; // Set arbitrary input data - SetSurfacePoint(input, AZ::EntityId(0x12345678), AZ::Vector3(1.0f), AZ::Vector3(0.0f), {}); + SetSurfacePoint(input, AZ::Vector3(1.0f), AZ::Vector3(0.0f), {}); // Output should match the input - no extra tags / values should be added. - SetSurfacePoint(expectedOutput, input.m_entityId, input.m_position, input.m_normal, {}); + SetSurfacePoint(expectedOutput, input.m_position, input.m_normal, {}); TestGradientSurfaceDataComponent( gradientValue, // constant gradient value @@ -149,8 +182,8 @@ namespace UnitTest { // Verify that if the component has multiple tags, all of them get put on the output with the same gradient value. - SurfaceData::SurfacePoint input; - SurfaceData::SurfacePoint expectedOutput; + AzFramework::SurfaceData::SurfacePoint input; + AzFramework::SurfaceData::SurfacePoint expectedOutput; const char* tag1 = "test_mask1"; const char* tag2 = "test_mask2"; @@ -158,9 +191,9 @@ namespace UnitTest float gradientValue = 0.5f; // Set arbitrary input data - SetSurfacePoint(input, AZ::EntityId(0x12345678), AZ::Vector3(1.0f), AZ::Vector3(0.0f), {}); + SetSurfacePoint(input, AZ::Vector3(1.0f), AZ::Vector3(0.0f), {}); // Output should match the input, but with two added tags - SetSurfacePoint(expectedOutput, input.m_entityId, input.m_position, input.m_normal, + SetSurfacePoint(expectedOutput, input.m_position, input.m_normal, { AZStd::make_pair(tag1, gradientValue), AZStd::make_pair(tag2, gradientValue) }); TestGradientSurfaceDataComponent( @@ -178,8 +211,8 @@ namespace UnitTest // Verify that the output contains input tags that are NOT on the modification list and adds any // new tags that weren't in the input - SurfaceData::SurfacePoint input; - SurfaceData::SurfacePoint expectedOutput; + AzFramework::SurfaceData::SurfacePoint input; + AzFramework::SurfaceData::SurfacePoint expectedOutput; const char* preservedTag = "preserved_tag"; const char* modifierTag = "modifier_tag"; @@ -187,9 +220,9 @@ namespace UnitTest float gradientValue = 0.5f; // Set arbitrary input data - SetSurfacePoint(input, AZ::EntityId(0x12345678), AZ::Vector3(1.0f), AZ::Vector3(0.0f), { AZStd::make_pair(preservedTag, 1.0f) }); + SetSurfacePoint(input, AZ::Vector3(1.0f), AZ::Vector3(0.0f), { AZStd::make_pair(preservedTag, 1.0f) }); // Output should match the input, but with two added tags - SetSurfacePoint(expectedOutput, input.m_entityId, input.m_position, input.m_normal, + SetSurfacePoint(expectedOutput, input.m_position, input.m_normal, { AZStd::make_pair(preservedTag, 1.0f), AZStd::make_pair(modifierTag, gradientValue) }); TestGradientSurfaceDataComponent( @@ -206,8 +239,8 @@ namespace UnitTest { // Verify that if the input has a higher value on the tag than the modifier, it keeps the higher value. - SurfaceData::SurfacePoint input; - SurfaceData::SurfacePoint expectedOutput; + AzFramework::SurfaceData::SurfacePoint input; + AzFramework::SurfaceData::SurfacePoint expectedOutput; const char* tag = "test_mask"; // Select a gradient value within the threshold range below @@ -216,9 +249,9 @@ namespace UnitTest float inputValue = 0.75f; // Set arbitrary input data - SetSurfacePoint(input, AZ::EntityId(0x12345678), AZ::Vector3(1.0f), AZ::Vector3(0.0f), { AZStd::make_pair(tag, inputValue) }); + SetSurfacePoint(input, AZ::Vector3(1.0f), AZ::Vector3(0.0f), { AZStd::make_pair(tag, inputValue) }); // Output should match the input - the higher input value on the tag is preserved - SetSurfacePoint(expectedOutput, input.m_entityId, input.m_position, input.m_normal, + SetSurfacePoint(expectedOutput, input.m_position, input.m_normal, { AZStd::make_pair(tag, inputValue) }); TestGradientSurfaceDataComponent( @@ -235,8 +268,8 @@ namespace UnitTest { // Verify that if the input has a lower value on the tag than the modifier, it keeps the higher value. - SurfaceData::SurfacePoint input; - SurfaceData::SurfacePoint expectedOutput; + AzFramework::SurfaceData::SurfacePoint input; + AzFramework::SurfaceData::SurfacePoint expectedOutput; const char* tag = "test_mask"; // Select a gradient value within the threshold range below @@ -245,9 +278,9 @@ namespace UnitTest float inputValue = 0.25f; // Set arbitrary input data - SetSurfacePoint(input, AZ::EntityId(0x12345678), AZ::Vector3(1.0f), AZ::Vector3(0.0f), { AZStd::make_pair(tag, inputValue) }); + SetSurfacePoint(input, AZ::Vector3(1.0f), AZ::Vector3(0.0f), { AZStd::make_pair(tag, inputValue) }); // Output should match the input, except that the value on the tag gets the higher modifier value - SetSurfacePoint(expectedOutput, input.m_entityId, input.m_position, input.m_normal, + SetSurfacePoint(expectedOutput, input.m_position, input.m_normal, { AZStd::make_pair(tag, gradientValue) }); TestGradientSurfaceDataComponent( @@ -264,17 +297,17 @@ namespace UnitTest { // Verify that if no shape has been added, the component modifies points in unbounded space - SurfaceData::SurfacePoint input; - SurfaceData::SurfacePoint expectedOutput; + AzFramework::SurfaceData::SurfacePoint input; + AzFramework::SurfaceData::SurfacePoint expectedOutput; const char* tag = "test_mask"; // Select a gradient value within the threshold range below float gradientValue = 0.5f; // Set arbitrary input data, but with a point that's extremely far away in space - SetSurfacePoint(input, AZ::EntityId(0x12345678), AZ::Vector3(-100000000.0f), AZ::Vector3(0.0f), {}); + SetSurfacePoint(input, AZ::Vector3(-100000000.0f), AZ::Vector3(0.0f), {}); // Output should match the input but with the tag added, even though the point was far away. - SetSurfacePoint(expectedOutput, input.m_entityId, input.m_position, input.m_normal, + SetSurfacePoint(expectedOutput, input.m_position, input.m_normal, { AZStd::make_pair(tag, gradientValue) }); TestGradientSurfaceDataComponent( @@ -292,17 +325,17 @@ namespace UnitTest // Verify that if a shape constraint is added, points within the shape are still modified. // Our default mock shape is a cube that exists from -0.5 to 0.5 in space. - SurfaceData::SurfacePoint input; - SurfaceData::SurfacePoint expectedOutput; + AzFramework::SurfaceData::SurfacePoint input; + AzFramework::SurfaceData::SurfacePoint expectedOutput; const char* tag = "test_mask"; // Select a gradient value within the threshold range below float gradientValue = 0.5f; // Set arbitrary input data, but with a point that's within the mock shape cube (0.25 vs -0.5 to 0.5) - SetSurfacePoint(input, AZ::EntityId(0x12345678), AZ::Vector3(0.25f), AZ::Vector3(0.0f), {}); + SetSurfacePoint(input, AZ::Vector3(0.25f), AZ::Vector3(0.0f), {}); // Output should match the input but with the tag added, since the point is within the shape constraint. - SetSurfacePoint(expectedOutput, input.m_entityId, input.m_position, input.m_normal, + SetSurfacePoint(expectedOutput, input.m_position, input.m_normal, { AZStd::make_pair(tag, gradientValue) }); TestGradientSurfaceDataComponent( @@ -320,17 +353,17 @@ namespace UnitTest // Verify that if a shape constraint is added, points outside the shape are not modified. // Our default mock shape is a cube that exists from -0.5 to 0.5 in space. - SurfaceData::SurfacePoint input; - SurfaceData::SurfacePoint expectedOutput; + AzFramework::SurfaceData::SurfacePoint input; + AzFramework::SurfaceData::SurfacePoint expectedOutput; const char* tag = "test_mask"; // Select a gradient value within the threshold range below float gradientValue = 0.5f; // Set arbitrary input data, but with a point that's outside the mock shape cube (10.0 vs -0.5 to 0.5) - SetSurfacePoint(input, AZ::EntityId(0x12345678), AZ::Vector3(10.0f), AZ::Vector3(0.0f), {}); + SetSurfacePoint(input, AZ::Vector3(10.0f), AZ::Vector3(0.0f), {}); // Output should match the input with no tag added, since the point is outside the shape constraint - SetSurfacePoint(expectedOutput, input.m_entityId, input.m_position, input.m_normal, {}); + SetSurfacePoint(expectedOutput, input.m_position, input.m_normal, {}); TestGradientSurfaceDataComponent( gradientValue, // constant gradient value diff --git a/Gems/GradientSignal/Code/Tests/GradientSignalTestFixtures.cpp b/Gems/GradientSignal/Code/Tests/GradientSignalTestFixtures.cpp index d81370f6a8..402438ee88 100644 --- a/Gems/GradientSignal/Code/Tests/GradientSignalTestFixtures.cpp +++ b/Gems/GradientSignal/Code/Tests/GradientSignalTestFixtures.cpp @@ -84,7 +84,7 @@ namespace UnitTest AZStd::unique_ptr GradientSignalBaseFixture::CreateMockSurfaceDataSystem(const AZ::Aabb& spawnerBox) { - SurfaceData::SurfacePoint point; + AzFramework::SurfaceData::SurfacePoint point; AZStd::unique_ptr mockSurfaceDataSystem = AZStd::make_unique(); // Give the mock surface data a bunch of fake point values to return. @@ -101,7 +101,8 @@ namespace UnitTest // Create an arbitrary normal value. point.m_normal = point.m_position.GetNormalized(); // Create an arbitrary surface value. - point.m_masks[AZ_CRC_CE("test_mask")] = arbitraryPercentage; + point.m_surfaceTags.clear(); + point.m_surfaceTags.emplace_back(AZ_CRC_CE("test_mask"), arbitraryPercentage); mockSurfaceDataSystem->m_GetSurfacePoints[AZStd::make_pair(x, y)] = { { point } }; } diff --git a/Gems/SurfaceData/Code/CMakeLists.txt b/Gems/SurfaceData/Code/CMakeLists.txt index 6cef8b6e25..cb75be2de4 100644 --- a/Gems/SurfaceData/Code/CMakeLists.txt +++ b/Gems/SurfaceData/Code/CMakeLists.txt @@ -37,7 +37,7 @@ ly_add_target( PUBLIC Include BUILD_DEPENDENCIES - PRIVATE + PUBLIC Gem::SurfaceData.Static Gem::LmbrCentral RUNTIME_DEPENDENCIES diff --git a/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataTypes.h b/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataTypes.h index 6d39efaa9f..c892bc8c09 100644 --- a/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataTypes.h +++ b/Gems/SurfaceData/Code/Include/SurfaceData/SurfaceDataTypes.h @@ -9,31 +9,215 @@ #pragma once #include +#include #include #include #include #include +#include #include namespace SurfaceData { //map of id or crc to contribution factor - using SurfaceTagWeightMap = AZStd::unordered_map; using SurfaceTagNameSet = AZStd::unordered_set; using SurfaceTagVector = AZStd::vector; - struct SurfacePoint final + //! SurfaceTagWeights stores a collection of surface tags and weights. + class SurfaceTagWeights { - AZ_CLASS_ALLOCATOR(SurfacePoint, AZ::SystemAllocator, 0); - AZ_TYPE_INFO(SurfacePoint, "{0DC7E720-68D6-47D4-BB6D-B89EF23C5A5C}"); + public: + SurfaceTagWeights() = default; - AZ::EntityId m_entityId; - AZ::Vector3 m_position; - AZ::Vector3 m_normal; - SurfaceTagWeightMap m_masks; + //! Construct a collection of SurfaceTagWeights from the given SurfaceTagWeightList. + //! @param weights - The list of weights to assign to the new instance. + SurfaceTagWeights(const AzFramework::SurfaceData::SurfaceTagWeightList& weights) + { + AssignSurfaceTagWeights(weights); + } + + //! Replace the existing surface tag weights with the given set. + //! @param weights - The list of weights to assign to this instance. + void AssignSurfaceTagWeights(const AzFramework::SurfaceData::SurfaceTagWeightList& weights); + + //! Replace the existing surface tag weights with the given set. + //! @param tags - The list of tags to assign to this instance. + //! @param weight - The weight to assign to each tag. + void AssignSurfaceTagWeights(const SurfaceTagVector& tags, float weight); + + //! Add a surface tag weight to this collection. + //! @param tag - The surface tag. + //! @param weight - The surface tag weight. + void AddSurfaceTagWeight(const AZ::Crc32 tag, const float weight); + + //! Replace the surface tag weight with the new one if it's higher, or add it if the tag isn't found. + //! (This method is intentionally inlined for its performance impact) + //! @param tag - The surface tag. + //! @param weight - The surface tag weight. + void AddSurfaceWeightIfGreater(const AZ::Crc32 tag, const float weight) + { + const auto maskItr = m_weights.find(tag); + const float previousValue = maskItr != m_weights.end() ? maskItr->second : 0.0f; + m_weights[tag] = AZ::GetMax(weight, previousValue); + } + + //! Replace the surface tag weight with the new one if it's higher, or add it if the tag isn't found. + //! (This method is intentionally inlined for its performance impact) + //! @param tags - The surface tags to replace/add. + //! @param weight - The surface tag weight to use for each tag. + void AddSurfaceWeightsIfGreater(const SurfaceTagVector& tags, const float weight) + { + for (const auto& tag : tags) + { + AddSurfaceWeightIfGreater(tag, weight); + } + } + + //! Replace the surface tag weight with the new one if it's higher, or add it if the tag isn't found. + //! (This method is intentionally inlined for its performance impact) + //! @param weights - The surface tags and weights to replace/add. + void AddSurfaceWeightsIfGreater(const SurfaceTagWeights& weights) + { + for (const auto& [tag, weight] : weights.m_weights) + { + AddSurfaceWeightIfGreater(tag, weight); + } + } + + //! Equality comparison operator for SurfaceTagWeights. + bool operator==(const SurfaceTagWeights& rhs) const; + + //! Inequality comparison operator for SurfaceTagWeights. + bool operator!=(const SurfaceTagWeights& rhs) const + { + return !(*this == rhs); + } + + //! Compares a SurfaceTagWeightList with a SurfaceTagWeights instance to look for equality. + //! They will be equal if they have the exact same set of tags and weights. + //! @param compareWeights - the set of weights to compare against. + bool SurfaceWeightsAreEqual(const AzFramework::SurfaceData::SurfaceTagWeightList& compareWeights) const; + + //! Clear the surface tag weight collection. + void Clear(); + + //! Get the size of the surface tag weight collection. + //! @return The size of the collection. + size_t GetSize() const; + + //! Get the collection of surface tag weights as a SurfaceTagWeightList. + //! @return SurfaceTagWeightList containing the same tags and weights as this collection. + AzFramework::SurfaceData::SurfaceTagWeightList GetSurfaceTagWeightList() const; + + //! Enumerate every tag and weight and call a callback for each one found. + //! Callback params: + //! AZ::Crc32 - The surface tag. + //! float - The surface tag weight. + //! return - true to keep enumerating, false to stop. + //! @param weightCallback - the callback to use for each surface tag / weight found. + void EnumerateWeights(AZStd::function weightCallback) const; + + //! Check to see if the collection has any valid tags stored within it. + //! A tag of "Unassigned" is considered an invalid tag. + //! @return True if there is at least one valid tag, false if there isn't. + bool HasValidTags() const; + + //! Check to see if the collection contains the given tag. + //! @param sampleTag - The tag to look for. + //! @return True if the tag is found, false if it isn't. + bool HasMatchingTag(const AZ::Crc32& sampleTag) const; + + //! Check to see if the collection contains the given tag with the given weight range. + //! The range check is inclusive on both sides of the range: [weightMin, weightMax] + //! @param sampleTag - The tag to look for. + //! @param weightMin - The minimum weight for this tag. + //! @param weightMax - The maximum weight for this tag. + //! @return True if the tag is found, false if it isn't. + bool HasMatchingTag(const AZ::Crc32& sampleTag, float weightMin, float weightMax) const; + + //! Check to see if the collection contains any of the given tags. + //! @param sampleTags - The tags to look for. + //! @return True if any of the tags is found, false if none are found. + bool HasAnyMatchingTags(const SurfaceTagVector& sampleTags) const; + + //! Check to see if the collection contains the given tag with the given weight range. + //! The range check is inclusive on both sides of the range: [weightMin, weightMax] + //! @param sampleTags - The tags to look for. + //! @param weightMin - The minimum weight for this tag. + //! @param weightMax - The maximum weight for this tag. + //! @return True if any of the tags is found, false if none are found. + bool HasAnyMatchingTags(const SurfaceTagVector& sampleTags, float weightMin, float weightMax) const; + + private: + AZStd::unordered_map m_weights; + }; + + //! SurfacePointList stores a collection of surface point data, which consists of positions, normals, and surface tag weights. + class SurfacePointList + { + public: + AZ_CLASS_ALLOCATOR(SurfacePointList, AZ::SystemAllocator, 0); + AZ_TYPE_INFO(SurfacePointList, "{DBA02848-2131-4279-BDEF-3581B76AB736}"); + + SurfacePointList() = default; + ~SurfacePointList() = default; + + //! Constructor for creating a SurfacePointList from a list of SurfacePoint data. + //! Primarily used as a convenience for unit tests. + //! @param surfacePoints - An initial set of SurfacePoint points to store in the SurfacePointList. + SurfacePointList(AZStd::initializer_list surfacePoints); + + //! Add a surface point to the list. + //! @param entityId - The entity creating the surface point. + //! @param position - The position of the surface point. + //! @param normal - The normal for the surface point. + //! @param weights - The surface tags and weights for this surface point. + void AddSurfacePoint(const AZ::EntityId& entityId, + const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceTagWeights& weights); + + //! Clear the surface point list. + void Clear(); + + //! Preallocate space in the list based on the maximum number of output points per input point we can generate. + //! @param maxPointsPerInput - The maximum number of output points per input point. + void ReserveSpace(size_t maxPointsPerInput); + + //! Check if the surface point list is empty. + //! @return - true if empty, false if it contains points. + bool IsEmpty() const; + + //! Get the size of the surface point list. + //! @return - The number of valid points in the list. + size_t GetSize() const; + + //! Enumerate every surface point and call a callback for each point found. + void EnumeratePoints(AZStd::function< + bool(const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceTagWeights& surfaceWeights)> pointCallback) const; + + //! Modify the surface weights for each surface point in the list. + void ModifySurfaceWeights( + const AZ::EntityId& currentEntityId, + AZStd::function modificationWeightCallback); + + //! Get the surface point with the highest Z value. + AzFramework::SurfaceData::SurfacePoint GetHighestSurfacePoint() const; + + //! Remove any points that don't contain any of the provided surface tags. + void FilterPoints(const SurfaceTagVector& desiredTags); + + protected: + // These are kept in separate parallel vectors instead of a single struct so that it's possible to pass just specific data + // "channels" into other methods as span<> without having to pass the full struct into the span<>. Specifically, we want to be + // able to pass spans of the positions down through nesting gradient/surface calls. + // A side benefit is that profiling showed the data access to be faster than packing all the fields into a single struct. + AZStd::vector m_surfaceCreatorIdList; + AZStd::vector m_surfacePositionList; + AZStd::vector m_surfaceNormalList; + AZStd::vector m_surfaceWeightsList; + + AZ::Aabb m_pointBounds = AZ::Aabb::CreateNull(); }; - using SurfacePointList = AZStd::vector; using SurfacePointLists = AZStd::vector; struct SurfaceDataRegistryEntry diff --git a/Gems/SurfaceData/Code/Include/SurfaceData/Utility/SurfaceDataUtility.h b/Gems/SurfaceData/Code/Include/SurfaceData/Utility/SurfaceDataUtility.h index 2a4a39ee3b..be053db5b9 100644 --- a/Gems/SurfaceData/Code/Include/SurfaceData/Utility/SurfaceDataUtility.h +++ b/Gems/SurfaceData/Code/Include/SurfaceData/Utility/SurfaceDataUtility.h @@ -88,48 +88,6 @@ namespace SurfaceData const AZ::Vector3& rayStart, const AZ::Vector3& rayEnd, AZ::Vector3& outPosition, AZ::Vector3& outNormal); - AZ_INLINE void AssignSurfaceTagWeights(const SurfaceTagVector& tags, float weight, SurfaceTagWeightMap& weights) - { - weights.clear(); - weights.reserve(tags.size()); - for (auto& tag : tags) - { - weights[tag] = weight; - } - } - - AZ_INLINE void AddMaxValueForMasks(SurfaceTagWeightMap& masks, const AZ::Crc32 tag, const float value) - { - const auto maskItr = masks.find(tag); - const float valueOld = maskItr != masks.end() ? maskItr->second : 0.0f; - masks[tag] = AZ::GetMax(value, valueOld); - } - - AZ_INLINE void AddMaxValueForMasks(SurfaceTagWeightMap& masks, const SurfaceTagVector& tags, const float value) - { - for (const auto& tag : tags) - { - AddMaxValueForMasks(masks, tag, value); - } - } - - AZ_INLINE void AddMaxValueForMasks(SurfaceTagWeightMap& outMasks, const SurfaceTagWeightMap& inMasks) - { - for (const auto& inMask : inMasks) - { - AddMaxValueForMasks(outMasks, inMask.first, inMask.second); - } - } - - template - AZ_INLINE void AddItemIfNotFound(Container& container, const Element& element) - { - if (AZStd::find(container.begin(), container.end(), element) == container.end()) - { - container.insert(container.end(), element); - } - } - template AZ_INLINE bool HasMatchingTag(const SourceContainer& sourceTags, const AZ::Crc32& sampleTag) { @@ -137,26 +95,7 @@ namespace SurfaceData } template - AZ_INLINE bool HasMatchingTags(const SourceContainer& sourceTags, const SampleContainer& sampleTags) - { - for (const auto& sampleTag : sampleTags) - { - if (HasMatchingTag(sourceTags, sampleTag)) - { - return true; - } - } - - return false; - } - - AZ_INLINE bool HasMatchingTag(const SurfaceTagWeightMap& sourceTags, const AZ::Crc32& sampleTag) - { - return sourceTags.find(sampleTag) != sourceTags.end(); - } - - template - AZ_INLINE bool HasMatchingTags(const SurfaceTagWeightMap& sourceTags, const SampleContainer& sampleTags) + AZ_INLINE bool HasAnyMatchingTags(const SourceContainer& sourceTags, const SampleContainer& sampleTags) { for (const auto& sampleTag : sampleTags) { @@ -168,36 +107,8 @@ namespace SurfaceData return false; } - - AZ_INLINE bool HasMatchingTag(const SurfaceTagWeightMap& sourceTags, const AZ::Crc32& sampleTag, float valueMin, float valueMax) - { - auto maskItr = sourceTags.find(sampleTag); - return maskItr != sourceTags.end() && valueMin <= maskItr->second && valueMax >= maskItr->second; - } - - template - AZ_INLINE bool HasMatchingTags( - const SurfaceTagWeightMap& sourceTags, const SampleContainer& sampleTags, float valueMin, float valueMax) - { - for (const auto& sampleTag : sampleTags) - { - if (HasMatchingTag(sourceTags, sampleTag, valueMin, valueMax)) - { - return true; - } - } - - return false; - } - - template - AZ_INLINE void RemoveUnassignedTags(const SourceContainer& sourceTags) - { - sourceTags.erase(AZStd::remove(sourceTags.begin(), sourceTags.end(), Constants::s_unassignedTagCrc), sourceTags.end()); - } - - template - AZ_INLINE bool HasValidTags(const SourceContainer& sourceTags) + + AZ_INLINE bool HasValidTags(const SurfaceTagVector& sourceTags) { for (const auto& sourceTag : sourceTags) { @@ -209,18 +120,6 @@ namespace SurfaceData return false; } - AZ_INLINE bool HasValidTags(const SurfaceTagWeightMap& sourceTags) - { - for (const auto& sourceTag : sourceTags) - { - if (sourceTag.first != Constants::s_unassignedTagCrc) - { - return true; - } - } - return false; - } - // Utility method to compare two AABBs for overlapping XY coordinates while ignoring the Z coordinates. AZ_INLINE bool AabbOverlaps2D(const AZ::Aabb& box1, const AZ::Aabb& box2) { diff --git a/Gems/SurfaceData/Code/Source/Components/SurfaceDataColliderComponent.cpp b/Gems/SurfaceData/Code/Source/Components/SurfaceDataColliderComponent.cpp index dc4e5e6e74..e51fbc09b2 100644 --- a/Gems/SurfaceData/Code/Source/Components/SurfaceDataColliderComponent.cpp +++ b/Gems/SurfaceData/Code/Source/Components/SurfaceDataColliderComponent.cpp @@ -132,7 +132,7 @@ namespace SurfaceData Physics::ColliderComponentEventBus::Handler::BusConnect(GetEntityId()); // Update the cached collider data and bounds, then register the surface data provider / modifier - AssignSurfaceTagWeights(m_configuration.m_providerTags, 1.0f, m_newPointWeights); + m_newPointWeights.AssignSurfaceTagWeights(m_configuration.m_providerTags, 1.0f); UpdateColliderData(); } @@ -237,12 +237,7 @@ namespace SurfaceData if (DoRayTrace(inPosition, queryPointOnly, hitPosition, hitNormal)) { - SurfacePoint point; - point.m_entityId = GetEntityId(); - point.m_position = hitPosition; - point.m_normal = hitNormal; - point.m_masks = m_newPointWeights; - surfacePointList.push_back(AZStd::move(point)); + surfacePointList.AddSurfacePoint(GetEntityId(), hitPosition, hitNormal, m_newPointWeights); } } @@ -252,20 +247,22 @@ namespace SurfaceData if (m_colliderBounds.IsValid() && !m_configuration.m_modifierTags.empty()) { - const AZ::EntityId entityId = GetEntityId(); - for (auto& point : surfacePointList) - { - if (point.m_entityId != entityId && m_colliderBounds.Contains(point.m_position)) + surfacePointList.ModifySurfaceWeights( + GetEntityId(), + [this](const AZ::Vector3& position, SurfaceData::SurfaceTagWeights& weights) { - AZ::Vector3 hitPosition; - AZ::Vector3 hitNormal; - constexpr bool queryPointOnly = true; - if (DoRayTrace(point.m_position, queryPointOnly, hitPosition, hitNormal)) + if (m_colliderBounds.Contains(position)) { - AddMaxValueForMasks(point.m_masks, m_configuration.m_modifierTags, 1.0f); + AZ::Vector3 hitPosition; + AZ::Vector3 hitNormal; + constexpr bool queryPointOnly = true; + if (DoRayTrace(position, queryPointOnly, hitPosition, hitNormal)) + { + // If the query point collides with the volume, add all our modifier tags with a weight of 1.0f. + weights.AddSurfaceWeightsIfGreater(m_configuration.m_modifierTags, 1.0f); + } } - } - } + }); } } diff --git a/Gems/SurfaceData/Code/Source/Components/SurfaceDataColliderComponent.h b/Gems/SurfaceData/Code/Source/Components/SurfaceDataColliderComponent.h index 50c84f9519..2ef25774aa 100644 --- a/Gems/SurfaceData/Code/Source/Components/SurfaceDataColliderComponent.h +++ b/Gems/SurfaceData/Code/Source/Components/SurfaceDataColliderComponent.h @@ -101,6 +101,6 @@ namespace SurfaceData AZStd::atomic_bool m_refresh{ false }; mutable AZStd::shared_mutex m_cacheMutex; AZ::Aabb m_colliderBounds = AZ::Aabb::CreateNull(); - SurfaceTagWeightMap m_newPointWeights; + SurfaceTagWeights m_newPointWeights; }; } diff --git a/Gems/SurfaceData/Code/Source/Components/SurfaceDataShapeComponent.cpp b/Gems/SurfaceData/Code/Source/Components/SurfaceDataShapeComponent.cpp index 682bac150a..b84607782e 100644 --- a/Gems/SurfaceData/Code/Source/Components/SurfaceDataShapeComponent.cpp +++ b/Gems/SurfaceData/Code/Source/Components/SurfaceDataShapeComponent.cpp @@ -89,7 +89,7 @@ namespace SurfaceData LmbrCentral::ShapeComponentNotificationsBus::Handler::BusConnect(GetEntityId()); // Update the cached shape data and bounds, then register the surface data provider / modifier - AssignSurfaceTagWeights(m_configuration.m_providerTags, 1.0f, m_newPointWeights); + m_newPointWeights.AssignSurfaceTagWeights(m_configuration.m_providerTags, 1.0f); UpdateShapeData(); } @@ -155,12 +155,8 @@ namespace SurfaceData LmbrCentral::ShapeComponentRequestsBus::EventResult(hitShape, GetEntityId(), &LmbrCentral::ShapeComponentRequestsBus::Events::IntersectRay, rayOrigin, rayDirection, intersectionDistance); if (hitShape) { - SurfacePoint point; - point.m_entityId = GetEntityId(); - point.m_position = rayOrigin + intersectionDistance * rayDirection; - point.m_normal = AZ::Vector3::CreateAxisZ(); - point.m_masks = m_newPointWeights; - surfacePointList.push_back(AZStd::move(point)); + AZ::Vector3 position = rayOrigin + intersectionDistance * rayDirection; + surfacePointList.AddSurfacePoint(GetEntityId(), position, AZ::Vector3::CreateAxisZ(), m_newPointWeights); } } } @@ -173,20 +169,19 @@ namespace SurfaceData { const AZ::EntityId entityId = GetEntityId(); LmbrCentral::ShapeComponentRequestsBus::Event( - GetEntityId(), + entityId, [entityId, this, &surfacePointList](LmbrCentral::ShapeComponentRequestsBus::Events* shape) { - for (auto& point : surfacePointList) + surfacePointList.ModifySurfaceWeights( + entityId, + [this, shape](const AZ::Vector3& position, SurfaceData::SurfaceTagWeights& weights) { - if (point.m_entityId != entityId && m_shapeBounds.Contains(point.m_position)) + if (m_shapeBounds.Contains(position) && shape->IsPointInside(position)) { - bool inside = shape->IsPointInside(point.m_position); - if (inside) - { - AddMaxValueForMasks(point.m_masks, m_configuration.m_modifierTags, 1.0f); - } + // If the point is inside our shape, add all our modifier tags with a weight of 1.0f. + weights.AddSurfaceWeightsIfGreater(m_configuration.m_modifierTags, 1.0f); } - } + }); }); } } diff --git a/Gems/SurfaceData/Code/Source/Components/SurfaceDataShapeComponent.h b/Gems/SurfaceData/Code/Source/Components/SurfaceDataShapeComponent.h index 59b7950412..25ac491809 100644 --- a/Gems/SurfaceData/Code/Source/Components/SurfaceDataShapeComponent.h +++ b/Gems/SurfaceData/Code/Source/Components/SurfaceDataShapeComponent.h @@ -97,6 +97,6 @@ namespace SurfaceData AZ::Aabb m_shapeBounds = AZ::Aabb::CreateNull(); bool m_shapeBoundsIsValid = false; static const float s_rayAABBHeightPadding; - SurfaceTagWeightMap m_newPointWeights; + SurfaceTagWeights m_newPointWeights; }; } diff --git a/Gems/SurfaceData/Code/Source/SurfaceDataSystemComponent.cpp b/Gems/SurfaceData/Code/Source/SurfaceDataSystemComponent.cpp index 66f282d12b..66b8c83a58 100644 --- a/Gems/SurfaceData/Code/Source/SurfaceDataSystemComponent.cpp +++ b/Gems/SurfaceData/Code/Source/SurfaceDataSystemComponent.cpp @@ -45,14 +45,10 @@ namespace SurfaceData if (auto behaviorContext = azrtti_cast(context)) { - behaviorContext->Class() + behaviorContext->Class() ->Constructor() ->Attribute(AZ::Script::Attributes::Category, "Vegetation") ->Attribute(AZ::Script::Attributes::Module, "surface_data") - ->Property("entityId", BehaviorValueProperty(&SurfacePoint::m_entityId)) - ->Property("position", BehaviorValueProperty(&SurfacePoint::m_position)) - ->Property("normal", BehaviorValueProperty(&SurfacePoint::m_normal)) - ->Property("masks", BehaviorValueProperty(&SurfacePoint::m_masks)) ; behaviorContext->Class() @@ -182,11 +178,12 @@ namespace SurfaceData void SurfaceDataSystemComponent::GetSurfacePoints(const AZ::Vector3& inPosition, const SurfaceTagVector& desiredTags, SurfacePointList& surfacePointList) const { const bool useTagFilters = HasValidTags(desiredTags); - const bool hasModifierTags = useTagFilters && HasMatchingTags(desiredTags, m_registeredModifierTags); + const bool hasModifierTags = useTagFilters && HasAnyMatchingTags(desiredTags, m_registeredModifierTags); AZStd::shared_lock registrationLock(m_registrationMutex); - surfacePointList.clear(); + surfacePointList.Clear(); + surfacePointList.ReserveSpace(m_registeredSurfaceDataProviders.size()); //gather all intersecting points for (const auto& entryPair : m_registeredSurfaceDataProviders) @@ -195,14 +192,14 @@ namespace SurfaceData const SurfaceDataRegistryEntry& entry = entryPair.second; if (!entry.m_bounds.IsValid() || AabbContains2D(entry.m_bounds, inPosition)) { - if (!useTagFilters || hasModifierTags || HasMatchingTags(desiredTags, entry.m_tags)) + if (!useTagFilters || hasModifierTags || HasAnyMatchingTags(desiredTags, entry.m_tags)) { SurfaceDataProviderRequestBus::Event(entryAddress, &SurfaceDataProviderRequestBus::Events::GetSurfacePoints, inPosition, surfacePointList); } } } - if (!surfacePointList.empty()) + if (!surfacePointList.IsEmpty()) { //modify or annotate reported points for (const auto& entryPair : m_registeredSurfaceDataModifiers) @@ -221,10 +218,8 @@ namespace SurfaceData // doesn't add a desired tag, and a surface modifier has the *potential* to add it, but then doesn't. if (useTagFilters) { - FilterPoints(surfacePointList, desiredTags); + surfacePointList.FilterPoints(desiredTags); } - - CombineAndSortNeighboringPoints(surfacePointList); } } @@ -260,8 +255,13 @@ namespace SurfaceData surfacePointLists.clear(); surfacePointLists.resize(totalQueryPositions); + for (auto& surfacePointList : surfacePointLists) + { + surfacePointList.ReserveSpace(m_registeredSurfaceDataProviders.size()); + } + const bool useTagFilters = HasValidTags(desiredTags); - const bool hasModifierTags = useTagFilters && HasMatchingTags(desiredTags, m_registeredModifierTags); + const bool hasModifierTags = useTagFilters && HasAnyMatchingTags(desiredTags, m_registeredModifierTags); // Loop through each data provider, and query all the points for each one. This allows us to check the tags and the overall // AABB bounds just once per provider, instead of once per point. It also allows for an eventual optimization in which we could @@ -270,7 +270,7 @@ namespace SurfaceData { bool hasInfiniteBounds = !provider.m_bounds.IsValid(); - if (!useTagFilters || hasModifierTags || HasMatchingTags(desiredTags, provider.m_tags)) + if (!useTagFilters || hasModifierTags || HasAnyMatchingTags(desiredTags, provider.m_tags)) { for (size_t index = 0; index < totalQueryPositions; index++) { @@ -299,7 +299,7 @@ namespace SurfaceData { const auto& inPosition = inPositions[index]; SurfacePointList& surfacePointList = surfacePointLists[index]; - if (!surfacePointList.empty()) + if (!surfacePointList.IsEmpty()) { if (hasInfiniteBounds || AabbContains2D(entry.m_bounds, inPosition)) { @@ -315,82 +315,14 @@ namespace SurfaceData // same XY coordinates and extremely similar Z values. This produces results that are sorted in decreasing Z order. // Also, this filters out any remaining points that don't match the desired tag list. This can happen when a surface provider // doesn't add a desired tag, and a surface modifier has the *potential* to add it, but then doesn't. - for (auto& surfacePointList : surfacePointLists) + if (useTagFilters) { - if (useTagFilters) + for (auto& surfacePointList : surfacePointLists) { - FilterPoints(surfacePointList, desiredTags); + surfacePointList.FilterPoints(desiredTags); } - CombineAndSortNeighboringPoints(surfacePointList); - } - - } - - void SurfaceDataSystemComponent::FilterPoints(SurfacePointList& sourcePointList, const SurfaceTagVector& desiredTags) const - { - // Before sorting and combining, filter out any points that don't match our search tags. - sourcePointList.erase( - AZStd::remove_if( - sourcePointList.begin(), sourcePointList.end(), - [desiredTags](SurfacePoint& point) -> bool - { - return !HasMatchingTags(point.m_masks, desiredTags); - }), - sourcePointList.end()); - } - - void SurfaceDataSystemComponent::CombineAndSortNeighboringPoints(SurfacePointList& sourcePointList) const - { - // If there's only 0 or 1 point, there is no sorting or combining that needs to happen, so just return. - if (sourcePointList.size() <= 1) - { - return; } - // Efficient point consolidation requires the points to be pre-sorted so we are only comparing/combining neighbors. - // Sort XY points together, with decreasing Z. - AZStd::sort(sourcePointList.begin(), sourcePointList.end(), [](const SurfacePoint& a, const SurfacePoint& b) - { - // Our goal is to have identical XY values sorted adjacent to each other with decreasing Z. - // We sort increasing Y, then increasing X, then decreasing Z, because we need to compare all 3 values for a - // stable sort. The choice of increasing Y first is because we'll often generate the points as ranges of X values within - // ranges of Y values, so this will produce the most usable and expected output sort. - if (a.m_position.GetY() != b.m_position.GetY()) - { - return a.m_position.GetY() < b.m_position.GetY(); - } - if (a.m_position.GetX() != b.m_position.GetX()) - { - return a.m_position.GetX() < b.m_position.GetX(); - } - if (a.m_position.GetZ() != b.m_position.GetZ()) - { - return a.m_position.GetZ() > b.m_position.GetZ(); - } - - // If we somehow ended up with two points with identical positions getting generated, use the entity ID as the tiebreaker - // to guarantee a stable sort. We should never have two identical positions generated from the same entity. - return a.m_entityId < b.m_entityId; - }); - - // iterate over subsequent source points for comparison and consolidation with the last added target/unique point - for (auto pointItr = sourcePointList.begin() + 1; pointItr < sourcePointList.end();) - { - auto prevPointItr = pointItr - 1; - - // (Someday we should add a configurable tolerance for comparison) - if (pointItr->m_position.IsClose(prevPointItr->m_position) && pointItr->m_normal.IsClose(prevPointItr->m_normal)) - { - // consolidate points with similar attributes by adding masks/weights to the previous point and deleting this point. - AddMaxValueForMasks(prevPointItr->m_masks, pointItr->m_masks); - - pointItr = sourcePointList.erase(pointItr); - } - else - { - pointItr++; - } - } } SurfaceDataRegistryHandle SurfaceDataSystemComponent::RegisterSurfaceDataProviderInternal(const SurfaceDataRegistryEntry& entry) diff --git a/Gems/SurfaceData/Code/Source/SurfaceDataSystemComponent.h b/Gems/SurfaceData/Code/Source/SurfaceDataSystemComponent.h index 6ec2cab4eb..bb554cec78 100644 --- a/Gems/SurfaceData/Code/Source/SurfaceDataSystemComponent.h +++ b/Gems/SurfaceData/Code/Source/SurfaceDataSystemComponent.h @@ -58,9 +58,6 @@ namespace SurfaceData void RefreshSurfaceData(const AZ::Aabb& dirtyArea) override; private: - void FilterPoints(SurfacePointList& sourcePointList, const SurfaceTagVector& desiredTags) const; - void CombineAndSortNeighboringPoints(SurfacePointList& sourcePointList) const; - SurfaceDataRegistryHandle RegisterSurfaceDataProviderInternal(const SurfaceDataRegistryEntry& entry); SurfaceDataRegistryEntry UnregisterSurfaceDataProviderInternal(const SurfaceDataRegistryHandle& handle); bool UpdateSurfaceDataProviderInternal(const SurfaceDataRegistryHandle& handle, const SurfaceDataRegistryEntry& entry, AZ::Aabb& oldBounds); diff --git a/Gems/SurfaceData/Code/Source/SurfaceDataTypes.cpp b/Gems/SurfaceData/Code/Source/SurfaceDataTypes.cpp new file mode 100644 index 0000000000..3071fd02ee --- /dev/null +++ b/Gems/SurfaceData/Code/Source/SurfaceDataTypes.cpp @@ -0,0 +1,319 @@ +/* + * 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 + +namespace SurfaceData +{ + void SurfaceTagWeights::AssignSurfaceTagWeights(const AzFramework::SurfaceData::SurfaceTagWeightList& weights) + { + m_weights.clear(); + m_weights.reserve(weights.size()); + for (auto& weight : weights) + { + m_weights.emplace(weight.m_surfaceType, weight.m_weight); + } + } + + void SurfaceTagWeights::AssignSurfaceTagWeights(const SurfaceTagVector& tags, float weight) + { + m_weights.clear(); + m_weights.reserve(tags.size()); + for (auto& tag : tags) + { + m_weights[tag] = weight; + } + } + + void SurfaceTagWeights::AddSurfaceTagWeight(const AZ::Crc32 tag, const float value) + { + m_weights[tag] = value; + } + + void SurfaceTagWeights::Clear() + { + m_weights.clear(); + } + + size_t SurfaceTagWeights::GetSize() const + { + return m_weights.size(); + } + + AzFramework::SurfaceData::SurfaceTagWeightList SurfaceTagWeights::GetSurfaceTagWeightList() const + { + AzFramework::SurfaceData::SurfaceTagWeightList weights; + weights.reserve(m_weights.size()); + for (auto& weight : m_weights) + { + weights.emplace_back(weight.first, weight.second); + } + return weights; + } + + bool SurfaceTagWeights::operator==(const SurfaceTagWeights& rhs) const + { + // If the lists are different sizes, they're not equal. + if (m_weights.size() != rhs.m_weights.size()) + { + return false; + } + + for (auto& weight : m_weights) + { + auto rhsWeight = rhs.m_weights.find(weight.first); + if ((rhsWeight == rhs.m_weights.end()) || (rhsWeight->second != weight.second)) + { + return false; + } + } + + // All the entries matched, and the lists are the same size, so they're equal. + return true; + } + + bool SurfaceTagWeights::SurfaceWeightsAreEqual(const AzFramework::SurfaceData::SurfaceTagWeightList& compareWeights) const + { + // If the lists are different sizes, they're not equal. + if (m_weights.size() != compareWeights.size()) + { + return false; + } + + for (auto& weight : m_weights) + { + auto maskEntry = AZStd::find_if( + compareWeights.begin(), compareWeights.end(), + [weight](const AzFramework::SurfaceData::SurfaceTagWeight& compareWeight) -> bool + { + return (weight.first == compareWeight.m_surfaceType) && (weight.second == compareWeight.m_weight); + }); + + // If we didn't find a match, they're not equal. + if (maskEntry == compareWeights.end()) + { + return false; + } + } + + // All the entries matched, and the lists are the same size, so they're equal. + return true; + } + + void SurfaceTagWeights::EnumerateWeights(AZStd::function weightCallback) const + { + for (auto& [tag, weight] : m_weights) + { + if (!weightCallback(tag, weight)) + { + break; + } + } + } + + bool SurfaceTagWeights::HasValidTags() const + { + for (const auto& sourceTag : m_weights) + { + if (sourceTag.first != Constants::s_unassignedTagCrc) + { + return true; + } + } + return false; + } + + bool SurfaceTagWeights::HasMatchingTag(const AZ::Crc32& sampleTag) const + { + return m_weights.find(sampleTag) != m_weights.end(); + } + + bool SurfaceTagWeights::HasAnyMatchingTags(const SurfaceTagVector& sampleTags) const + { + for (const auto& sampleTag : sampleTags) + { + if (HasMatchingTag(sampleTag)) + { + return true; + } + } + + return false; + } + + bool SurfaceTagWeights::HasMatchingTag(const AZ::Crc32& sampleTag, float weightMin, float weightMax) const + { + auto maskItr = m_weights.find(sampleTag); + return maskItr != m_weights.end() && weightMin <= maskItr->second && weightMax >= maskItr->second; + } + + bool SurfaceTagWeights::HasAnyMatchingTags(const SurfaceTagVector& sampleTags, float weightMin, float weightMax) const + { + for (const auto& sampleTag : sampleTags) + { + if (HasMatchingTag(sampleTag, weightMin, weightMax)) + { + return true; + } + } + + return false; + } + + + + + SurfacePointList::SurfacePointList(AZStd::initializer_list surfacePoints) + { + ReserveSpace(surfacePoints.size()); + + for (auto& point : surfacePoints) + { + SurfaceTagWeights weights(point.m_surfaceTags); + AddSurfacePoint(AZ::EntityId(), point.m_position, point.m_normal, weights); + } + } + + void SurfacePointList::AddSurfacePoint(const AZ::EntityId& entityId, + const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceTagWeights& masks) + { + // When adding a surface point, we'll either merge it with a similar existing point, or else add it in order of + // decreasing Z, so that our final results are sorted. + + for (size_t index = 0; index < m_surfacePositionList.size(); ++index) + { + // (Someday we should add a configurable tolerance for comparison) + if (m_surfacePositionList[index].IsClose(position) && m_surfaceNormalList[index].IsClose(normal)) + { + // consolidate points with similar attributes by adding masks/weights to the similar point instead of adding a new one. + m_surfaceWeightsList[index].AddSurfaceWeightsIfGreater(masks); + return; + } + else if (m_surfacePositionList[index].GetZ() < position.GetZ()) + { + m_pointBounds.AddPoint(position); + m_surfacePositionList.insert(m_surfacePositionList.begin() + index, position); + m_surfaceNormalList.insert(m_surfaceNormalList.begin() + index, normal); + m_surfaceWeightsList.insert(m_surfaceWeightsList.begin() + index, masks); + m_surfaceCreatorIdList.insert(m_surfaceCreatorIdList.begin() + index, entityId); + return; + } + } + + // The point wasn't merged and the sort puts it at the end, so just add the point to the end of the list. + m_pointBounds.AddPoint(position); + m_surfacePositionList.emplace_back(position); + m_surfaceNormalList.emplace_back(normal); + m_surfaceWeightsList.emplace_back(masks); + m_surfaceCreatorIdList.emplace_back(entityId); + } + + void SurfacePointList::Clear() + { + m_surfacePositionList.clear(); + m_surfaceNormalList.clear(); + m_surfaceWeightsList.clear(); + m_surfaceCreatorIdList.clear(); + } + + void SurfacePointList::ReserveSpace(size_t maxPointsPerInput) + { + AZ_Assert( + m_surfacePositionList.size() < maxPointsPerInput, + "Trying to reserve space on a list that is already using more points than requested."); + + m_surfaceCreatorIdList.reserve(maxPointsPerInput); + m_surfacePositionList.reserve(maxPointsPerInput); + m_surfaceNormalList.reserve(maxPointsPerInput); + m_surfaceWeightsList.reserve(maxPointsPerInput); + } + + bool SurfacePointList::IsEmpty() const + { + return m_surfacePositionList.empty(); + } + + size_t SurfacePointList::GetSize() const + { + return m_surfacePositionList.size(); + } + + void SurfacePointList::EnumeratePoints( + AZStd::function + pointCallback) const + { + for (size_t index = 0; index < m_surfacePositionList.size(); index++) + { + if (!pointCallback(m_surfacePositionList[index], m_surfaceNormalList[index], m_surfaceWeightsList[index])) + { + break; + } + } + } + + void SurfacePointList::ModifySurfaceWeights( + const AZ::EntityId& currentEntityId, + AZStd::function modificationWeightCallback) + { + for (size_t index = 0; index < m_surfacePositionList.size(); index++) + { + if (m_surfaceCreatorIdList[index] != currentEntityId) + { + modificationWeightCallback(m_surfacePositionList[index], m_surfaceWeightsList[index]); + } + } + } + + AzFramework::SurfaceData::SurfacePoint SurfacePointList::GetHighestSurfacePoint() const + { + AzFramework::SurfaceData::SurfacePoint point; + point.m_position = m_surfacePositionList.front(); + point.m_normal = m_surfaceNormalList.front(); + point.m_surfaceTags = m_surfaceWeightsList.front().GetSurfaceTagWeightList(); + + return point; + } + + void SurfacePointList::FilterPoints(const SurfaceTagVector& desiredTags) + { + // Filter out any points that don't match our search tags. + // This has to be done after the Surface Modifiers have processed the points, not at point insertion time, because + // Surface Modifiers add tags to existing points. + size_t listSize = m_surfacePositionList.size(); + size_t index = 0; + for (; index < listSize; index++) + { + if (!m_surfaceWeightsList[index].HasAnyMatchingTags(desiredTags)) + { + break; + } + } + + if (index != listSize) + { + size_t next = index + 1; + for (; next < listSize; ++next) + { + if (m_surfaceWeightsList[index].HasAnyMatchingTags(desiredTags)) + { + m_surfaceCreatorIdList[index] = m_surfaceCreatorIdList[next]; + m_surfacePositionList[index] = m_surfacePositionList[next]; + m_surfaceNormalList[index] = m_surfaceNormalList[next]; + m_surfaceWeightsList[index] = m_surfaceWeightsList[next]; + ++index; + } + } + + m_surfaceCreatorIdList.resize(index); + m_surfacePositionList.resize(index); + m_surfaceNormalList.resize(index); + m_surfaceWeightsList.resize(index); + } + } +} diff --git a/Gems/SurfaceData/Code/Tests/SurfaceDataBenchmarks.cpp b/Gems/SurfaceData/Code/Tests/SurfaceDataBenchmarks.cpp index 65487c155b..226a711cff 100644 --- a/Gems/SurfaceData/Code/Tests/SurfaceDataBenchmarks.cpp +++ b/Gems/SurfaceData/Code/Tests/SurfaceDataBenchmarks.cpp @@ -190,7 +190,7 @@ namespace UnitTest for (float x = 0.0f; x < worldSize; x += 1.0f) { AZ::Vector3 queryPosition(x, y, 0.0f); - points.clear(); + points.Clear(); SurfaceData::SurfaceDataSystemRequestBus::Broadcast( &SurfaceData::SurfaceDataSystemRequestBus::Events::GetSurfacePoints, queryPosition, filterTags, points); diff --git a/Gems/SurfaceData/Code/Tests/SurfaceDataColliderComponentTest.cpp b/Gems/SurfaceData/Code/Tests/SurfaceDataColliderComponentTest.cpp index c51ea49f14..884c831393 100644 --- a/Gems/SurfaceData/Code/Tests/SurfaceDataColliderComponentTest.cpp +++ b/Gems/SurfaceData/Code/Tests/SurfaceDataColliderComponentTest.cpp @@ -26,7 +26,8 @@ namespace UnitTest : public AzPhysics::SimulatedBodyComponentRequestsBus::Handler { public: - MockPhysicsWorldBusProvider(const AZ::EntityId& id, AZ::Vector3 inPosition, bool setHitResult, const SurfaceData::SurfacePoint& hitResult) + MockPhysicsWorldBusProvider( + const AZ::EntityId& id, AZ::Vector3 inPosition, bool setHitResult, const AzFramework::SurfaceData::SurfacePoint& hitResult) { AzPhysics::SimulatedBodyComponentRequestsBus::Handler::BusConnect(id); @@ -77,51 +78,36 @@ namespace UnitTest { protected: // Create a new SurfacePoint with the given fields. - SurfaceData::SurfacePoint CreateSurfacePoint(AZ::EntityId id, AZ::Vector3 position, AZ::Vector3 normal, AZStd::vector> tags) + AzFramework::SurfaceData::SurfacePoint CreateSurfacePoint( + AZ::Vector3 position, AZ::Vector3 normal, AZStd::vector> tags) { - SurfaceData::SurfacePoint point; - point.m_entityId = id; + AzFramework::SurfaceData::SurfacePoint point; point.m_position = position; point.m_normal = normal; for (auto& tag : tags) { - point.m_masks[SurfaceData::SurfaceTag(tag.first)] = tag.second; + point.m_surfaceTags.emplace_back(SurfaceData::SurfaceTag(tag.first), tag.second); } return point; } // Compare two surface points. - bool SurfacePointsAreEqual(const SurfaceData::SurfacePoint& lhs, const SurfaceData::SurfacePoint& rhs) + bool SurfacePointsAreEqual( + const AZ::Vector3& lhsPosition, + const AZ::Vector3& lhsNormal, + const SurfaceData::SurfaceTagWeights& lhsMasks, + const AzFramework::SurfaceData::SurfacePoint& rhs) { - if ((lhs.m_entityId != rhs.m_entityId) - || (lhs.m_position != rhs.m_position) - || (lhs.m_normal != rhs.m_normal) - || (lhs.m_masks.size() != rhs.m_masks.size())) - { - return false; - } - - for (auto& mask : lhs.m_masks) - { - auto maskEntry = rhs.m_masks.find(mask.first); - if (maskEntry == rhs.m_masks.end()) - { - return false; - } - if (maskEntry->second != mask.second) - { - return false; - } - } - - return true; + return ((lhsPosition == rhs.m_position) + && (lhsNormal == rhs.m_normal) + && (lhsMasks.SurfaceWeightsAreEqual(rhs.m_surfaceTags))); } // Common test function for testing the "Provider" functionality of the component. // Given a set of tags and an expected output, check to see if the component provides the // expected output point. void TestSurfaceDataColliderProvider(AZStd::vector providerTags, bool pointOnProvider, - AZ::Vector3 queryPoint, const SurfaceData::SurfacePoint& expectedOutput) + AZ::Vector3 queryPoint, const AzFramework::SurfaceData::SurfacePoint& expectedOutput) { // This lets our component register with surfaceData successfully. MockSurfaceDataSystem mockSurfaceDataSystem; @@ -135,8 +121,6 @@ namespace UnitTest // Create the test entity with the SurfaceDataCollider component and the required physics collider dependency auto entity = CreateEntity(); - // Initialize our Entity ID to the one passed in on the expectedOutput - entity->SetId(expectedOutput.m_entityId); // Create the components CreateComponent(entity.get()); CreateComponent(entity.get(), config); @@ -155,17 +139,25 @@ namespace UnitTest queryPoint, pointList); if (pointOnProvider) { - ASSERT_TRUE(pointList.size() == 1); - EXPECT_TRUE(SurfacePointsAreEqual(pointList[0], expectedOutput)); + ASSERT_EQ(pointList.GetSize(), 1); + pointList.EnumeratePoints( + [this, expectedOutput]( + const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool + { + EXPECT_TRUE(SurfacePointsAreEqual(position, normal, masks, expectedOutput)); + return true; + }); } else { - EXPECT_TRUE(pointList.empty()); + EXPECT_TRUE(pointList.IsEmpty()); } } void TestSurfaceDataColliderModifier(AZStd::vector modifierTags, - const SurfaceData::SurfacePoint& input, bool pointInCollider, const SurfaceData::SurfacePoint& expectedOutput) + const AzFramework::SurfaceData::SurfacePoint& input, + bool pointInCollider, + const AzFramework::SurfaceData::SurfacePoint& expectedOutput) { // This lets our component register with surfaceData successfully. MockSurfaceDataSystem mockSurfaceDataSystem; @@ -191,11 +183,18 @@ namespace UnitTest EXPECT_TRUE(modifierHandle != SurfaceData::InvalidSurfaceDataRegistryHandle); // Call ModifySurfacePoints and verify the results - SurfaceData::SurfacePointList pointList; - pointList.emplace_back(input); + // Add the surface point with a different entity ID than the entity doing the modification, so that the point doesn't get + // filtered out. + SurfaceData::SurfacePointList pointList = { input }; SurfaceData::SurfaceDataModifierRequestBus::Event(modifierHandle, &SurfaceData::SurfaceDataModifierRequestBus::Events::ModifySurfacePoints, pointList); - ASSERT_TRUE(pointList.size() == 1); - EXPECT_TRUE(SurfacePointsAreEqual(pointList[0], expectedOutput)); + ASSERT_EQ(pointList.GetSize(), 1); + pointList.EnumeratePoints( + [this, expectedOutput]( + const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool + { + EXPECT_TRUE(SurfacePointsAreEqual(position, normal, masks, expectedOutput)); + return true; + }); } }; @@ -232,7 +231,8 @@ namespace UnitTest // Set the expected output to an arbitrary entity ID, position, and normal. // We'll use this to initialize the mock physics, so the output of the query should match. const char* tag = "test_mask"; - SurfaceData::SurfacePoint expectedOutput = CreateSurfacePoint(AZ::EntityId(0x12345678), AZ::Vector3(1.0f), AZ::Vector3::CreateAxisZ(), + AzFramework::SurfaceData::SurfacePoint expectedOutput = + CreateSurfacePoint(AZ::Vector3(1.0f), AZ::Vector3::CreateAxisZ(), { AZStd::make_pair(tag, 1.0f) }); // Query from the same XY, but one unit higher on Z, just so we can verify that the output returns the collision @@ -248,7 +248,8 @@ namespace UnitTest // Set the expected output to an arbitrary entity ID, position, and normal. // We'll use this to initialize the mock physics. const char* tag = "test_mask"; - SurfaceData::SurfacePoint expectedOutput = CreateSurfacePoint(AZ::EntityId(0x12345678), AZ::Vector3(1.0f), AZ::Vector3::CreateAxisZ(), + AzFramework::SurfaceData::SurfacePoint expectedOutput = + CreateSurfacePoint(AZ::Vector3(1.0f), AZ::Vector3::CreateAxisZ(), { AZStd::make_pair(tag, 1.0f) }); // Query from the same XY, but one unit higher on Z. However, we're also telling our test to provide @@ -266,9 +267,9 @@ namespace UnitTest // We'll use this to initialize the mock physics. const char* tag1 = "test_mask1"; const char* tag2 = "test_mask2"; - SurfaceData::SurfacePoint expectedOutput = CreateSurfacePoint(AZ::EntityId(0x12345678), AZ::Vector3(1.0f), AZ::Vector3::CreateAxisZ(), - { AZStd::make_pair(tag1, 1.0f), - AZStd::make_pair(tag2, 1.0f) }); + AzFramework::SurfaceData::SurfacePoint expectedOutput = CreateSurfacePoint( + AZ::Vector3(1.0f), AZ::Vector3::CreateAxisZ(), + { AZStd::make_pair(tag1, 1.0f), AZStd::make_pair(tag2, 1.0f) }); // Query from the same XY, but one unit higher on Z, just so we can verify that the output returns the collision // result, not the input point. @@ -281,11 +282,12 @@ namespace UnitTest // Verify that for a point inside the collider, the output point contains the correct tag and value. // Set arbitrary input data - SurfaceData::SurfacePoint input = CreateSurfacePoint(AZ::EntityId(0x12345678), AZ::Vector3(1.0f), AZ::Vector3(0.0f), {}); + AzFramework::SurfaceData::SurfacePoint input = CreateSurfacePoint(AZ::Vector3(1.0f), AZ::Vector3(0.0f), {}); // Output should match the input, but with an added tag / value const char* tag = "test_mask"; - SurfaceData::SurfacePoint expectedOutput = CreateSurfacePoint(input.m_entityId, input.m_position, input.m_normal, - { AZStd::make_pair(tag, 1.0f) }); + AzFramework::SurfaceData::SurfacePoint expectedOutput = + CreateSurfacePoint(input.m_position, input.m_normal, + { AZStd::make_pair(tag, 1.0f) }); constexpr bool pointInCollider = true; TestSurfaceDataColliderModifier({ tag }, input, pointInCollider, expectedOutput); @@ -296,10 +298,10 @@ namespace UnitTest // Verify that for a point outside the collider, the output point contains no tags / values. // Set arbitrary input data - SurfaceData::SurfacePoint input = CreateSurfacePoint(AZ::EntityId(0x12345678), AZ::Vector3(1.0f), AZ::Vector3(0.0f), {}); + AzFramework::SurfaceData::SurfacePoint input = CreateSurfacePoint(AZ::Vector3(1.0f), AZ::Vector3(0.0f), {}); // Output should match the input - no extra tags / values should be added. const char* tag = "test_mask"; - SurfaceData::SurfacePoint expectedOutput = CreateSurfacePoint(input.m_entityId, input.m_position, input.m_normal, {}); + AzFramework::SurfaceData::SurfacePoint expectedOutput = CreateSurfacePoint(input.m_position, input.m_normal, {}); constexpr bool pointInCollider = true; TestSurfaceDataColliderModifier({ tag }, input, !pointInCollider, expectedOutput); @@ -310,11 +312,12 @@ namespace UnitTest // Verify that if the component has multiple tags, all of them get put on the output with the same value. // Set arbitrary input data - SurfaceData::SurfacePoint input = CreateSurfacePoint(AZ::EntityId(0x12345678), AZ::Vector3(1.0f), AZ::Vector3(0.0f), {}); + AzFramework::SurfaceData::SurfacePoint input = CreateSurfacePoint(AZ::Vector3(1.0f), AZ::Vector3(0.0f), {}); // Output should match the input, but with two added tags const char* tag1 = "test_mask1"; const char* tag2 = "test_mask2"; - SurfaceData::SurfacePoint expectedOutput = CreateSurfacePoint(input.m_entityId, input.m_position, input.m_normal, + AzFramework::SurfaceData::SurfacePoint expectedOutput = CreateSurfacePoint( + input.m_position, input.m_normal, { AZStd::make_pair(tag1, 1.0f), AZStd::make_pair(tag2, 1.0f) }); constexpr bool pointInCollider = true; @@ -328,11 +331,13 @@ namespace UnitTest // Set arbitrary input data const char* preservedTag = "preserved_tag"; - SurfaceData::SurfacePoint input = CreateSurfacePoint(AZ::EntityId(0x12345678), AZ::Vector3(1.0f), AZ::Vector3(0.0f), + AzFramework::SurfaceData::SurfacePoint input = + CreateSurfacePoint(AZ::Vector3(1.0f), AZ::Vector3(0.0f), { AZStd::make_pair(preservedTag, 1.0f) }); // Output should match the input, but with two added tags const char* modifierTag = "modifier_tag"; - SurfaceData::SurfacePoint expectedOutput = CreateSurfacePoint(input.m_entityId, input.m_position, input.m_normal, + AzFramework::SurfaceData::SurfacePoint expectedOutput = CreateSurfacePoint( + input.m_position, input.m_normal, { AZStd::make_pair(preservedTag, 1.0f), AZStd::make_pair(modifierTag, 1.0f) }); constexpr bool pointInCollider = true; @@ -349,10 +354,12 @@ namespace UnitTest float inputValue = 0.25f; // Set arbitrary input data - SurfaceData::SurfacePoint input = CreateSurfacePoint(AZ::EntityId(0x12345678), AZ::Vector3(1.0f), AZ::Vector3(0.0f), + AzFramework::SurfaceData::SurfacePoint input = + CreateSurfacePoint(AZ::Vector3(1.0f), AZ::Vector3(0.0f), { AZStd::make_pair(tag, inputValue) }); // Output should match the input, except that the value on the tag gets the higher modifier value - SurfaceData::SurfacePoint expectedOutput = CreateSurfacePoint(input.m_entityId, input.m_position, input.m_normal, + AzFramework::SurfaceData::SurfacePoint expectedOutput = + CreateSurfacePoint(input.m_position, input.m_normal, { AZStd::make_pair(tag, 1.0f) }); constexpr bool pointInCollider = true; diff --git a/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp b/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp index 2d6d64f65d..39659313f5 100644 --- a/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp +++ b/Gems/SurfaceData/Code/Tests/SurfaceDataTest.cpp @@ -54,7 +54,7 @@ class MockSurfaceProvider } private: - AZStd::unordered_map, SurfaceData::SurfacePointList> m_GetSurfacePoints; + AZStd::unordered_map, AZStd::vector> m_GetSurfacePoints; SurfaceData::SurfaceTagVector m_tags; ProviderType m_providerType; AZ::EntityId m_id; @@ -71,15 +71,16 @@ class MockSurfaceProvider { for (float x = start.GetX(); x < end.GetX(); x += stepSize.GetX()) { - SurfaceData::SurfacePointList points; + AZStd::vector points; for (float z = start.GetZ(); z < end.GetZ(); z += stepSize.GetZ()) { - SurfaceData::SurfacePoint point; - point.m_entityId = m_id; + AzFramework::SurfaceData::SurfacePoint point; point.m_position = AZ::Vector3(x, y, z); point.m_normal = AZ::Vector3::CreateAxisZ(); - AddMaxValueForMasks(point.m_masks, m_tags, 1.0f); - + for (auto& tag : m_tags) + { + point.m_surfaceTags.emplace_back(tag, 1.0f); + } points.push_back(point); } m_GetSurfacePoints[AZStd::pair(x, y)] = points; @@ -150,7 +151,8 @@ class MockSurfaceProvider { for (auto& point : surfacePoints->second) { - surfacePointList.push_back(point); + SurfaceData::SurfaceTagWeights weights(point.m_surfaceTags); + surfacePointList.AddSurfacePoint(m_id, point.m_position, point.m_normal, weights); } } } @@ -159,16 +161,17 @@ class MockSurfaceProvider // SurfaceDataModifierRequestBus void ModifySurfacePoints(SurfaceData::SurfacePointList& surfacePointList) const override { - for (auto& point : surfacePointList) - { - auto surfacePoints = m_GetSurfacePoints.find(AZStd::make_pair(point.m_position.GetX(), point.m_position.GetY())); - - if (surfacePoints != m_GetSurfacePoints.end()) + surfacePointList.ModifySurfaceWeights( + AZ::EntityId(), + [this](const AZ::Vector3& position, SurfaceData::SurfaceTagWeights& weights) { - AddMaxValueForMasks(point.m_masks, m_tags, 1.0f); - } - } + auto surfacePoints = m_GetSurfacePoints.find(AZStd::make_pair(position.GetX(), position.GetY())); + if (surfacePoints != m_GetSurfacePoints.end()) + { + weights.AddSurfaceWeightsIfGreater(m_tags, 1.0f); + } + }); } SurfaceData::SurfaceDataRegistryHandle m_providerHandle = SurfaceData::InvalidSurfaceDataRegistryHandle; @@ -205,42 +208,49 @@ public: } void CompareSurfacePointListWithGetSurfacePoints( - const AZStd::vector& queryPositions, SurfaceData::SurfacePointLists surfacePointLists, + const AZStd::vector& queryPositions, SurfaceData::SurfacePointLists& surfacePointLists, const SurfaceData::SurfaceTagVector& testTags) { - SurfaceData::SurfacePointLists singleQueryPointLists; + AZStd::vector singleQueryResults; for (auto& queryPosition : queryPositions) { SurfaceData::SurfacePointList tempSingleQueryPointList; SurfaceData::SurfaceDataSystemRequestBus::Broadcast( &SurfaceData::SurfaceDataSystemRequestBus::Events::GetSurfacePoints, queryPosition, testTags, tempSingleQueryPointList); - singleQueryPointLists.push_back(tempSingleQueryPointList); + tempSingleQueryPointList.EnumeratePoints( + [&singleQueryResults]( + const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool + { + AzFramework::SurfaceData::SurfacePoint point; + point.m_position = position; + point.m_normal = normal; + point.m_surfaceTags = masks.GetSurfaceTagWeightList(); + singleQueryResults.emplace_back(AZStd::move(point)); + return true; + }); } - // Verify the two point lists are the same size, then verify that each point in each list is equal. - ASSERT_EQ(singleQueryPointLists.size(), surfacePointLists.size()); + // Verify that each point in each list is equal. + AzFramework::SurfaceData::SurfacePoint* singleQueryPoint = singleQueryResults.begin(); for (size_t listIndex = 0; listIndex < surfacePointLists.size(); listIndex++) { auto& surfacePointList = surfacePointLists[listIndex]; - auto& singleQueryPointList = singleQueryPointLists[listIndex]; - - ASSERT_EQ(singleQueryPointList.size(), surfacePointList.size()); - for (size_t index = 0; index < surfacePointList.size(); index++) - { - SurfaceData::SurfacePoint& point1 = surfacePointList[index]; - SurfaceData::SurfacePoint& point2 = singleQueryPointList[index]; - - EXPECT_EQ(point1.m_entityId, point2.m_entityId); - EXPECT_EQ(point1.m_position, point2.m_position); - EXPECT_EQ(point1.m_normal, point2.m_normal); - ASSERT_EQ(point1.m_masks.size(), point2.m_masks.size()); - for (auto& mask : point1.m_masks) + surfacePointList.EnumeratePoints( + [&singleQueryPoint, singleQueryResults]( + const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool { - EXPECT_EQ(mask.second, point2.m_masks[mask.first]); - } - } + EXPECT_NE(singleQueryPoint, singleQueryResults.end()); + + EXPECT_EQ(position, singleQueryPoint->m_position); + EXPECT_EQ(normal, singleQueryPoint->m_normal); + EXPECT_TRUE(masks.SurfaceWeightsAreEqual(singleQueryPoint->m_surfaceTags)); + ++singleQueryPoint; + return true; + }); } + + EXPECT_EQ(singleQueryPoint, singleQueryResults.end()); } @@ -488,13 +498,18 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion) // We *could* check every mask as well for completeness, but that seems like overkill. for (auto& pointList : availablePointsPerPosition) { - EXPECT_EQ(pointList.size(), 2); - EXPECT_EQ(pointList[0].m_position.GetZ(), 4.0f); - EXPECT_EQ(pointList[1].m_position.GetZ(), 0.0f); - for (auto& point : pointList) - { - EXPECT_EQ(point.m_masks.size(), providerTags.size()); - } + EXPECT_EQ(pointList.GetSize(), 2); + float expectedZ = 4.0f; + pointList.EnumeratePoints( + [providerTags, + &expectedZ](const AZ::Vector3& position, [[maybe_unused]] const AZ::Vector3& normal, + const SurfaceData::SurfaceTagWeights& masks) -> bool + { + EXPECT_EQ(position.GetZ(), expectedZ); + EXPECT_EQ(masks.GetSize(), providerTags.size()); + expectedZ = (expectedZ == 4.0f) ? 0.0f : 4.0f; + return true; + }); } } @@ -523,7 +538,7 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_NoMatchingMas // any of the masks from our mock surface provider. for (auto& queryPosition : availablePointsPerPosition) { - EXPECT_TRUE(queryPosition.empty()); + EXPECT_TRUE(queryPosition.IsEmpty()); } } @@ -551,7 +566,7 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_NoMatchingReg // our surface provider. for (auto& pointList : availablePointsPerPosition) { - EXPECT_TRUE(pointList.empty()); + EXPECT_TRUE(pointList.IsEmpty()); } } @@ -601,14 +616,17 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_ProviderModif // and each point should have both the "test_surface1" and "test_surface2" tag. for (auto& pointList : availablePointsPerPosition) { - EXPECT_EQ(pointList.size(), 2); + EXPECT_EQ(pointList.GetSize(), 2); float expectedZ = 4.0f; - for (auto& point : pointList) - { - EXPECT_EQ(point.m_position.GetZ(), expectedZ); - EXPECT_EQ(point.m_masks.size(), 2); - expectedZ = (expectedZ == 4.0f) ? 0.0f : 4.0f; - } + pointList.EnumeratePoints( + [&expectedZ](const AZ::Vector3& position, + [[maybe_unused]] const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool + { + EXPECT_EQ(position.GetZ(), expectedZ); + EXPECT_EQ(masks.GetSize(), 2); + expectedZ = (expectedZ == 4.0f) ? 0.0f : 4.0f; + return true; + }); } } } @@ -648,14 +666,20 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_SimilarPoints // should have both surface tags on them. for (auto& pointList : availablePointsPerPosition) { - EXPECT_EQ(pointList.size(), 2); - float expectedZ = 4.0005f; - for (auto& point : pointList) - { - EXPECT_EQ(point.m_position.GetZ(), expectedZ); - EXPECT_EQ(point.m_masks.size(), 2); - expectedZ = (expectedZ == 4.0005f) ? 0.0005f : 4.0005f; - } + EXPECT_EQ(pointList.GetSize(), 2); + float expectedZ = 4.0f; + pointList.EnumeratePoints( + [&expectedZ]( + const AZ::Vector3& position, [[maybe_unused]] const AZ::Vector3& normal, + const SurfaceData::SurfaceTagWeights& masks) -> bool + { + // Similar points get merged, but there's no guarantee which value will be kept, so we set our comparison tolerance + // high enough to allow both x.0 and x.0005 to pass. + EXPECT_NEAR(position.GetZ(), expectedZ, 0.001f); + EXPECT_EQ(masks.GetSize(), 2); + expectedZ = (expectedZ == 4.0f) ? 0.0f : 4.0f; + return true; + }); } } @@ -693,11 +717,14 @@ TEST_F(SurfaceDataTestApp, SurfaceData_TestSurfacePointsFromRegion_DissimilarPoi // because the points are far enough apart that they won't merge. for (auto& pointList : availablePointsPerPosition) { - EXPECT_EQ(pointList.size(), 4); - for (auto& point : pointList) - { - EXPECT_EQ(point.m_masks.size(), 1); - } + EXPECT_EQ(pointList.GetSize(), 4); + pointList.EnumeratePoints( + []([[maybe_unused]] const AZ::Vector3& position, [[maybe_unused]] const AZ::Vector3& normal, + const SurfaceData::SurfaceTagWeights& masks) -> bool + { + EXPECT_EQ(masks.GetSize(), 1); + return true; + }); } } diff --git a/Gems/SurfaceData/Code/surfacedata_files.cmake b/Gems/SurfaceData/Code/surfacedata_files.cmake index 4b8ac914d7..1c36ca43a0 100644 --- a/Gems/SurfaceData/Code/surfacedata_files.cmake +++ b/Gems/SurfaceData/Code/surfacedata_files.cmake @@ -19,6 +19,7 @@ set(FILES Include/SurfaceData/Utility/SurfaceDataUtility.h Source/SurfaceDataSystemComponent.cpp Source/SurfaceDataSystemComponent.h + Source/SurfaceDataTypes.cpp Source/SurfaceTag.cpp Source/Components/SurfaceDataColliderComponent.cpp Source/Components/SurfaceDataColliderComponent.h diff --git a/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp b/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp index 9a9cb24e4c..3092f7c000 100644 --- a/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp +++ b/Gems/Terrain/Code/Source/Components/TerrainSurfaceDataSystemComponent.cpp @@ -159,25 +159,13 @@ namespace Terrain const bool isHole = !isTerrainValidAtPoint; - SurfaceData::SurfacePoint point; - point.m_entityId = GetEntityId(); - point.m_position = terrainSurfacePoint.m_position; - point.m_normal = terrainSurfacePoint.m_normal; - - // Preallocate enough space for all of our terrain's surface tags, plus the default "terrain" / "terrainHole" tag. - point.m_masks.reserve(terrainSurfacePoint.m_surfaceTags.size() + 1); - - // Add all of the surface tags that the terrain has at this point. - for (auto& tag : terrainSurfacePoint.m_surfaceTags) - { - point.m_masks[tag.m_surfaceType] = tag.m_weight; - } + SurfaceData::SurfaceTagWeights weights(terrainSurfacePoint.m_surfaceTags); // Always add a "terrain" or "terrainHole" tag. const AZ::Crc32 terrainTag = isHole ? Constants::s_terrainHoleTagCrc : Constants::s_terrainTagCrc; - point.m_masks[terrainTag] = 1.0f; + weights.AddSurfaceTagWeight(terrainTag, 1.0f); - surfacePointList.push_back(AZStd::move(point)); + surfacePointList.AddSurfacePoint(GetEntityId(), terrainSurfacePoint.m_position, terrainSurfacePoint.m_normal, weights); } AZ::Aabb TerrainSurfaceDataSystemComponent::GetSurfaceAabb() const diff --git a/Gems/Vegetation/Code/Include/Vegetation/Ebuses/AreaRequestBus.h b/Gems/Vegetation/Code/Include/Vegetation/Ebuses/AreaRequestBus.h index 3323e2a805..605369671d 100644 --- a/Gems/Vegetation/Code/Include/Vegetation/Ebuses/AreaRequestBus.h +++ b/Gems/Vegetation/Code/Include/Vegetation/Ebuses/AreaRequestBus.h @@ -56,12 +56,12 @@ namespace Vegetation ClaimHandle m_handle; AZ::Vector3 m_position; AZ::Vector3 m_normal; - SurfaceData::SurfaceTagWeightMap m_masks; + SurfaceData::SurfaceTagWeights m_masks; }; struct ClaimContext { - SurfaceData::SurfaceTagWeightMap m_masks; + SurfaceData::SurfaceTagWeights m_masks; AZStd::vector m_availablePoints; AZStd::function m_existedCallback; AZStd::function m_createdCallback; diff --git a/Gems/Vegetation/Code/Include/Vegetation/InstanceData.h b/Gems/Vegetation/Code/Include/Vegetation/InstanceData.h index 7e685105f7..b13bef5ae7 100644 --- a/Gems/Vegetation/Code/Include/Vegetation/InstanceData.h +++ b/Gems/Vegetation/Code/Include/Vegetation/InstanceData.h @@ -34,7 +34,7 @@ namespace Vegetation AZ::Quaternion m_rotation = AZ::Quaternion::CreateIdentity(); AZ::Quaternion m_alignment = AZ::Quaternion::CreateIdentity(); float m_scale = 1.0f; - SurfaceData::SurfaceTagWeightMap m_masks; //[LY-90908] remove when surface mask filtering is done in area + SurfaceData::SurfaceTagWeights m_masks; //[LY-90908] remove when surface mask filtering is done in area DescriptorPtr m_descriptorPtr; // Determine if two different sets of instance data are similar enough to be considered the same when placing diff --git a/Gems/Vegetation/Code/Source/AreaSystemComponent.cpp b/Gems/Vegetation/Code/Source/AreaSystemComponent.cpp index 9cd9c1a7cb..e575480ff4 100644 --- a/Gems/Vegetation/Code/Source/AreaSystemComponent.cpp +++ b/Gems/Vegetation/Code/Source/AreaSystemComponent.cpp @@ -1091,7 +1091,7 @@ namespace Vegetation const float vegStep = sectorSizeInMeters / static_cast(sectorDensity); //build a free list of all points in the sector for areas to consume - sectorInfo.m_baseContext.m_masks.clear(); + sectorInfo.m_baseContext.m_masks.Clear(); sectorInfo.m_baseContext.m_availablePoints.clear(); sectorInfo.m_baseContext.m_availablePoints.reserve(sectorDensity * sectorDensity); @@ -1127,16 +1127,19 @@ namespace Vegetation uint claimIndex = 0; for (auto& availablePoints : availablePointsPerPosition) { - for (auto& surfacePoint : availablePoints) - { - sectorInfo.m_baseContext.m_availablePoints.push_back(); - ClaimPoint& claimPoint = sectorInfo.m_baseContext.m_availablePoints.back(); - claimPoint.m_handle = CreateClaimHandle(sectorInfo, ++claimIndex); - claimPoint.m_position = surfacePoint.m_position; - claimPoint.m_normal = surfacePoint.m_normal; - claimPoint.m_masks = surfacePoint.m_masks; - SurfaceData::AddMaxValueForMasks(sectorInfo.m_baseContext.m_masks, surfacePoint.m_masks); - } + availablePoints.EnumeratePoints( + [this, §orInfo, + &claimIndex](const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool + { + sectorInfo.m_baseContext.m_availablePoints.push_back(); + ClaimPoint& claimPoint = sectorInfo.m_baseContext.m_availablePoints.back(); + claimPoint.m_handle = CreateClaimHandle(sectorInfo, ++claimIndex); + claimPoint.m_position = position; + claimPoint.m_normal = normal; + claimPoint.m_masks = masks; + sectorInfo.m_baseContext.m_masks.AddSurfaceWeightsIfGreater(masks); + return true; + }); } } diff --git a/Gems/Vegetation/Code/Source/Components/PositionModifierComponent.cpp b/Gems/Vegetation/Code/Source/Components/PositionModifierComponent.cpp index 17457786f2..ed81ebae87 100644 --- a/Gems/Vegetation/Code/Source/Components/PositionModifierComponent.cpp +++ b/Gems/Vegetation/Code/Source/Components/PositionModifierComponent.cpp @@ -306,31 +306,40 @@ namespace Vegetation m_surfaceTagsToSnapToCombined.clear(); m_surfaceTagsToSnapToCombined.reserve( m_configuration.m_surfaceTagsToSnapTo.size() + - instanceData.m_masks.size()); + instanceData.m_masks.GetSize()); m_surfaceTagsToSnapToCombined.insert(m_surfaceTagsToSnapToCombined.end(), m_configuration.m_surfaceTagsToSnapTo.begin(), m_configuration.m_surfaceTagsToSnapTo.end()); - for (const auto& maskPair : instanceData.m_masks) - { - m_surfaceTagsToSnapToCombined.push_back(maskPair.first); - } + instanceData.m_masks.EnumerateWeights( + [this](AZ::Crc32 surfaceType, [[maybe_unused]] float weight) + { + m_surfaceTagsToSnapToCombined.push_back(surfaceType); + return true; + }); //get the intersection data at the new position - m_points.clear(); + m_points.Clear(); SurfaceData::SurfaceDataSystemRequestBus::Broadcast(&SurfaceData::SurfaceDataSystemRequestBus::Events::GetSurfacePoints, instanceData.m_position, m_surfaceTagsToSnapToCombined, m_points); - if (!m_points.empty()) - { - //sort the intersection data by distance from the new position in case there are multiple intersections at different or unrelated heights - AZStd::sort(m_points.begin(), m_points.end(), [&instanceData](const SurfaceData::SurfacePoint& a, const SurfaceData::SurfacePoint& b) + + // Get the point with the closest distance from the new position in case there are multiple intersections at different or + // unrelated heights + float closestPointDistanceSq = AZStd::numeric_limits::max(); + AZ::Vector3 originalInstanceDataPosition = instanceData.m_position; + m_points.EnumeratePoints( + [&instanceData, originalInstanceDataPosition, &closestPointDistanceSq]( + const AZ::Vector3& position, const AZ::Vector3& normal, const SurfaceData::SurfaceTagWeights& masks) -> bool { - return a.m_position.GetDistanceSq(instanceData.m_position) < b.m_position.GetDistanceSq(instanceData.m_position); + float distanceSq = position.GetDistanceSq(originalInstanceDataPosition); + if (distanceSq < closestPointDistanceSq) + { + instanceData.m_position = position; + instanceData.m_normal = normal; + instanceData.m_masks = masks; + closestPointDistanceSq = distanceSq; + } + return true; }); - - instanceData.m_position = m_points[0].m_position; - instanceData.m_normal = m_points[0].m_normal; - instanceData.m_masks = m_points[0].m_masks; - } } instanceData.m_position.SetZ(instanceData.m_position.GetZ() + delta.GetZ()); diff --git a/Gems/Vegetation/Code/Source/Components/SpawnerComponent.cpp b/Gems/Vegetation/Code/Source/Components/SpawnerComponent.cpp index 404e425489..cc7b75f867 100644 --- a/Gems/Vegetation/Code/Source/Components/SpawnerComponent.cpp +++ b/Gems/Vegetation/Code/Source/Components/SpawnerComponent.cpp @@ -416,9 +416,9 @@ namespace Vegetation AZ_PROFILE_FUNCTION(Entity); //reject entire spawner if there are inclusion tags to consider that don't exist in the context - if (SurfaceData::HasValidTags(context.m_masks) && + if (context.m_masks.HasValidTags() && SurfaceData::HasValidTags(m_inclusiveTagsToConsider) && - !SurfaceData::HasMatchingTags(context.m_masks, m_inclusiveTagsToConsider)) + !context.m_masks.HasAnyMatchingTags(m_inclusiveTagsToConsider)) { VEG_PROFILE_METHOD(DebugNotificationBus::TryQueueBroadcast(&DebugNotificationBus::Events::MarkAreaRejectedByMask, GetEntityId())); return; diff --git a/Gems/Vegetation/Code/Source/Components/SurfaceMaskDepthFilterComponent.cpp b/Gems/Vegetation/Code/Source/Components/SurfaceMaskDepthFilterComponent.cpp index b4e65beb3f..aa6c3a8440 100644 --- a/Gems/Vegetation/Code/Source/Components/SurfaceMaskDepthFilterComponent.cpp +++ b/Gems/Vegetation/Code/Source/Components/SurfaceMaskDepthFilterComponent.cpp @@ -210,26 +210,38 @@ namespace Vegetation float lowerZDistanceRange = useOverrides ? instanceData.m_descriptorPtr->m_surfaceTagDistance.m_lowerDistanceInMeters : m_configuration.m_lowerDistance; float upperZDistanceRange = useOverrides ? instanceData.m_descriptorPtr->m_surfaceTagDistance.m_upperDistanceInMeters : m_configuration.m_upperDistance; + bool passesFilter = false; + if (!surfaceTagsToCompare.empty()) { - m_points.clear(); + m_points.Clear(); SurfaceData::SurfaceDataSystemRequestBus::Broadcast(&SurfaceData::SurfaceDataSystemRequestBus::Events::GetSurfacePoints, instanceData.m_position, surfaceTagsToCompare, m_points); float instanceZ = instanceData.m_position.GetZ(); - for (auto& point : m_points) - { - float pointZ = point.m_position.GetZ(); - float zDistance = instanceZ - pointZ; - if (lowerZDistanceRange <= zDistance && zDistance <= upperZDistanceRange) + m_points.EnumeratePoints( + [instanceZ, lowerZDistanceRange, upperZDistanceRange, &passesFilter]( + const AZ::Vector3& position, + [[maybe_unused]] const AZ::Vector3& normal, [[maybe_unused]] const SurfaceData::SurfaceTagWeights& masks) -> bool { + float pointZ = position.GetZ(); + float zDistance = instanceZ - pointZ; + if (lowerZDistanceRange <= zDistance && zDistance <= upperZDistanceRange) + { + passesFilter = true; + return false; + } return true; - } - } + }); } - // if we get here instance is marked filtered - VEG_PROFILE_METHOD(DebugNotificationBus::TryQueueBroadcast(&DebugNotificationBus::Events::FilterInstance, instanceData.m_id, AZStd::string_view("SurfaceDepthMaskFilter"))); - return false; + if (!passesFilter) + { + // if we get here instance is marked filtered + VEG_PROFILE_METHOD(DebugNotificationBus::TryQueueBroadcast( + &DebugNotificationBus::Events::FilterInstance, instanceData.m_id, AZStd::string_view("SurfaceDepthMaskFilter"))); + } + + return passesFilter; } FilterStage SurfaceMaskDepthFilterComponent::GetFilterStage() const diff --git a/Gems/Vegetation/Code/Source/Components/SurfaceMaskFilterComponent.cpp b/Gems/Vegetation/Code/Source/Components/SurfaceMaskFilterComponent.cpp index 9be6764460..23ef5ab3cb 100644 --- a/Gems/Vegetation/Code/Source/Components/SurfaceMaskFilterComponent.cpp +++ b/Gems/Vegetation/Code/Source/Components/SurfaceMaskFilterComponent.cpp @@ -281,14 +281,15 @@ namespace Vegetation const float exclusiveWeightMax = AZ::GetMax(m_configuration.m_exclusiveWeightMin, m_configuration.m_exclusiveWeightMax); if (useCompTags && - SurfaceData::HasMatchingTags(instanceData.m_masks, m_configuration.m_exclusiveSurfaceMasks, exclusiveWeightMin, exclusiveWeightMax)) + instanceData.m_masks.HasAnyMatchingTags(m_configuration.m_exclusiveSurfaceMasks, exclusiveWeightMin, exclusiveWeightMax)) { VEG_PROFILE_METHOD(DebugNotificationBus::TryQueueBroadcast(&DebugNotificationBus::Events::FilterInstance, instanceData.m_id, AZStd::string_view("SurfaceMaskFilter"))); return false; } if (useDescTags && - SurfaceData::HasMatchingTags(instanceData.m_masks, instanceData.m_descriptorPtr->m_exclusiveSurfaceFilterTags, exclusiveWeightMin, exclusiveWeightMax)) + instanceData.m_masks.HasAnyMatchingTags( + instanceData.m_descriptorPtr->m_exclusiveSurfaceFilterTags, exclusiveWeightMin, exclusiveWeightMax)) { VEG_PROFILE_METHOD(DebugNotificationBus::TryQueueBroadcast(&DebugNotificationBus::Events::FilterInstance, instanceData.m_id, AZStd::string_view("SurfaceMaskFilter"))); return false; @@ -299,13 +300,14 @@ namespace Vegetation const float inclusiveWeightMax = AZ::GetMax(m_configuration.m_inclusiveWeightMin, m_configuration.m_inclusiveWeightMax); if (useCompTags && - SurfaceData::HasMatchingTags(instanceData.m_masks, m_configuration.m_inclusiveSurfaceMasks, inclusiveWeightMin, inclusiveWeightMax)) + instanceData.m_masks.HasAnyMatchingTags(m_configuration.m_inclusiveSurfaceMasks, inclusiveWeightMin, inclusiveWeightMax)) { return true; } if (useDescTags && - SurfaceData::HasMatchingTags(instanceData.m_masks, instanceData.m_descriptorPtr->m_inclusiveSurfaceFilterTags, inclusiveWeightMin, inclusiveWeightMax)) + instanceData.m_masks.HasAnyMatchingTags( + instanceData.m_descriptorPtr->m_inclusiveSurfaceFilterTags, inclusiveWeightMin, inclusiveWeightMax)) { return true; } diff --git a/Gems/Vegetation/Code/Source/Debugger/DebugComponent.cpp b/Gems/Vegetation/Code/Source/Debugger/DebugComponent.cpp index b415a7c41c..abb577acfc 100644 --- a/Gems/Vegetation/Code/Source/Debugger/DebugComponent.cpp +++ b/Gems/Vegetation/Code/Source/Debugger/DebugComponent.cpp @@ -862,7 +862,7 @@ void DebugComponent::PrepareNextReport() SurfaceData::SurfacePointList points; SurfaceData::SurfaceDataSystemRequestBus::Broadcast(&SurfaceData::SurfaceDataSystemRequestBus::Events::GetSurfacePoints, pos, SurfaceData::SurfaceTagVector(), points); - timing.m_worldPosition = points.empty() ? pos : points.front().m_position; + timing.m_worldPosition = points.IsEmpty() ? pos : points.GetHighestSurfacePoint().m_position; return timing; }, [](const SectorTracker& sectorTracker, SectorTiming& sectorTiming) diff --git a/Gems/Vegetation/Code/Tests/VegetationComponentFilterTests.cpp b/Gems/Vegetation/Code/Tests/VegetationComponentFilterTests.cpp index bb1efb5ebd..377ed31890 100644 --- a/Gems/Vegetation/Code/Tests/VegetationComponentFilterTests.cpp +++ b/Gems/Vegetation/Code/Tests/VegetationComponentFilterTests.cpp @@ -80,7 +80,7 @@ namespace UnitTest }); Vegetation::InstanceData vegInstance; - vegInstance.m_masks[maskValue] = 1.0f; + vegInstance.m_masks.AddSurfaceTagWeight(maskValue, 1.0f); // passes { @@ -119,8 +119,7 @@ namespace UnitTest MockSurfaceHandler mockSurfaceHandler; mockSurfaceHandler.m_outPosition = AZ::Vector3::CreateZero(); mockSurfaceHandler.m_outNormal = AZ::Vector3::CreateAxisZ(); - mockSurfaceHandler.m_outMasks.clear(); - mockSurfaceHandler.m_outMasks[SurfaceData::Constants::s_unassignedTagCrc] = 1.0f; + mockSurfaceHandler.m_outMasks.AddSurfaceTagWeight(SurfaceData::Constants::s_unassignedTagCrc, 1.0f); // passes { diff --git a/Gems/Vegetation/Code/Tests/VegetationComponentModifierTests.cpp b/Gems/Vegetation/Code/Tests/VegetationComponentModifierTests.cpp index e60a681512..f9ee408140 100644 --- a/Gems/Vegetation/Code/Tests/VegetationComponentModifierTests.cpp +++ b/Gems/Vegetation/Code/Tests/VegetationComponentModifierTests.cpp @@ -71,7 +71,7 @@ namespace UnitTest MockSurfaceHandler mockSurfaceHandler; mockSurfaceHandler.m_outPosition = AZ::Vector3(vegInstance.m_position.GetX(), vegInstance.m_position.GetY(), 6.0f); mockSurfaceHandler.m_outNormal = AZ::Vector3(0.0f, 0.0f, 1.0f); - mockSurfaceHandler.m_outMasks[crcMask] = 1.0f; + mockSurfaceHandler.m_outMasks.AddSurfaceTagWeight(crcMask, 1.0f); entity->Deactivate(); config.m_autoSnapToSurface = true; diff --git a/Gems/Vegetation/Code/Tests/VegetationMocks.h b/Gems/Vegetation/Code/Tests/VegetationMocks.h index c1d83fb68b..d77862d578 100644 --- a/Gems/Vegetation/Code/Tests/VegetationMocks.h +++ b/Gems/Vegetation/Code/Tests/VegetationMocks.h @@ -329,15 +329,11 @@ namespace UnitTest AZ::Vector3 m_outPosition = {}; AZ::Vector3 m_outNormal = {}; - SurfaceData::SurfaceTagWeightMap m_outMasks; + SurfaceData::SurfaceTagWeights m_outMasks; void GetSurfacePoints([[maybe_unused]] const AZ::Vector3& inPosition, [[maybe_unused]] const SurfaceData::SurfaceTagVector& masks, SurfaceData::SurfacePointList& surfacePointList) const override { ++m_count; - SurfaceData::SurfacePoint outPoint; - outPoint.m_position = m_outPosition; - outPoint.m_normal = m_outNormal; - SurfaceData::AddMaxValueForMasks(outPoint.m_masks, m_outMasks); - surfacePointList.push_back(outPoint); + surfacePointList.AddSurfacePoint(AZ::EntityId(), m_outPosition, m_outNormal, m_outMasks); } void GetSurfacePointsFromRegion([[maybe_unused]] const AZ::Aabb& inRegion, [[maybe_unused]] const AZ::Vector2 stepSize, [[maybe_unused]] const SurfaceData::SurfaceTagVector& desiredTags, From 5baf7d5147330fd6913764bd9ab5ca2bcd5cf23b Mon Sep 17 00:00:00 2001 From: antonmic <56370189+antonmic@users.noreply.github.com> Date: Fri, 4 Feb 2022 12:46:42 -0800 Subject: [PATCH 7/8] removing compute queue flag from SSAO since it's causing crashes for some people Signed-off-by: antonmic <56370189+antonmic@users.noreply.github.com> --- Gems/Atom/Feature/Common/Assets/Passes/SsaoCompute.pass | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Gems/Atom/Feature/Common/Assets/Passes/SsaoCompute.pass b/Gems/Atom/Feature/Common/Assets/Passes/SsaoCompute.pass index 2c48506f8f..3c4a8c76f1 100644 --- a/Gems/Atom/Feature/Common/Assets/Passes/SsaoCompute.pass +++ b/Gems/Atom/Feature/Common/Assets/Passes/SsaoCompute.pass @@ -50,8 +50,7 @@ "FilePath": "Shaders/PostProcessing/SsaoCompute.shader" }, "Make Fullscreen Pass": true, - "PipelineViewTag": "MainCamera", - "Use Async Compute": true + "PipelineViewTag": "MainCamera" }, "FallbackConnections": [ { From 69e0090a0b0f6e952832d17bd7cbae3c1db48309 Mon Sep 17 00:00:00 2001 From: Steve Pham <82231385+spham-amzn@users.noreply.github.com> Date: Fri, 4 Feb 2022 14:20:09 -0800 Subject: [PATCH 8/8] Remove -Wno-return-local-addr warning suppression for GCC - Remove '-Wno-return-local-addr' warning suppression flag for GCC - Fixed discovered error resulting from -Wreturn-local-addr Signed-off-by: Steve Pham <82231385+spham-amzn@users.noreply.github.com> * Remove extra ws Signed-off-by: Steve Pham <82231385+spham-amzn@users.noreply.github.com> --- .../AzCore/RTTI/AzStdOnDemandReflectionSpecializations.cpp | 2 +- cmake/Platform/Common/GCC/Configurations_gcc.cmake | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflectionSpecializations.cpp b/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflectionSpecializations.cpp index c42e32cd42..582d106964 100644 --- a/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflectionSpecializations.cpp +++ b/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflectionSpecializations.cpp @@ -136,7 +136,7 @@ namespace AZ::CommonOnDemandReflections ->template Constructor() ->Attribute(AZ::Script::Attributes::ConstructorOverride, &OnDemandLuaFunctions::ConstructStringView) ->Attribute(AZ::Script::Attributes::ReaderWriterOverride, ScriptContext::CustomReaderWriter(&OnDemandLuaFunctions::StringTypeToLua, &OnDemandLuaFunctions::StringTypeFromLua)) - ->Method("ToString", [](const ContainerType& stringView) { return static_cast(stringView).c_str(); }, { { { "Reference", "String view object being converted to string" } } }) + ->Method("ToString", [](const ContainerType& stringView) { return stringView.data(); }, { { { "Reference", "String view object being converted to string" } } }) ->Attribute(AZ::Script::Attributes::ToolTip, "Converts string_view to string") ->Attribute(AZ::Script::Attributes::Operator, AZ::Script::Attributes::OperatorType::ToString) ->template WrappingMember(&ContainerType::data) diff --git a/cmake/Platform/Common/GCC/Configurations_gcc.cmake b/cmake/Platform/Common/GCC/Configurations_gcc.cmake index 12dd4d7bdd..ad5f3bec28 100644 --- a/cmake/Platform/Common/GCC/Configurations_gcc.cmake +++ b/cmake/Platform/Common/GCC/Configurations_gcc.cmake @@ -55,7 +55,6 @@ ly_append_configurations_options( -Wno-parentheses -Wno-reorder -Wno-restrict - -Wno-return-local-addr -Wno-sequence-point -Wno-sign-compare -Wno-strict-aliasing