[Terrain] PaintBrush bugfixes and code cleanup (#13213)

* Bugfixes and code rearranging.

Signed-off-by: Mike Balfour <[email protected]>

* Removed accidental double-include.

Signed-off-by: Mike Balfour <[email protected]>

* Build fix for non-unity builds.

Signed-off-by: Mike Balfour <[email protected]>

Signed-off-by: Mike Balfour <[email protected]>

Code/Framework/AzToolsFramework/AzToolsFramework/PaintBrush/PaintBrushSubModeCluster.cpp

@@ -0,0 +1,87 @@
+/*
+ * 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 <AzToolsFramework/API/ToolsApplicationAPI.h>
+#include <AzToolsFramework/PaintBrush/PaintBrushSubModeCluster.h>
+#include <AzToolsFramework/PaintBrushSettings/PaintBrushSettingsRequestBus.h>
+#include <AzToolsFramework/PaintBrushSettings/PaintBrushSettingsWindow.h>
+
+namespace AzToolsFramework
+{
+    PaintBrushSubModeCluster::PaintBrushSubModeCluster()
+    {
+        auto RegisterClusterButton = [](AzToolsFramework::ViewportUi::ClusterId clusterId,
+                                        const char* iconName,
+                                        const char* tooltip) -> AzToolsFramework::ViewportUi::ButtonId
+        {
+            AzToolsFramework::ViewportUi::ButtonId buttonId;
+            AzToolsFramework::ViewportUi::ViewportUiRequestBus::EventResult(
+                buttonId,
+                AzToolsFramework::ViewportUi::DefaultViewportId,
+                &AzToolsFramework::ViewportUi::ViewportUiRequestBus::Events::CreateClusterButton,
+                clusterId,
+                AZStd::string::format(":/stylesheet/img/UI20/toolbar/%s.svg", iconName));
+
+            AzToolsFramework::ViewportUi::ViewportUiRequestBus::Event(
+                AzToolsFramework::ViewportUi::DefaultViewportId,
+                &AzToolsFramework::ViewportUi::ViewportUiRequestBus::Events::SetClusterButtonTooltip,
+                clusterId,
+                buttonId,
+                tooltip);
+
+            return buttonId;
+        };
+
+        // create the cluster for showing the Paint Brush Settings window
+        AzToolsFramework::ViewportUi::ViewportUiRequestBus::EventResult(
+            m_paintBrushControlClusterId,
+            AzToolsFramework::ViewportUi::DefaultViewportId,
+            &AzToolsFramework::ViewportUi::ViewportUiRequestBus::Events::CreateCluster,
+            AzToolsFramework::ViewportUi::Alignment::TopLeft);
+
+        // create and register the "Show Paint Brush Settings" button.
+        // This button is needed because the window is only shown while in component mode, and the window can be closed by the user,
+        // so we need to provide an alternate way for the user to re-open the window.
+        m_paintModeButtonId = RegisterClusterButton(m_paintBrushControlClusterId, "Paint", "Switch to Paint Mode");
+        m_eyedropperModeButtonId = RegisterClusterButton(m_paintBrushControlClusterId, "Eyedropper", "Switch to Eyedropper Mode");
+        m_smoothModeButtonId = RegisterClusterButton(m_paintBrushControlClusterId, "Smooth", "Switch to Smooth Mode");
+
+        m_buttonSelectionHandler = AZ::Event<AzToolsFramework::ViewportUi::ButtonId>::Handler(
+            [this](AzToolsFramework::ViewportUi::ButtonId buttonId)
+            {
+                AzToolsFramework::PaintBrushMode brushMode = AzToolsFramework::PaintBrushMode::Paintbrush;
+
+                if (buttonId == m_eyedropperModeButtonId)
+                {
+                    brushMode = AzToolsFramework::PaintBrushMode::Eyedropper;
+                }
+                else if (buttonId == m_smoothModeButtonId)
+                {
+                    brushMode = AzToolsFramework::PaintBrushMode::Smooth;
+                }
+
+                AzToolsFramework::OpenViewPane(::PaintBrush::s_paintBrushSettingsName);
+
+                AzToolsFramework::PaintBrushSettingsRequestBus::Broadcast(
+                    &AzToolsFramework::PaintBrushSettingsRequestBus::Events::SetBrushMode, brushMode);
+            });
+        AzToolsFramework::ViewportUi::ViewportUiRequestBus::Event(
+            AzToolsFramework::ViewportUi::DefaultViewportId,
+            &AzToolsFramework::ViewportUi::ViewportUiRequestBus::Events::RegisterClusterEventHandler,
+            m_paintBrushControlClusterId,
+            m_buttonSelectionHandler);
+    }
+
+    PaintBrushSubModeCluster::~PaintBrushSubModeCluster()
+    {
+        AzToolsFramework::ViewportUi::ViewportUiRequestBus::Event(
+            AzToolsFramework::ViewportUi::DefaultViewportId,
+            &AzToolsFramework::ViewportUi::ViewportUiRequestBus::Events::RemoveCluster,
+            m_paintBrushControlClusterId);
+    }
+} // namespace AzToolsFramework

Code/Framework/AzToolsFramework/AzToolsFramework/PaintBrush/PaintBrushSubModeCluster.h

@@ -0,0 +1,32 @@
+/*
+ * 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 <AzToolsFramework/ViewportUi/ViewportUiRequestBus.h>
+
+namespace AzToolsFramework
+{
+    //! Create and manage the Viewport UI SubMode cluster of buttons that enable the user to switch between paint modes.
+    class PaintBrushSubModeCluster
+    {
+    public:
+        PaintBrushSubModeCluster();
+        ~PaintBrushSubModeCluster();
+
+    private:
+        AZ::Event<ViewportUi::ButtonId>::Handler m_buttonSelectionHandler;
+
+        ViewportUi::ClusterId m_paintBrushControlClusterId;
+
+        ViewportUi::ButtonId m_paintModeButtonId;
+        ViewportUi::ButtonId m_eyedropperModeButtonId;
+        ViewportUi::ButtonId m_smoothModeButtonId;
+    };
+} // namespace AzToolsFramework
+

Code/Framework/AzToolsFramework/AzToolsFramework/aztoolsframework_files.cmake

@@ -256,6 +256,8 @@ set(FILES
     PaintBrush/PaintBrush.cpp
     PaintBrush/PaintBrush.h
     PaintBrush/PaintBrushNotificationBus.h
+    PaintBrush/PaintBrushSubModeCluster.cpp
+    PaintBrush/PaintBrushSubModeCluster.h
     PaintBrushSettings/PaintBrushSettings.cpp
     PaintBrushSettings/PaintBrushSettings.h
     PaintBrushSettings/PaintBrushSettingsNotificationBus.h

Gems/GradientSignal/Code/Source/Components/ImageGradientComponent.cpp

@@ -724,7 +724,10 @@ namespace GradientSignal
         if (shouldRefreshModificationBuffer)
         {
             m_modifiedImageData.resize(0);
-            CreateImageModificationBuffer();
+            if (!m_imageData.empty())
+            {
+                CreateImageModificationBuffer();
+            }
         }
     }
 

Gems/GradientSignal/Code/Source/Editor/EditorImageGradientComponent.cpp

@@ -243,7 +243,8 @@ namespace GradientSignal
         // Make sure our image asset settings are synced in the runtime component's configuration.
         RefreshImageAssetStatus();
 
-        EnableComponentMode();
+        // Register the component mode only if we're in an editable state.
+        RefreshComponentModeStatus();
     }
 
     void EditorImageGradientComponent::Deactivate()
@@ -372,7 +373,7 @@ namespace GradientSignal
 
     void EditorImageGradientComponent::EnableComponentMode()
     {
-        if (m_componentModeDelegate.AddedToComponentMode())
+        if (m_componentModeDelegate.IsConnected())
         {
             return;
         }
@@ -384,7 +385,7 @@ namespace GradientSignal
 
     void EditorImageGradientComponent::DisableComponentMode()
     {
-        if (!m_componentModeDelegate.AddedToComponentMode())
+        if (!m_componentModeDelegate.IsConnected())
         {
             return;
         }
@@ -411,6 +412,8 @@ namespace GradientSignal
             AzToolsFramework::ToolsApplicationEvents::Bus::Broadcast(
                 &AzToolsFramework::ToolsApplicationEvents::InvalidatePropertyDisplay, AzToolsFramework::Refresh_AttributesAndValues);
         }
+
+        RefreshComponentModeStatus();
     }
 
     AZ::u32 EditorImageGradientComponent::ConfigurationChanged()
@@ -446,6 +449,9 @@ namespace GradientSignal
 
     AZ::u32 EditorImageGradientComponent::RefreshCreationSelectionChoice()
     {
+        // Refresh the component mode status, since changing this dropdown affects the component mode visibility.
+        RefreshComponentModeStatus();
+
         // We need to refresh the entire tree because this selection changes the visibility of other properties.
         return AZ::Edit::PropertyRefreshLevels::EntireTree;
     }

Gems/GradientSignal/Code/Source/Editor/EditorImageGradientComponentMode.cpp

@@ -296,6 +296,7 @@ namespace GradientSignal
         const AZ::EntityComponentIdPair& entityComponentIdPair, AZ::Uuid componentType)
         : EditorBaseComponentMode(entityComponentIdPair, componentType)
         , m_ownerEntityComponentId(entityComponentIdPair)
+        , m_anyValuesChanged(false)
     {
         EditorImageGradientRequestBus::Event(GetEntityId(), &EditorImageGradientRequests::StartImageModification);
         ImageGradientModificationBus::Event(GetEntityId(), &ImageGradientModifications::StartImageModification);
@@ -326,17 +327,11 @@ namespace GradientSignal
 
         m_brushManipulator = AzToolsFramework::PaintBrushManipulator::MakeShared(
             worldFromLocal, entityComponentIdPair, AzToolsFramework::PaintBrushColorMode::Greyscale);
-        Refresh();
-
         m_brushManipulator->Register(AzToolsFramework::g_mainManipulatorManagerId);
-
-        CreateSubModeSelectionCluster();
     }
 
     EditorImageGradientComponentMode::~EditorImageGradientComponentMode()
     {
-        RemoveSubModeSelectionCluster();
-
         AzToolsFramework::PaintBrushNotificationBus::Handler::BusDisconnect();
         m_brushManipulator->Unregister();
         m_brushManipulator.reset();
@@ -345,7 +340,7 @@ namespace GradientSignal
 
         // It's possible that we're leaving component mode as the result of an "undo" action.
         // If that's the case, don't prompt the user to save the changes.
-        if (!AzToolsFramework::UndoRedoOperationInProgress())
+        if (!AzToolsFramework::UndoRedoOperationInProgress() && m_anyValuesChanged)
         {
             EditorImageGradientRequestBus::Event(GetEntityId(), &EditorImageGradientRequests::SaveImage);
         }
@@ -569,6 +564,9 @@ namespace GradientSignal
         // Notify anything listening to the image gradient that the modified region has changed.
         LmbrCentral::DependencyNotificationBus::Event(
             GetEntityId(), &LmbrCentral::DependencyNotificationBus::Events::OnCompositionRegionChanged, expandedDirtyArea);
+
+        // Track that we've changed image values, so we should prompt to save on exit.
+        m_anyValuesChanged = true;
     }
 
     void EditorImageGradientComponentMode::OnPaint(const AZ::Aabb& dirtyArea, ValueLookupFn& valueLookupFn, BlendFn& blendFn)
@@ -627,76 +625,4 @@ namespace GradientSignal
         OnPaintSmoothInternal(dirtyArea, valueLookupFn, combineFn);
     }
 
-    void EditorImageGradientComponentMode::CreateSubModeSelectionCluster()
-    {
-        auto RegisterClusterButton = [](AzToolsFramework::ViewportUi::ClusterId clusterId,
-                                        const char* iconName,
-                                        const char* tooltip) -> AzToolsFramework::ViewportUi::ButtonId
-        {
-            AzToolsFramework::ViewportUi::ButtonId buttonId;
-            AzToolsFramework::ViewportUi::ViewportUiRequestBus::EventResult(
-                buttonId,
-                AzToolsFramework::ViewportUi::DefaultViewportId,
-                &AzToolsFramework::ViewportUi::ViewportUiRequestBus::Events::CreateClusterButton,
-                clusterId,
-                AZStd::string::format(":/stylesheet/img/UI20/toolbar/%s.svg", iconName));
-
-            AzToolsFramework::ViewportUi::ViewportUiRequestBus::Event(
-                AzToolsFramework::ViewportUi::DefaultViewportId,
-                &AzToolsFramework::ViewportUi::ViewportUiRequestBus::Events::SetClusterButtonTooltip,
-                clusterId,
-                buttonId,
-                tooltip);
-
-            return buttonId;
-        };
-
-        // create the cluster for showing the Paint Brush Settings window
-        AzToolsFramework::ViewportUi::ViewportUiRequestBus::EventResult(
-            m_paintBrushControlClusterId,
-            AzToolsFramework::ViewportUi::DefaultViewportId,
-            &AzToolsFramework::ViewportUi::ViewportUiRequestBus::Events::CreateCluster,
-            AzToolsFramework::ViewportUi::Alignment::TopLeft);
-
-        // create and register the "Show Paint Brush Settings" button.
-        // This button is needed because the window is only shown while in component mode, and the window can be closed by the user,
-        // so we need to provide an alternate way for the user to re-open the window. 
-        m_paintModeButtonId = RegisterClusterButton(m_paintBrushControlClusterId, "Paint", "Switch to Paint Mode");
-        m_eyedropperModeButtonId = RegisterClusterButton(m_paintBrushControlClusterId, "Eyedropper", "Switch to Eyedropper Mode");
-        m_smoothModeButtonId = RegisterClusterButton(m_paintBrushControlClusterId, "Smooth", "Switch to Smooth Mode");
-
-        m_buttonSelectionHandler = AZ::Event<AzToolsFramework::ViewportUi::ButtonId>::Handler(
-            [this](AzToolsFramework::ViewportUi::ButtonId buttonId)
-            {
-                AzToolsFramework::PaintBrushMode brushMode = AzToolsFramework::PaintBrushMode::Paintbrush;
-
-                if (buttonId == m_eyedropperModeButtonId)
-                {
-                    brushMode = AzToolsFramework::PaintBrushMode::Eyedropper;
-                }
-                else if (buttonId == m_smoothModeButtonId)
-                {
-                    brushMode = AzToolsFramework::PaintBrushMode::Smooth;
-                }
-
-                AzToolsFramework::OpenViewPane(PaintBrush::s_paintBrushSettingsName);
-
-                AzToolsFramework::PaintBrushSettingsRequestBus::Broadcast(
-                    &AzToolsFramework::PaintBrushSettingsRequestBus::Events::SetBrushMode, brushMode);
-            });
-        AzToolsFramework::ViewportUi::ViewportUiRequestBus::Event(
-            AzToolsFramework::ViewportUi::DefaultViewportId,
-            &AzToolsFramework::ViewportUi::ViewportUiRequestBus::Events::RegisterClusterEventHandler,
-            m_paintBrushControlClusterId,
-            m_buttonSelectionHandler);
-    }
-
-    void EditorImageGradientComponentMode::RemoveSubModeSelectionCluster()
-    {
-        AzToolsFramework::ViewportUi::ViewportUiRequestBus::Event(
-            AzToolsFramework::ViewportUi::DefaultViewportId,
-            &AzToolsFramework::ViewportUi::ViewportUiRequestBus::Events::RemoveCluster,
-            m_paintBrushControlClusterId);
-    }
-
 } // namespace GradientSignal

Gems/GradientSignal/Code/Source/Editor/EditorImageGradientComponentMode.h

@@ -11,6 +11,7 @@
 #include <AzToolsFramework/ComponentMode/EditorBaseComponentMode.h>
 #include <AzToolsFramework/Manipulators/PaintBrushManipulator.h>
 #include <AzToolsFramework/PaintBrush/PaintBrushNotificationBus.h>
+#include <AzToolsFramework/PaintBrush/PaintBrushSubModeCluster.h>
 #include <AzToolsFramework/Undo/UndoSystem.h>
 #include <AzToolsFramework/ViewportUi/ViewportUiRequestBus.h>
 
@@ -52,9 +53,6 @@ namespace GradientSignal
         void BeginUndoBatch();
         void EndUndoBatch();
 
-        void CreateSubModeSelectionCluster();
-        void RemoveSubModeSelectionCluster();
-
     private:
         struct PaintStrokeData
         {
@@ -84,17 +82,17 @@ namespace GradientSignal
         //! The entity/component that owns this paintbrush.
         AZ::EntityComponentIdPair m_ownerEntityComponentId;
 
+        //! The core paintbrush manipulator and painting logic.
         AZStd::shared_ptr<AzToolsFramework::PaintBrushManipulator> m_brushManipulator;
 
         //! The undo information for the in-progress painting brush stroke.
         AzToolsFramework::UndoSystem::URSequencePoint* m_undoBatch = nullptr;
         PaintBrushUndoBuffer* m_paintBrushUndoBuffer = nullptr;
 
-        AzToolsFramework::ViewportUi::ClusterId m_paintBrushControlClusterId;
-        AzToolsFramework::ViewportUi::ButtonId m_paintModeButtonId;
-        AzToolsFramework::ViewportUi::ButtonId m_eyedropperModeButtonId;
-        AzToolsFramework::ViewportUi::ButtonId m_smoothModeButtonId;
+        //! Track whether or not anything has changed while editing. If not, then don't prompt to save the image at the end.
+        bool m_anyValuesChanged = false;
 
-        AZ::Event<AzToolsFramework::ViewportUi::ButtonId>::Handler m_buttonSelectionHandler;
+        //! The paint brush cluster that manages switching between paint/smooth/eyedropper modes
+        AzToolsFramework::PaintBrushSubModeCluster m_subModeCluster;
     };
 } // namespace GradientSignal