UX Workflow improvements for Physics Materials (#1237)

- Added button to PhysX Collider Component in PhysX Mesh's field to open FBX Settings.
- Added button Material Selection to open the physics material library in Asset Editor.
- Default Material in PhysX configuration is read only and consistent with the text in combo boxes.
- Material configuration field "Surface Type" renamed to "Name"
- Fixed bug in EditorColliderComponent where the material selection was not updated when changing the library.
- Fixed bug where the materials selection was not set to default when a physics material from the asset was not found in the library.
- Added attributes 'BrowseButtonEnabled' and 'BrowseButtonVisible' to PropertyAssetCtrl.
- Updated physx configuration setreg files of AutomatedTesting project.

AutomatedTesting/Registry/C3510644_Collider_CollisionGroups.setreg_override

@@ -119,9 +119,6 @@
                             ]
                         }
                     },
-                    "DefaultMaterial": {
-                        "SurfaceType": "Default_1"
-                    },
                     "MaterialLibrary": {
                         "assetId": {
                             "guid": "{3A055A3F-8CB7-5FEE-B437-EB365FACD0D4}"

AutomatedTesting/Registry/C4044459_Material_DynamicFriction.setreg_override

@@ -101,9 +101,6 @@
                             ]
                         }
                     },
-                    "DefaultMaterial": {
-                        "SurfaceType": "Default_1"
-                    },
                     "MaterialLibrary": {
                         "assetId": {
                             "guid": "{6AA79EE4-7EC3-5717-87AE-EDD7D886FD7F}"

AutomatedTesting/Registry/C4976227_Collider_NewGroup.setreg_override

@@ -107,9 +107,6 @@
                             ]
                         }
                     },
-                    "DefaultMaterial": {
-                        "SurfaceType": "Default_1"
-                    },
                     "MaterialLibrary": {
                         "assetId": {
                             "guid": "{3A055A3F-8CB7-5FEE-B437-EB365FACD0D4}"

AutomatedTesting/Registry/C4976244_Collider_SameGroupSameLayerCollision.setreg_override

@@ -119,9 +119,6 @@
                             ]
                         }
                     },
-                    "DefaultMaterial": {
-                        "SurfaceType": "Default_1"
-                    },
                     "MaterialLibrary": {
                         "assetId": {
                             "guid": "{3A055A3F-8CB7-5FEE-B437-EB365FACD0D4}"

AutomatedTesting/Registry/C4976245_PhysXCollider_CollisionLayerTest.setreg_override

@@ -119,9 +119,6 @@
                             ]
                         }
                     },
-                    "DefaultMaterial": {
-                        "SurfaceType": "Default_1"
-                    },
                     "MaterialLibrary": {
                         "assetId": {
                             "guid": "{3A055A3F-8CB7-5FEE-B437-EB365FACD0D4}"

AutomatedTesting/Registry/C4982593_PhysXCollider_CollisionLayer.setreg_override

@@ -119,9 +119,6 @@
                             ]
                         }
                     },
-                    "DefaultMaterial": {
-                        "SurfaceType": "Default_1"
-                    },
                     "MaterialLibrary": {
                         "assetId": {
                             "guid": "{3A055A3F-8CB7-5FEE-B437-EB365FACD0D4}"

AutomatedTesting/Registry/physxsystemconfiguration.setreg

@@ -101,9 +101,6 @@
                             ]
                         }
                     },
