Signed-off-by: Nicholas Lawson <70027408+lawsonamzn@users.noreply.git…

…hub.com> Fixes from code review. Signed-off-by: Nicholas Lawson <70027408+lawsonamzn@users.noreply.github.com>
o3de · nick-l-o3de · Apr 7, 2023 · Apr 3, 2023 · Apr 4, 2023 · Apr 4, 2023
commit 3b30b2884603b195ea7b7cfa87ac60cf2793c361
@@ -55,7 +55,7 @@ namespace AZ
                             AZ_Warning(
                                 AZ::SceneAPI::Utilities::WarningWindow,
                                 false,
-                                "'" AZ_STRING_FORMAT "' is not a valid default UV generation method. Check the value of %s in your "
+                                "'%s' is not a valid default UV generation method. Check the value of " AZ_STRING_FORMAT " in your "
                                 "settings registry, and change "
                                 "it to 'LeaveSceneDataAsIs' or 'SphericalProjection'",
                                 stringFromRegistry.c_str(),
@@ -100,7 +100,7 @@ namespace AZ
                 }
 
                 serializeContext->Class<UVsRule, DataTypes::IRule>()
-                    ->Version(2)
+                    ->Version(1)
                     ->Field("generationMethod", &UVsRule::m_generationMethod)
                     ->Field("replaceExisting", &UVsRule::m_replaceExisting);
 

@@ -11,21 +11,67 @@
 
 #include <AzCore/std/smart_ptr/make_shared.h>
 
+#include <SceneAPI/SceneCore/Components/GenerationComponent.h>
 #include <SceneAPI/SceneCore/Containers/Views/PairIterator.h>
 #include <SceneAPI/SceneCore/Containers/Views/SceneGraphDownwardsIterator.h>
 #include <SceneAPI/SceneCore/Containers/Views/SceneGraphChildIterator.h>
-
 #include <SceneAPI/SceneCore/DataTypes/DataTypeUtilities.h>
 #include <SceneAPI/SceneCore/DataTypes/Groups/IGroup.h>
 #include <SceneAPI/SceneCore/DataTypes/GraphData/IMeshData.h>
 #include <SceneAPI/SceneCore/DataTypes/GraphData/IMeshVertexUVData.h>
-
 #include <SceneAPI/SceneData/GraphData/MeshVertexUVData.h>
