birkeh/o3de
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Change editor viewport mdoe tracker id from viewport to entity context.

Browse Source 
Signed-off-by: John <jonawals@amazon.com>
This commit is contained in:
John
2021-10-01 10:36:08 +01:00
parent cfbe76fdf6
commit 10c7ef7147
7 changed files with 77 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Code/Framework/AzToolsFramework/AzToolsFramework/API/ViewportEditorModeTrackerNotificationBus.h
+4 -4
View File
@@ -9,7 +9,7 @@
#pragma once
#include <AzCore/EBus/Event.h>
#include <AzFramework/Viewport/ViewportId.h>
#include <AzFramework/Entity/EntityContextBus.h>
#include <AzToolsFramework/ViewportUi/ViewportUiRequestBus.h>
namespace AzToolsFramework
@@ -23,11 +23,11 @@ namespace AzToolsFramework
Pick
};
//! Viewport identifier and other relevant viewport data.
//! Viewport editor mode tracker identifier and other relevant data.
struct ViewportEditorModeInfo
{
using IdType = AzFramework::ViewportId;
IdType m_id = ViewportUi::DefaultViewportId; //!< The unique identifier for a given viewport.
using IdType = AzFramework::EntityContextId;
IdType m_id = AzFramework::EntityContextId::CreateNull(); //!< The unique identifier for a given viewport editor mode tracker.
};
//! Interface for the editor modes of a given viewport.
Code/Framework/AzToolsFramework/AzToolsFramework/ComponentMode/ComponentModeCollection.cpp
+2 -2
View File
@@ -218,7 +218,7 @@ namespace AzToolsFramework
// this call to activate the component mode editor state should eventually replace the bus call in
// ComponentModeCollection::BeginComponentMode() to EditorComponentModeNotifications::EnteredComponentMode
// such that all of the notifications for activating/deactivating the different editor modes are in a central location
m_viewportEditorModeTracker->ActivateMode({ /* DefaultViewportId */ }, ViewportEditorMode::Component);
m_viewportEditorModeTracker->ActivateMode({ GetEntityContextId() }, ViewportEditorMode::Component);
// enable actions for the first/primary ComponentMode
// note: if multiple ComponentModes are activated at the same time, actions
@@ -296,7 +296,7 @@ namespace AzToolsFramework
// this call to deactivate the component mode editor state should eventually replace the bus call in
// ComponentModeCollection::EndComponentMode() to EditorComponentModeNotifications::LeftComponentMode
// such that all of the notifications for activating/deactivating the different editor modes are in a central location
m_viewportEditorModeTracker->DeactivateMode({ /* DefaultViewportId */ }, ViewportEditorMode::Component);
m_viewportEditorModeTracker->DeactivateMode({ GetEntityContextId() }, ViewportEditorMode::Component);
// clear stored modes and builders for this ComponentMode
// TLDR: avoid 'use after free' error
Code/Framework/AzToolsFramework/AzToolsFramework/FocusMode/FocusModeSystemComponent.cpp
+3 -3
View File
@@ -10,7 +10,7 @@
#include <AzToolsFramework/API/ViewportEditorModeTrackerInterface.h>
#include <AzToolsFramework/FocusMode/FocusModeSystemComponent.h>
#include <AzToolsFramework/API/ViewportEditorModeTrackerInterface.h>
#include <AzToolsFramework/Viewport/ViewportMessages.h>
namespace AzToolsFramework
{
@@ -72,11 +72,11 @@ namespace AzToolsFramework
{
if (!m_focusRoot.IsValid() && entityId.IsValid())
{
tracker->ActivateMode({ /* DefaultViewportId */ }, ViewportEditorMode::Focus);
tracker->ActivateMode({ GetEntityContextId() }, ViewportEditorMode::Focus);
}
else if (m_focusRoot.IsValid() && !entityId.IsValid())
{
tracker->DeactivateMode({ /* DefaultViewportId */ }, ViewportEditorMode::Focus);
tracker->DeactivateMode({ GetEntityContextId() }, ViewportEditorMode::Focus);
}
}
}
Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorDefaultSelection.cpp
+2 -2
View File
@@ -32,14 +32,14 @@ namespace AzToolsFramework
m_manipulatorManager = AZStd::make_shared<AzToolsFramework::ManipulatorManager>(AzToolsFramework::g_mainManipulatorManagerId);
m_transformComponentSelection = AZStd::make_unique<EditorTransformComponentSelection>(entityDataCache);
m_viewportEditorModeTracker->ActivateMode({ /* DefaultViewportId */ }, ViewportEditorMode::Default);
m_viewportEditorModeTracker->ActivateMode({ GetEntityContextId() }, ViewportEditorMode::Default);
}
EditorDefaultSelection::~EditorDefaultSelection()
{
ComponentModeFramework::ComponentModeSystemRequestBus::Handler::BusDisconnect();
ActionOverrideRequestBus::Handler::BusDisconnect();
m_viewportEditorModeTracker->DeactivateMode({ /* DefaultViewportId */ }, ViewportEditorMode::Default);
m_viewportEditorModeTracker->DeactivateMode({ GetEntityContextId() }, ViewportEditorMode::Default);
}
void EditorDefaultSelection::SetOverridePhantomWidget(QWidget* phantomOverrideWidget)
Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorPickEntitySelection.cpp
+2 -2
View File
@@ -21,7 +21,7 @@ namespace AzToolsFramework
: m_editorHelpers(AZStd::make_unique<EditorHelpers>(entityDataCache))
, m_viewportEditorModeTracker(viewportEditorModeTracker)
{
m_viewportEditorModeTracker->ActivateMode({ /* DefaultViewportId */ }, ViewportEditorMode::Pick);
m_viewportEditorModeTracker->ActivateMode({ GetEntityContextId() }, ViewportEditorMode::Pick);
}
EditorPickEntitySelection::~EditorPickEntitySelection()
@@ -31,7 +31,7 @@ namespace AzToolsFramework
ToolsApplicationRequestBus::Broadcast(&ToolsApplicationRequests::SetEntityHighlighted, m_hoveredEntityId, false);
}
m_viewportEditorModeTracker->DeactivateMode({ /* DefaultViewportId */ }, ViewportEditorMode::Pick);
m_viewportEditorModeTracker->DeactivateMode({ GetEntityContextId() }, ViewportEditorMode::Pick);
}
// note: entityIdUnderCursor is the authoritative entityId we get each frame by querying
Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/ViewportEditorModeTracker.cpp
+6 -4
View File
@@ -52,7 +52,8 @@ namespace AzToolsFramework
if (editorModes.IsModeActive(mode))
{
return AZ::Failure(AZStd::string::format(
"Duplicate call to ActivateMode for mode '%u' on id '%i'", static_cast<AZ::u32>(mode), viewportEditorModeInfo.m_id));
"Duplicate call to ActivateMode for mode '%u' on id '%s'", static_cast<AZ::u32>(mode),
viewportEditorModeInfo.m_id.ToString<AZStd::string>().c_str()));
}
if (const auto result = editorModes.ActivateMode(mode);
@@ -78,7 +79,8 @@ namespace AzToolsFramework
if (!editorModes->IsModeActive(mode))
{
return AZ::Failure(AZStd::string::format(
"Duplicate call to DeactivateMode for mode '%u' on id '%i'", static_cast<AZ::u32>(mode), viewportEditorModeInfo.m_id));
"Duplicate call to DeactivateMode for mode '%u' on id '%s'", static_cast<AZ::u32>(mode),
viewportEditorModeInfo.m_id.ToString<AZStd::string>().c_str()));
}
}
else
@@ -103,8 +105,8 @@ namespace AzToolsFramework
else
{
return AZ::Failure(AZStd::string::format(
"Call to DeactivateMode for mode '%u' on id '%i' without precursor call to ActivateMode", static_cast<AZ::u32>(mode),
viewportEditorModeInfo.m_id));
"Call to DeactivateMode for mode '%u' on id '%s' without precursor call to ActivateMode", static_cast<AZ::u32>(mode),
viewportEditorModeInfo.m_id.ToString<AZStd::string>().c_str()));
}
}
Code/Framework/AzToolsFramework/Tests/Viewport/ViewportEditorModeTests.cpp
+58 -54
View File
@@ -9,6 +9,7 @@
#include <AzTest/AzTest.h>
#include <AzToolsFramework/ComponentMode/EditorComponentModeBus.h>
#include <AzToolsFramework/UnitTest/AzToolsFrameworkTestHelpers.h>
#include <AzToolsFramework/Viewport/ViewportMessages.h>
#include <AzToolsFramework/ViewportSelection/EditorPickEntitySelection.h>
#include <AzToolsFramework/ViewportSelection/ViewportEditorModeTracker.h>
@@ -18,7 +19,7 @@ namespace UnitTest
using ViewportEditorModes = AzToolsFramework::ViewportEditorModes;
using ViewportEditorModeTracker = AzToolsFramework::ViewportEditorModeTracker;
using ViewportEditorModeInfo = AzToolsFramework::ViewportEditorModeInfo;
using ViewportId = ViewportEditorModeInfo::IdType;
using TrackerId = ViewportEditorModeInfo::IdType;
using ViewportEditorModesInterface = AzToolsFramework::ViewportEditorModesInterface;
using ViewportEditorModeTrackerInterface = AzToolsFramework::ViewportEditorModeTrackerInterface;
@@ -113,10 +114,10 @@ namespace UnitTest
using EditModeTracker = AZStd::unordered_map<ViewportEditorMode, ReceivedEvents>;
ViewportEditorModeNotificationsBusHandler(ViewportId viewportId)
: m_viewportSubscription(viewportId)
ViewportEditorModeNotificationsBusHandler(TrackerId id)
: m_trackerSubscription(id)
{
AzToolsFramework::ViewportEditorModeNotificationsBus::Handler::BusConnect(m_viewportSubscription);
AzToolsFramework::ViewportEditorModeNotificationsBus::Handler::BusConnect(m_trackerSubscription);
}
~ViewportEditorModeNotificationsBusHandler()
@@ -124,11 +125,6 @@ namespace UnitTest
AzToolsFramework::ViewportEditorModeNotificationsBus::Handler::BusDisconnect();
}
ViewportId GetViewportSubscription() const
{
return m_viewportSubscription;
}
const EditModeTracker& GetEditorModes() const
{
return m_editorModes;
@@ -145,7 +141,7 @@ namespace UnitTest
}
private:
ViewportId m_viewportSubscription;
TrackerId m_trackerSubscription;
EditModeTracker m_editorModes;
};
@@ -158,10 +154,13 @@ namespace UnitTest
void SetUpEditorFixtureImpl() override
{
m_handlerIds.resize(ViewportEditorModes::NumEditorModes);
for (auto mode = 0; mode < ViewportEditorModes::NumEditorModes; mode++)
{
m_editorModeHandlers[mode] = AZStd::make_unique<ViewportEditorModeNotificationsBusHandler>(mode);
m_handlerIds[mode] = TrackerId::CreateRandom();
m_editorModeHandlers[mode] = AZStd::make_unique<ViewportEditorModeNotificationsBusHandler>(m_handlerIds[mode]);
}
}
void TearDownEditorFixtureImpl() override
@@ -173,6 +172,7 @@ namespace UnitTest
}
AZStd::array<AZStd::unique_ptr<ViewportEditorModeNotificationsBusHandler>, ViewportEditorModes::NumEditorModes> m_editorModeHandlers;
AZStd::vector<TrackerId> m_handlerIds;
};
// Fixture for testing the integration of viewport editor mode state tracker
@@ -184,7 +184,8 @@ namespace UnitTest
{
m_viewportEditorModeTracker = AZ::Interface<ViewportEditorModeTrackerInterface>::Get();
ASSERT_NE(m_viewportEditorModeTracker, nullptr);
m_viewportEditorModes = m_viewportEditorModeTracker->GetViewportEditorModes({});
m_viewportEditorModes = m_viewportEditorModeTracker->GetViewportEditorModes({AzToolsFramework::GetEntityContextId()});
ASSERT_NE(m_viewportEditorModes, nullptr);
}
ViewportEditorModeTrackerInterface* m_viewportEditorModeTracker = nullptr;
@@ -316,17 +317,17 @@ namespace UnitTest
TEST_F(ViewportEditorModeTrackerTestFixture, ActivatingViewportEditorModeForNonExistentIdCreatesViewportEditorModesForThatId)
{
// Given a viewport not currently being tracked
const ViewportId viewportid = 0;
EXPECT_FALSE(m_viewportEditorModeTracker.IsViewportModeTracked({ viewportid }));
EXPECT_EQ(m_viewportEditorModeTracker.GetViewportEditorModes({ viewportid }), nullptr);
const TrackerId id = 0;
EXPECT_FALSE(m_viewportEditorModeTracker.IsViewportModeTracked({ id }));
EXPECT_EQ(m_viewportEditorModeTracker.GetViewportEditorModes({ id }), nullptr);
// When a mode is activated for that viewport
const auto editorMode = ViewportEditorMode::Default;
m_viewportEditorModeTracker.ActivateMode({ viewportid }, editorMode);
const auto* viewportEditorModeState = m_viewportEditorModeTracker.GetViewportEditorModes({ viewportid });
m_viewportEditorModeTracker.ActivateMode({ id }, editorMode);
const auto* viewportEditorModeState = m_viewportEditorModeTracker.GetViewportEditorModes({ id });
// Expect that viewport to now be tracked
EXPECT_TRUE(m_viewportEditorModeTracker.IsViewportModeTracked({ viewportid }));
EXPECT_TRUE(m_viewportEditorModeTracker.IsViewportModeTracked({ id }));
EXPECT_NE(viewportEditorModeState, nullptr);
// Expect the mode for that viewport to be active
@@ -336,23 +337,24 @@ namespace UnitTest
TEST_F(ViewportEditorModeTrackerTestFixture, DeactivatingViewportEditorModeForNonExistentIdCreatesViewportEditorModesForThatIdButReturnsError)
{
// Given a viewport not currently being tracked
const ViewportId viewportid = 0;
EXPECT_FALSE(m_viewportEditorModeTracker.IsViewportModeTracked({ viewportid }));
EXPECT_EQ(m_viewportEditorModeTracker.GetViewportEditorModes({ viewportid }), nullptr);
const TrackerId id = 0;
EXPECT_FALSE(m_viewportEditorModeTracker.IsViewportModeTracked({ id }));
EXPECT_EQ(m_viewportEditorModeTracker.GetViewportEditorModes({ id }), nullptr);
// When a mode is deactivated for that viewport
const auto editorMode = ViewportEditorMode::Default;
const auto expectedErrorMsg = AZStd::string::format(
"Call to DeactivateMode for mode '%u' on id '%i' without precursor call to ActivateMode", static_cast<AZ::u32>(editorMode), viewportid);
const auto result = m_viewportEditorModeTracker.DeactivateMode({ viewportid }, editorMode);
"Call to DeactivateMode for mode '%u' on id '%s' without precursor call to ActivateMode", static_cast<AZ::u32>(editorMode),
id.ToString<AZStd::string>().c_str());
const auto result = m_viewportEditorModeTracker.DeactivateMode({ id }, editorMode);
// Expect an error due to no precursor activation of that mode
EXPECT_FALSE(result.IsSuccess());
EXPECT_EQ(result.GetError(), expectedErrorMsg);
// Expect that viewport to now be tracked
const auto* viewportEditorModeState = m_viewportEditorModeTracker.GetViewportEditorModes({ viewportid });
EXPECT_TRUE(m_viewportEditorModeTracker.IsViewportModeTracked({ viewportid }));
const auto* viewportEditorModeState = m_viewportEditorModeTracker.GetViewportEditorModes({ id });
EXPECT_TRUE(m_viewportEditorModeTracker.IsViewportModeTracked({ id }));
// Expect the mode for that viewport to be inactive
EXPECT_NE(viewportEditorModeState, nullptr);
@@ -361,45 +363,46 @@ namespace UnitTest
TEST_F(ViewportEditorModeTrackerTestFixture, GettingNonExistentViewportEditorModesForIdReturnsNull)
{
const ViewportId viewportid = 0;
EXPECT_FALSE(m_viewportEditorModeTracker.IsViewportModeTracked({ viewportid }));
EXPECT_EQ(m_viewportEditorModeTracker.GetViewportEditorModes({ viewportid }), nullptr);
const TrackerId id = 0;
EXPECT_FALSE(m_viewportEditorModeTracker.IsViewportModeTracked({ id }));
EXPECT_EQ(m_viewportEditorModeTracker.GetViewportEditorModes({ id }), nullptr);
}
TEST_F(ViewportEditorModeTrackerTestFixture, ActivatingViewportEditorModesForExistingIdInThatStateReturnsError)
{
// Given a viewport not currently tracked
const ViewportId viewportid = 0;
EXPECT_FALSE(m_viewportEditorModeTracker.IsViewportModeTracked({ viewportid }));
EXPECT_EQ(m_viewportEditorModeTracker.GetViewportEditorModes({ viewportid }), nullptr);
const TrackerId id = 0;
EXPECT_FALSE(m_viewportEditorModeTracker.IsViewportModeTracked({ id }));
EXPECT_EQ(m_viewportEditorModeTracker.GetViewportEditorModes({ id }), nullptr);
const auto editorMode = ViewportEditorMode::Default;
{
// When the mode is activated for the viewport
const auto result = m_viewportEditorModeTracker.ActivateMode({ viewportid }, editorMode);
const auto result = m_viewportEditorModeTracker.ActivateMode({ id }, editorMode);
// Expect no error as there is no duplicate activation
EXPECT_TRUE(result.IsSuccess());
// Expect the mode to be active for the viewport
const auto* viewportEditorModeState = m_viewportEditorModeTracker.GetViewportEditorModes({ viewportid });
EXPECT_TRUE(m_viewportEditorModeTracker.IsViewportModeTracked({ viewportid }));
const auto* viewportEditorModeState = m_viewportEditorModeTracker.GetViewportEditorModes({ id });
EXPECT_TRUE(m_viewportEditorModeTracker.IsViewportModeTracked({ id }));
EXPECT_NE(viewportEditorModeState, nullptr);
EXPECT_TRUE(viewportEditorModeState->IsModeActive(editorMode));
}
{
// When the mode is activated again for the viewport
const auto result = m_viewportEditorModeTracker.ActivateMode({ viewportid }, editorMode);
const auto result = m_viewportEditorModeTracker.ActivateMode({ id }, editorMode);
// Expect an error for the duplicate activation
const auto expectedErrorMsg = AZStd::string::format(
"Duplicate call to ActivateMode for mode '%u' on id '%i'", static_cast<AZ::u32>(editorMode), viewportid);
"Duplicate call to ActivateMode for mode '%u' on id '%s'", static_cast<AZ::u32>(editorMode),
id.ToString<AZStd::string>().c_str());
EXPECT_FALSE(result.IsSuccess());
EXPECT_EQ(result.GetError(), expectedErrorMsg);
// Expect the mode to still be active for the viewport
const auto* viewportEditorModeState = m_viewportEditorModeTracker.GetViewportEditorModes({ viewportid });
EXPECT_TRUE(m_viewportEditorModeTracker.IsViewportModeTracked({ viewportid }));
const auto* viewportEditorModeState = m_viewportEditorModeTracker.GetViewportEditorModes({ id });
EXPECT_TRUE(m_viewportEditorModeTracker.IsViewportModeTracked({ id }));
EXPECT_NE(viewportEditorModeState, nullptr);
EXPECT_TRUE(viewportEditorModeState->IsModeActive(editorMode));
}
@@ -408,38 +411,39 @@ namespace UnitTest
TEST_F(ViewportEditorModeTrackerTestFixture, DeactivatingViewportEditorModesForExistingIdNotInThatStateReturnssError)
{
// Given a viewport not currently tracked
const ViewportId viewportid = 0;
EXPECT_FALSE(m_viewportEditorModeTracker.IsViewportModeTracked({ viewportid }));
EXPECT_EQ(m_viewportEditorModeTracker.GetViewportEditorModes({ viewportid }), nullptr);
const TrackerId id = 0;
EXPECT_FALSE(m_viewportEditorModeTracker.IsViewportModeTracked({ id }));
EXPECT_EQ(m_viewportEditorModeTracker.GetViewportEditorModes({ id }), nullptr);
const auto editorMode = ViewportEditorMode::Default;
{
// When the mode is activated and then deactivated for the viewport
m_viewportEditorModeTracker.ActivateMode({ viewportid }, editorMode);
const auto result = m_viewportEditorModeTracker.DeactivateMode({ viewportid }, editorMode);
m_viewportEditorModeTracker.ActivateMode({ id }, editorMode);
const auto result = m_viewportEditorModeTracker.DeactivateMode({ id }, editorMode);
// Expect no error as there is no duplicate deactivation
EXPECT_TRUE(result.IsSuccess());
// Expect the mode to be inctive for the viewport
const auto* viewportEditorModeState = m_viewportEditorModeTracker.GetViewportEditorModes({ viewportid });
EXPECT_TRUE(m_viewportEditorModeTracker.IsViewportModeTracked({ viewportid }));
const auto* viewportEditorModeState = m_viewportEditorModeTracker.GetViewportEditorModes({ id });
EXPECT_TRUE(m_viewportEditorModeTracker.IsViewportModeTracked({ id }));
EXPECT_NE(viewportEditorModeState, nullptr);
EXPECT_FALSE(viewportEditorModeState->IsModeActive(editorMode));
}
{
// When the mode is deactivated again for the viewport
const auto result = m_viewportEditorModeTracker.DeactivateMode({ viewportid }, editorMode);
const auto result = m_viewportEditorModeTracker.DeactivateMode({ id }, editorMode);
// Expect an error for the duplicate deactivation
const auto expectedErrorMsg = AZStd::string::format(
"Duplicate call to DeactivateMode for mode '%u' on id '%i'", static_cast<AZ::u32>(editorMode), viewportid);
"Duplicate call to DeactivateMode for mode '%u' on id '%s'", static_cast<AZ::u32>(editorMode),
id.ToString<AZStd::string>().c_str());
EXPECT_FALSE(result.IsSuccess());
EXPECT_EQ(result.GetError(), expectedErrorMsg);
// Expect the mode to still be inactive for the viewport
const auto* viewportEditorModeState = m_viewportEditorModeTracker.GetViewportEditorModes({ viewportid });
EXPECT_TRUE(m_viewportEditorModeTracker.IsViewportModeTracked({ viewportid }));
const auto* viewportEditorModeState = m_viewportEditorModeTracker.GetViewportEditorModes({ id });
EXPECT_TRUE(m_viewportEditorModeTracker.IsViewportModeTracked({ id }));
EXPECT_NE(viewportEditorModeState, nullptr);
EXPECT_FALSE(viewportEditorModeState->IsModeActive(editorMode));
}
@@ -459,9 +463,9 @@ namespace UnitTest
// When each editor mode is activated by the state tracker for a specific viewport
for (auto mode = 0; mode < ViewportEditorModes::NumEditorModes; mode++)
{
const ViewportId viewportId = mode;
const TrackerId id = m_handlerIds[mode];
const ViewportEditorMode editorMode = static_cast<ViewportEditorMode>(mode);
m_viewportEditorModeTracker.ActivateMode({ viewportId }, editorMode);
m_viewportEditorModeTracker.ActivateMode({ id }, editorMode);
}
for (auto mode = 0; mode < ViewportEditorModes::NumEditorModes; mode++)
@@ -491,10 +495,10 @@ namespace UnitTest
// When each editor mode is activated deactivated by the state tracker for a specific viewport
for (auto mode = 0; mode < ViewportEditorModes::NumEditorModes; mode++)
{
const ViewportId viewportId = mode;
const TrackerId id = m_handlerIds[mode];
const ViewportEditorMode editorMode = static_cast<ViewportEditorMode>(mode);
m_viewportEditorModeTracker.ActivateMode({ viewportId }, editorMode);
m_viewportEditorModeTracker.DeactivateMode({ viewportId }, editorMode);
m_viewportEditorModeTracker.ActivateMode({ id }, editorMode);
m_viewportEditorModeTracker.DeactivateMode({ id }, editorMode);
}
for (auto mode = 0; mode < ViewportEditorModes::NumEditorModes; mode++)