-                    "DefaultMaterial": {
-                        "SurfaceType": "Default_1"
-                    },
                     "MaterialLibrary": {
                         "assetId": {
                             "guid": "{3A055A3F-8CB7-5FEE-B437-EB365FACD0D4}"

Code/Framework/AzFramework/AzFramework/Physics/Configuration/SystemConfiguration.h

@@ -46,7 +46,7 @@ namespace AzPhysics
         //! Each Physics Scene uses this as a base and will override as needed.
         CollisionConfiguration m_collisionConfig;
 
-        Physics::MaterialConfiguration m_defaultMaterialConfiguration; //!< Default material parameters for the project.
+        Physics::DefaultMaterialConfiguration m_defaultMaterialConfiguration; //!< Default material parameters for the project.
         AZ::Data::Asset<Physics::MaterialLibraryAsset> m_materialLibraryAsset = AZ::Data::AssetLoadBehavior::NoLoad; //!< Material Library exposed by the system component SystemBus API.
 
         //! Controls whether the Physics System will self register to the TickBus and call StartSimulation / FinishSimulation on each Scene.

Code/Framework/AzFramework/AzFramework/Physics/Material.cpp

@@ -83,14 +83,12 @@ namespace Physics
             AZ::EditContext* editContext = serializeContext->GetEditContext();
             if (editContext)
             {
-                AZStd::unordered_set<AZStd::string> forbiddenSurfaceTypeNames;
-                forbiddenSurfaceTypeNames.insert("Default");
                 editContext->Class<Physics::MaterialConfiguration>("", "")
                     ->ClassElement(AZ::Edit::ClassElements::EditorData, "Physics Material")
-                    ->DataElement(MaterialConfiguration::s_configLineEdit, &MaterialConfiguration::m_surfaceType, "Surface type", "Game surface type") // Uses ConfigStringLineEditCtrl in PhysX gem.
+                    ->DataElement(MaterialConfiguration::s_configLineEdit, &MaterialConfiguration::m_surfaceType, "Name", "Name of the physics material") // Uses ConfigStringLineEditCtrl in PhysX gem.
                         ->Attribute(AZ::Edit::Attributes::MaxLength, 64)
+                        ->Attribute(AZ::Edit::Attributes::ReadOnly, &MaterialConfiguration::IsNameReadOnly)
                         ->Attribute(MaterialConfiguration::s_stringGroup, AZ_CRC("LineEditGroupSurfaceType", 0x6670659e))
-                        ->Attribute(MaterialConfiguration::s_forbiddenStringSet, forbiddenSurfaceTypeNames)
                     ->DataElement(AZ::Edit::UIHandlers::Default, &MaterialConfiguration::m_staticFriction, "Static friction", "Friction coefficient when object is still")
                         ->Attribute(AZ::Edit::Attributes::Min, 0.f)
                     ->DataElement(AZ::Edit::UIHandlers::Default, &MaterialConfiguration::m_dynamicFriction, "Dynamic friction", "Friction coefficient when object is moving")
@@ -178,6 +176,23 @@ namespace Physics
 
     //////////////////////////////////////////////////////////////////////////
 
+    DefaultMaterialConfiguration::DefaultMaterialConfiguration()
+    {
+        m_surfaceType = Physics::DefaultPhysicsMaterialLabel;
+    }
+
+    void DefaultMaterialConfiguration::Reflect(AZ::ReflectContext* context)
+    {
+        if (auto* serializeContext = azrtti_cast<AZ::SerializeContext*>(context))
+        {
+            serializeContext->Class<Physics::DefaultMaterialConfiguration, Physics::MaterialConfiguration>()
+                ->Version(1)
+                ;
+        }
+    }
+
+    //////////////////////////////////////////////////////////////////////////
+
     void MaterialLibraryAsset::Reflect(AZ::ReflectContext * context)
     {
         AZ::SerializeContext* serializeContext = azrtti_cast<AZ::SerializeContext*>(context);
@@ -212,7 +227,7 @@ namespace Physics
         if (serializeContext)
         {
             serializeContext->Class<Physics::MaterialInfoReflectionWrapper>()
-                ->Version(1)
+                ->Version(2)
                 ->Field("DefaultMaterial", &MaterialInfoReflectionWrapper::m_defaultMaterialConfiguration)
                 ->Field("Asset", &MaterialInfoReflectionWrapper::m_materialLibraryAsset)
                 ;
@@ -372,6 +387,7 @@ namespace Physics
             serializeContext->Class<Physics::MaterialSelection>()
                 ->Version(3, &ClassConverters::MaterialSelectionConverter)
                 ->EventHandler<MaterialSelectionEventHandler>()
+                ->Field("EditContextMaterialLibrary", &MaterialSelection::m_editContextMaterialLibrary)
                 ->Field("MaterialIds", &MaterialSelection::m_materialIdsAssignedToSlots)
                 ;
 
@@ -380,6 +396,12 @@ namespace Physics
                 editContext->Class<Physics::MaterialSelection>("Physics Materials", "Select which physics materials to use for each element of this object")
                     ->ClassElement(AZ::Edit::ClassElements::EditorData, "")
                         ->Attribute(AZ::Edit::Attributes::AutoExpand, true)
+                    ->DataElement(AZ::Edit::UIHandlers::Default, &MaterialSelection::m_editContextMaterialLibrary, "Library", "Physics material library from PhysX Configuration")
+                        ->Attribute(AZ::Edit::Attributes::ContainerCanBeModified, false)
+                        ->Attribute(AZ_CRC_CE("BrowseButtonEnabled"), false)
+                        ->Attribute(AZ_CRC_CE("EditButton"), "")
+                        ->Attribute(AZ_CRC_CE("EditDescription"), "Open in Asset Editor")
+                        ->Attribute(AZ::Edit::Attributes::DefaultAsset, &MaterialSelection::GetMaterialLibraryId)
                     ->DataElement(AZ::Edit::UIHandlers::Default, &MaterialSelection::m_materialIdsAssignedToSlots, "", "")
                         ->ElementAttribute(Attributes::MaterialLibraryAssetId, &MaterialSelection::GetMaterialLibraryId)
                         ->Attribute(AZ::Edit::Attributes::IndexedChildNameLabelOverride, &MaterialSelection::GetMaterialSlotLabel)

Code/Framework/AzFramework/AzFramework/Physics/Material.h

@@ -99,10 +99,13 @@ namespace Physics
     class MaterialConfiguration
     {
     public:
-        AZ_TYPE_INFO(Physics::MaterialConfiguration, "{8807CAA1-AD08-4238-8FDB-2154ADD084A1}");
+        AZ_RTTI(Physics::MaterialConfiguration, "{8807CAA1-AD08-4238-8FDB-2154ADD084A1}");
 
         static void Reflect(AZ::ReflectContext* context);
 
+        MaterialConfiguration() = default;
+        virtual ~MaterialConfiguration() = default;
+
         const static AZ::Crc32 s_stringGroup; ///< Edit context data attribute. Identifies a string group instance. String values in the same group are unique.
         const static AZ::Crc32 s_forbiddenStringSet; ///<  Edit context data attribute. A set of strings that are not acceptable as values to the data element. Can be AZStd::unordered_set<AZStd::string>, AZStd::set<AZStd::string>, AZStd::vector<AZStd::string>
         const static AZ::Crc32 s_configLineEdit; ///< Edit context data element handler. Creates custom line edit widget that allows string values to be unique in a group.
@@ -123,11 +126,28 @@ namespace Physics
         bool operator==(const MaterialConfiguration& other) const;
         bool operator!=(const MaterialConfiguration& other) const;
 
+    protected:
+        virtual bool IsNameReadOnly() const { return false; }
+
     private:
         static bool VersionConverter(AZ::SerializeContext& context, AZ::SerializeContext::DataElementNode& classElement);
         static AZ::Color GenerateDebugColor(const char* materialName);
     };
 
+    class DefaultMaterialConfiguration
+        : public MaterialConfiguration
+    {
+    public:
+        AZ_RTTI(Physics::DefaultMaterialConfiguration, "{A1F64C5C-D413-4757-9D42-51DD0EBFC270}", Physics::MaterialConfiguration);
+
+        static void Reflect(AZ::ReflectContext* context);
+        
+        DefaultMaterialConfiguration();
+
+    protected:
+        bool IsNameReadOnly() const override { return true; }
+    };
+
     namespace Attributes
     {
         const static AZ::Crc32 MaterialLibraryAssetId = AZ_CRC("MaterialAssetId", 0x4a88a3f5);
@@ -239,7 +259,7 @@ namespace Physics
         AZ_TYPE_INFO(Physics::MaterialInfoReflectionWrapper, "{02AB8CBC-D35B-4E0F-89BA-A96D94DAD4F9}");
         static void Reflect(AZ::ReflectContext* context);
 
-        Physics::MaterialConfiguration m_defaultMaterialConfiguration;
+        Physics::DefaultMaterialConfiguration m_defaultMaterialConfiguration;
         AZ::Data::Asset<Physics::MaterialLibraryAsset> m_materialLibraryAsset =
             AZ::Data::AssetLoadBehavior::NoLoad;
     };
@@ -298,6 +318,10 @@ namespace Physics
 
         // EditorContext callbacks
         AZStd::string GetMaterialSlotLabel(int index);
+
+        // Only used for Edit Context as it requires to have an asset reflected.
+        // To get the material library use GetMaterialLibraryId()
+        AZ::Data::Asset<Physics::MaterialLibraryAsset> m_editContextMaterialLibrary{ AZ::Data::AssetLoadBehavior::NoLoad };
     };
 
     /// Editor Bus used to assign material to terrain surface id.

Code/Framework/AzFramework/AzFramework/Physics/Shape.cpp

@@ -62,7 +62,7 @@ namespace Physics
                         ->Attribute(AZ::Edit::Attributes::Step, 0.01f)
                     ->DataElement(AZ::Edit::UIHandlers::Default, &ColliderConfiguration::m_rotation, "Rotation", "Local rotation relative to the rigid body")
                         ->Attribute(AZ::Edit::Attributes::Visibility, &ColliderConfiguration::GetOffsetVisibility)
-                    ->DataElement(AZ::Edit::UIHandlers::Default, &ColliderConfiguration::m_materialSelection, "Physics Material", "Select physics material library and which materials to use for the shape")
+                    ->DataElement(AZ::Edit::UIHandlers::Default, &ColliderConfiguration::m_materialSelection, "Physics Materials", "Select which physics materials to use for each element of this shape")
                         ->Attribute(AZ::Edit::Attributes::Visibility, &ColliderConfiguration::GetMaterialSelectionVisibility)
                     ->DataElement(AZ::Edit::UIHandlers::Default, &ColliderConfiguration::m_tag, "Tag", "Tag used to identify colliders from one another")
                     ->DataElement(AZ::Edit::UIHandlers::Default, &ColliderConfiguration::m_restOffset, "Rest offset",

Code/Framework/AzFramework/AzFramework/Physics/Utils.cpp

@@ -118,6 +118,7 @@ namespace Physics
             AzPhysics::TriggerEvent::Reflect(context);
             AzPhysics::SceneConfiguration::Reflect(context);
             MaterialConfiguration::Reflect(context);
+            DefaultMaterialConfiguration::Reflect(context);
             MaterialLibraryAsset::Reflect(context);
             MaterialInfoReflectionWrapper::Reflect(context);
             JointLimitConfiguration::Reflect(context);

Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyAssetCtrl.cpp

@@ -1130,6 +1130,16 @@ namespace AzToolsFramework
         m_browseEdit->setAttachedButtonIcon(icon);
     }
 
+    void PropertyAssetCtrl::SetBrowseButtonEnabled(bool enabled)
+    {
+        m_browseEdit->setEnabled(enabled);
+    }
+
+    void PropertyAssetCtrl::SetBrowseButtonVisible(bool visible)
+    {
+        m_browseEdit->setVisible(visible);
+    }
+
     const QModelIndex PropertyAssetCtrl::GetSourceIndex(const QModelIndex& index)
     {
         if (!index.isValid())
@@ -1360,6 +1370,22 @@ namespace AzToolsFramework
                 GUI->SetBrowseButtonIcon(QIcon(iconPath.c_str()));
             }
         }
+        else if (attrib == AZ_CRC_CE("BrowseButtonEnabled"))
+        {
+            bool enabled = true;
+            if (attrValue->Read<bool>(enabled))
+            {
+                GUI->SetBrowseButtonEnabled(enabled);
+            }
+        }
+        else if (attrib == AZ_CRC_CE("BrowseButtonVisible"))
+        {
+            bool visible = true;
+            if (attrValue->Read<bool>(visible))
+            {
+                GUI->SetBrowseButtonVisible(visible);
+            }
+        }
         else if (attrib == AZ_CRC_CE("Thumbnail"))
         {
             bool showThumbnail = false;

Code/Framework/AzToolsFramework/AzToolsFramework/UI/PropertyEditor/PropertyAssetCtrl.hxx

@@ -208,6 +208,8 @@ namespace AzToolsFramework
         void SetEditButtonIcon(const QIcon& icon);
         void SetEditButtonTooltip(QString tooltip);
         void SetBrowseButtonIcon(const QIcon& icon);
+        void SetBrowseButtonEnabled(bool enabled);
+        void SetBrowseButtonVisible(bool visible);
         void SetClearButtonEnabled(bool enable);
         void SetClearButtonVisible(bool visible);
 

Gems/PhysX/Code/Editor/ConfigStringLineEditCtrl.cpp

@@ -219,6 +219,14 @@ namespace PhysX
                 GUI->setMaxLen(maxLen);
             }
         }
+        else if (attrib == AZ::Edit::Attributes::ReadOnly)
+        {
+            bool isReadOnly = false;
+            if (attrValue->Read<AZ::Crc32>(isReadOnly))
+            {
+                GUI->setEnabled(!isReadOnly);
+            }
+        }
         else if (attrib == Physics::MaterialConfiguration::s_stringGroup)
         {
             AZ::Crc32 uniqueGroup;

Gems/PhysX/Code/Include/PhysX/MeshAsset.h

@@ -75,6 +75,8 @@ namespace PhysX
             AZ_CLASS_ALLOCATOR(MeshAsset, AZ::SystemAllocator, 0);
             AZ_RTTI(MeshAsset, "{7A2871B9-5EAB-4DE0-A901-B0D2C6920DDB}", AZ::Data::AssetData);
 
+            static void Reflect(AZ::ReflectContext* context);
+
             MeshAssetData m_assetData;
         };
     } // namespace Pipeline

Gems/PhysX/Code/Source/EditorColliderComponent.cpp

@@ -58,10 +58,12 @@ namespace PhysX
             {
                 editContext->Class<EditorProxyAssetShapeConfig>("EditorProxyShapeConfig", "PhysX Base shape collider")
                     ->ClassElement(AZ::Edit::ClassElements::EditorData, "")
-                    ->Attribute(AZ::Edit::Attributes::AutoExpand, true)
+                        ->Attribute(AZ::Edit::Attributes::AutoExpand, true)
                     ->DataElement(AZ::Edit::UIHandlers::Default, &EditorProxyAssetShapeConfig::m_pxAsset, "PhysX Mesh", "PhysX mesh collider asset")
+                        ->Attribute(AZ_CRC_CE("EditButton"), "")
+                        ->Attribute(AZ_CRC_CE("EditDescription"), "Open in FBX Settings")
                     ->DataElement(AZ::Edit::UIHandlers::Default, &EditorProxyAssetShapeConfig::m_configuration, "Configuration", "Configuration of asset shape")
-                    ->Attribute(AZ::Edit::Attributes::Visibility, AZ::Edit::PropertyVisibility::ShowChildrenOnly);
+                        ->Attribute(AZ::Edit::Attributes::Visibility, AZ::Edit::PropertyVisibility::ShowChildrenOnly);
             }
         }
     }