-
+#include <SceneAPI/SceneData/Rules/UVsRule.h>
 #include <SceneAPI/SceneCore/Utilities/Reporting.h>
 
 namespace AZ::SceneGenerationComponents
 {
+    //! Check whether UVs are to be generated, and if so, generate them.
+    class UVsGenerateComponent : public AZ::SceneAPI::SceneCore::GenerationComponent
+    {
+    public:
+        AZ_COMPONENT(UVsGenerateComponent, s_UVsGenerateComponentTypeId, SceneAPI::SceneCore::GenerationComponent);
+
+        UVsGenerateComponent();
+
+        static void Reflect(AZ::ReflectContext* context);
+
+        // Invoked by the CallProcessorBinder flow.  This is essentially the entry point for this operation.
+        AZ::SceneAPI::Events::ProcessingResult GenerateUVsData(UVsGenerateContext& context);
+
+    private:
+        bool GenerateUVsForMesh(
+            AZ::SceneAPI::Containers::Scene& scene,
+            const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex,
+            AZ::SceneAPI::DataTypes::IMeshData* meshData,
+            const AZ::SceneAPI::DataTypes::UVsGenerationMethod generationMethod,
+            const bool replaceExisting);
+
+        //! How many UV Sets already exist on the mesh?
+        size_t CalcUvSetCount(
+            AZ::SceneAPI::Containers::SceneGraph& graph, const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex) const;
+
+        //! find the Nth UV Set on the mesh and return it.
+        AZ::SceneAPI::DataTypes::IMeshVertexUVData* FindUvData(
+            AZ::SceneAPI::Containers::SceneGraph& graph,
+            const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex,
+            AZ::u64 uvSet) const;
+
+        //! Return the UV Rule (the modifier on the mesh group) or nullptr if no such modifier is applied.
+        const AZ::SceneAPI::SceneData::UVsRule* GetUVsRule(const AZ::SceneAPI::Containers::Scene& scene) const;
+
+        //! Create a new UV set and hook it into the scene graph
+        bool CreateUVsLayer(
+            AZ::SceneAPI::Containers::SceneManifest& manifest,
+            const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex,
+            AZ::SceneAPI::Containers::SceneGraph& graph,
+            SceneData::GraphData::MeshVertexUVData** outUVsData);
+    };
+
+    AZ::ComponentDescriptor* CreateUVsGenerateComponentDescriptor()
+    {
+        return UVsGenerateComponent::CreateDescriptor();
+    }
+
     UVsGenerateComponent::UVsGenerateComponent()
     {
         BindToCall(&UVsGenerateComponent::GenerateUVsData);
@@ -161,7 +207,7 @@ namespace AZ::SceneGenerationComponents
             // for future expansion - add new methods here if you want to support additional methods of UV auto generation.
         default:
             {
-                AZ_Assert(false, "Unknown UVs generation method selected (%d) cannot generate UVs.\n", static_cast<AZ::u32>(generationMethod));
+                AZ_Assert(false, "Unknown UVs generation method selected (%u) cannot generate UVs.\n", static_cast<AZ::u32>(generationMethod));
                 allSuccess = false;
             }
         }

@@ -8,14 +8,25 @@
 
 #pragma once
 
-#include <SceneAPI/SceneCore/Components/GenerationComponent.h>
-#include <SceneAPI/SceneCore/Containers/Scene.h>
-#include <SceneAPI/SceneData/Rules/UVsRule.h>
 #include <AzCore/RTTI/RTTI.h>
+#include <SceneAPI/SceneCore/Events/CallProcessorBus.h>
+#include <SceneAPI/SceneCore/Containers/Scene.h>
+
+namespace AZ
+{
+    namespace SceneAPI::DataTypes
+    {
+        class IMeshData;
+        class IMeshVertexUVData;
+    }
+
+    namespace SceneData::GraphData
+    {
+        class MeshVertexUVData;
+    }
 
-namespace AZ::SceneAPI::DataTypes { class IMeshData; }
-namespace AZ::SceneAPI::DataTypes { class IMeshVertexUVData; }
-namespace AZ::SceneData::GraphData { class MeshVertexUVData;  }
+    class ComponentDescriptor;
+} // namespace AZ
 
 namespace AZ::SceneGenerationComponents
 {
@@ -35,40 +46,9 @@ namespace AZ::SceneGenerationComponents
         AZ::SceneAPI::Containers::Scene& m_scene;
     };
 
-    //! Check whether UVs are to be generated, and if so, generate them.
-    class UVsGenerateComponent
-        : public AZ::SceneAPI::SceneCore::GenerationComponent
-    {
-    public:
-        AZ_COMPONENT(UVsGenerateComponent, "{49121BDD-C7E5-4D39-89BC-28789C90057F}", AZ::SceneAPI::SceneCore::GenerationComponent);
-        UVsGenerateComponent();
+    inline constexpr const char* s_UVsGenerateComponentTypeId = "{49121BDD-C7E5-4D39-89BC-28789C90057F}";
 
-        static void Reflect(AZ::ReflectContext* context);
-
-        // Invoked by the CallProcessorBinder flow.  This is essentially the entry point for this operation.
-        AZ::SceneAPI::Events::ProcessingResult GenerateUVsData(UVsGenerateContext& context);
-    private:
-        bool GenerateUVsForMesh(
-            AZ::SceneAPI::Containers::Scene& scene,
-            const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex,
-            AZ::SceneAPI::DataTypes::IMeshData* meshData,
-            const AZ::SceneAPI::DataTypes::UVsGenerationMethod generationMethod,
-            const bool replaceExisting);
-
-        //! How many UV Sets already exist on the mesh?
-        size_t CalcUvSetCount(AZ::SceneAPI::Containers::SceneGraph& graph, const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex) const;
-
-        //! find the Nth UV Set on the mesh and return it.
-        AZ::SceneAPI::DataTypes::IMeshVertexUVData* FindUvData(AZ::SceneAPI::Containers::SceneGraph& graph, const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex, AZ::u64 uvSet) const;
-
-        //! Return the UV Rule (the modifier on the mesh group) or nullptr if no such modifier is applied.
-        const AZ::SceneAPI::SceneData::UVsRule* GetUVsRule(const AZ::SceneAPI::Containers::Scene& scene) const;
-
-        //! Create a new UV set and hook it into the scene graph
-        bool CreateUVsLayer(
-            AZ::SceneAPI::Containers::SceneManifest& manifest,
-            const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex,
-            AZ::SceneAPI::Containers::SceneGraph& graph,
-            SceneData::GraphData::MeshVertexUVData** outUVsData);
-    };
+    //! This function will be called by the module class to get the descriptor.  Doing it this way saves
+    //! it from having to actually see the entire component declaration here, it can all be in the implementation file.
+    AZ::ComponentDescriptor* CreateUVsGenerateComponentDescriptor();
 } // namespace AZ::SceneGenerationComponents
@@ -33,7 +33,7 @@ namespace AZ::UVsGeneration::Mesh::SphericalMapping
 
         // calculate mesh center by bounding box center.
         AZ::Aabb meshAabb = AZ::Aabb::CreateNull();
-        for (unsigned int vertexIndex = 0; vertexIndex < vertexCount ; ++vertexIndex)
+        for (unsigned int vertexIndex = 0; vertexIndex < vertexCount; ++vertexIndex)
         {
             meshAabb.AddPoint(meshData->GetPosition(vertexIndex));
         }