@@ -356,6 +358,7 @@ namespace PhysX
             [this](const AZ::Data::AssetId& defaultMaterialLibrary)
             {
                 m_configuration.m_materialSelection.OnMaterialLibraryChanged(defaultMaterialLibrary);
+                UpdateMaterialSlotsFromMeshAsset();
 
                 AzToolsFramework::PropertyEditorGUIMessages::Bus::Broadcast(&AzToolsFramework::PropertyEditorGUIMessages::RequestRefresh,
                     AzToolsFramework::PropertyModificationRefreshLevel::Refresh_AttributesAndValues);

Gems/PhysX/Code/Source/Material.cpp

@@ -441,6 +441,7 @@ namespace PhysX
                     "UpdateMaterialSelectionFromPhysicsAsset: Physics material '%s' not found in the material library. Mesh material '%s' will use the default physics material.",
                     physicsMaterialNameFromPhysicsAsset.c_str(),
                     meshAsset->m_assetData.m_materialNames[slotIndex].c_str());
+                materialSelection.SetMaterialId(Physics::MaterialId(), slotIndex);
             }
         }
     }

Gems/PhysX/Code/Source/Pipeline/MeshAssetHandler.cpp

@@ -17,6 +17,7 @@
 #include <AzCore/IO/GenericStreams.h>
 #include <AzCore/IO/FileIO.h>
 #include <AzCore/Serialization/Utils.h>