@@ -7,13 +7,36 @@
  */
 
 #include <Generation/Components/UVsGenerator/UVsPreExportComponent.h>
-#include <Generation/Components/UVsGenerator/UVsGenerateComponent.h>
+
+#include <AzCore/RTTI/RTTI.h>
+
+#include <Generation/Components/UVsGenerator/UVsGenerateComponent.h> // for the context
+#include <SceneAPI/SceneCore/Components/GenerationComponent.h>
+#include <AzCore/Serialization/SerializeContext.h>
 #include <SceneAPI/SceneCore/Events/GenerateEventContext.h>
 #include <SceneAPI/SceneCore/Events/ProcessingResult.h>
 #include <SceneAPI/SceneCore/Events/CallProcessorBus.h>
 
 namespace AZ::SceneGenerationComponents
 {
+    //! This is the component responsible for actually hooking into the scene API's processing flow
+    //! during the generation step.
+    class UVsPreExportComponent : public AZ::SceneAPI::SceneCore::GenerationComponent
+    {
+    public:
+        AZ_COMPONENT(UVsPreExportComponent, s_UVsPreExportComponentTypeId, AZ::SceneAPI::SceneCore::GenerationComponent);
+        UVsPreExportComponent();
+
+        static void Reflect(AZ::ReflectContext* context);
+
+        AZ::SceneAPI::Events::ProcessingResult Register(AZ::SceneAPI::Events::GenerateAdditionEventContext& context);
+    };
+
+    AZ::ComponentDescriptor* CreateUVsPreExportComponentDescriptor()
+    {
+        return UVsPreExportComponent::CreateDescriptor();
+    }
+
     namespace SceneEvents = AZ::SceneAPI::Events;
 
     UVsPreExportComponent::UVsPreExportComponent()

@@ -8,27 +8,16 @@
 
 #pragma once
 
-#include <SceneAPI/SceneCore/Components/GenerationComponent.h>
-#include <SceneAPI/SceneCore/Containers/Scene.h>
-
-#include <AzCore/RTTI/RTTI.h>
-
-namespace AZ::SceneAPI::Events { class GenerateAdditionEventContext; }
+namespace AZ
+{
+    class ComponentDescriptor;
+}
 
 namespace AZ::SceneGenerationComponents
 {
-    //! This is the component responsible for actually hooking into the scene API's processing flow
-    //! during the generation step.
-    class UVsPreExportComponent
-        : public AZ::SceneAPI::SceneCore::GenerationComponent
-    {
-    public:
-        AZ_COMPONENT(UVsPreExportComponent, "{64F79C1E-CED6-42A9-8229-6607F788C731}", AZ::SceneAPI::SceneCore::GenerationComponent)
-
-        UVsPreExportComponent();
-
-        static void Reflect(AZ::ReflectContext* context);
+    inline constexpr const char* s_UVsPreExportComponentTypeId = "{64F79C1E-CED6-42A9-8229-6607F788C731}";
 
-        AZ::SceneAPI::Events::ProcessingResult Register(AZ::SceneAPI::Events::GenerateAdditionEventContext& context);
-    };
+    //! This function will be called by the module class to get the descriptor.  Doing it this way saves
+    //! it from having to actually see the entire component declaration here, it can all be in the implementation file.
+    AZ::ComponentDescriptor* CreateUVsPreExportComponentDescriptor();
 } // namespace AZ::SceneGenerationComponents
@@ -44,11 +44,11 @@ namespace AZ
                     SceneProcessingConfig::SoftNameBehavior::CreateDescriptor(),
                     SceneBuilder::BuilderPluginComponent::CreateDescriptor(),
                     SceneBuilder::SceneSerializationHandler::CreateDescriptor(),
-                        AZ::SceneGenerationComponents::TangentPreExportComponent::CreateDescriptor(),
-                        AZ::SceneGenerationComponents::TangentGenerateComponent::CreateDescriptor(),
-                        AZ::SceneGenerationComponents::UVsPreExportComponent::CreateDescriptor(),
-                        AZ::SceneGenerationComponents::UVsGenerateComponent::CreateDescriptor(),
-                        AZ::SceneGenerationComponents::MeshOptimizerComponent::CreateDescriptor(),
+                    AZ::SceneGenerationComponents::TangentPreExportComponent::CreateDescriptor(),
+                    AZ::SceneGenerationComponents::TangentGenerateComponent::CreateDescriptor(),
+                    AZ::SceneGenerationComponents::CreateUVsGenerateComponentDescriptor(),
+                    AZ::SceneGenerationComponents::CreateUVsPreExportComponentDescriptor(),
+                    AZ::SceneGenerationComponents::MeshOptimizerComponent::CreateDescriptor(),
                 });
 
                 // This is an internal Amazon gem, so register it's components for metrics tracking, otherwise the name of the component won't get sent back.