+#include <AzCore/Serialization/EditContext.h>
 #include <PhysX/MeshAsset.h>
 #include <PhysX/SystemComponentBus.h>
 #include <Source/Pipeline/MeshAssetHandler.h>
@@ -158,6 +159,27 @@ namespace PhysX
             }
         }
 
+        void MeshAsset::Reflect(AZ::ReflectContext* context)
+        {
+            MeshAssetData::Reflect(context);
+
+            if (auto* serializeContext = azrtti_cast<AZ::SerializeContext*>(context))
+            {
+                serializeContext->Class<MeshAsset>()
+                    ->Field("MeshAssetData", &MeshAsset::m_assetData)
+                    ;
+
+                // Note: This class needs to have edit context reflection so PropertyAssetCtrl::OnEditButtonClicked
+                //       can open the asset with the preferred asset editor (FBX Settings).
+                if (auto* editContext = serializeContext->GetEditContext())
+                {
+                    editContext->Class<MeshAsset>("PhysX Mesh Asset", "")
+                        ->ClassElement(AZ::Edit::ClassElements::EditorData, "")
+                        ;
+                }
+            }
+        }
+
         void AssetColliderConfiguration::Reflect(AZ::ReflectContext* context)
         {
             if (AZ::SerializeContext* serializeContext = azrtti_cast<AZ::SerializeContext*>(context))

Gems/PhysX/Code/Source/Pipeline/MeshExporter.cpp

@@ -311,7 +311,7 @@ namespace PhysX
                         AZStd::string materialName = DefaultMaterialName;
                         if (!localFbxMaterialsList.empty())
                         {
-                            int materialId = nodeMesh->GetFaceMaterialId(faceIndex);
+                            const int materialId = nodeMesh->GetFaceMaterialId(faceIndex);
                             if (materialId >= localFbxMaterialsList.size())
                             {
                                 AZ_TracePrintf(AZ::SceneAPI::Utilities::ErrorWindow,
@@ -332,7 +332,7 @@ namespace PhysX
                             }
                         }
 
-                        AZ::u16 materialIndex = InsertMaterialIndexByName(materialName, assetMaterialData);
+                        const AZ::u16 materialIndex = InsertMaterialIndexByName(materialName, assetMaterialData);
                         assetMaterialData.m_nodesToPerFaceMaterialIndices[nodeName][faceIndex] = materialIndex;
                     }
                 }
@@ -729,7 +729,7 @@ namespace PhysX
             {
                 // Gather material data from asset for the mesh group
                 AZStd::optional<Utils::AssetMaterialsData> assetMaterialData = Utils::GatherMaterialsFromMeshGroup(pxMeshGroup, graph);
-                if (!assetMaterialData)
+                if (!assetMaterialData.has_value())
                 {
                     return SceneEvents::ProcessingResult::Failure;
                 }

Gems/PhysX/Code/Source/SystemComponent.cpp

@@ -94,7 +94,7 @@ namespace PhysX
     void SystemComponent::Reflect(AZ::ReflectContext* context)
     {
         D6JointLimitConfiguration::Reflect(context);
-        Pipeline::MeshAssetData::Reflect(context);
+        Pipeline::MeshAsset::Reflect(context);
 
         PhysX::ReflectionUtils::ReflectPhysXOnlyApi(context);
 

Gems/PhysX/Code/Tests/PhysXSceneQueryTests.cpp

@@ -172,7 +172,7 @@ namespace PhysX
         ASSERT_EQ(hit.m_material, rigidBody->GetShape(0).get()->GetMaterial().get());
 
         const AZStd::string& typeName = hit.m_material->GetSurfaceTypeName();
-        ASSERT_EQ(typeName, AZStd::string("Default"));
+        ASSERT_EQ(typeName, Physics::DefaultPhysicsMaterialLabel);
     }
 
     TEST_F(PhysXSceneQueryFixture, RayCast_AgainstStaticObject_ReturnsHit)