From 619f31572e5b70dce5eea6abf7dfed5c19d34699 Mon Sep 17 00:00:00 2001 From: srikappa-amzn Date: Wed, 15 Sep 2021 13:26:55 -0700 Subject: [PATCH 01/20] Enable destroying game entities spawned with a spawnable Signed-off-by: srikappa-amzn --- .../AzFramework/Entity/GameEntityContextBus.h | 18 ----- .../Entity/GameEntityContextComponent.cpp | 50 ++++---------- .../Entity/GameEntityContextComponent.h | 5 -- .../Spawnable/SpawnableEntitiesInterface.h | 8 +++ .../Spawnable/SpawnableEntitiesManager.cpp | 67 +++++++++++++++---- .../Spawnable/SpawnableEntitiesManager.h | 14 +++- .../Spawnable/SpawnableSystemComponent.h | 2 + .../Spawnable/SpawnedEntityTicketMapper.cpp | 43 ++++++++++++ .../Spawnable/SpawnedEntityTicketMapper.h | 37 ++++++++++ .../SpawnedEntityTicketMapperInterface.h | 24 +++++++ .../AzFramework/azframework_files.cmake | 3 + 11 files changed, 194 insertions(+), 77 deletions(-) create mode 100644 Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.cpp create mode 100644 Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.h create mode 100644 Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapperInterface.h diff --git a/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextBus.h b/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextBus.h index b5ca37df6e..f8e4dfa98b 100644 --- a/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextBus.h +++ b/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextBus.h @@ -91,15 +91,6 @@ namespace AzFramework */ virtual void DestroyGameEntity(const AZ::EntityId& /*id*/) = 0; - /** - * Destroys an entity only in slice mode (when prefabs are disabled). This request is only added as a stop-gap solution - * to prevent the editor from crashing when prefabs are enabled and must only be called through the BehaviorContext binding - * for 'DestroyGameEntity'. No code should be written to directly call this method. This will be removed soon. - * - * @param id The ID of the entity to destroy. - */ - virtual void DestroyGameEntityOnlyInSliceMode(const AZ::EntityId& /*id*/) = 0; - /** * Destroys an entity and all of its descendants. * The entity and its descendants are immediately deactivated and will be @@ -108,15 +99,6 @@ namespace AzFramework */ virtual void DestroyGameEntityAndDescendants(const AZ::EntityId& /*id*/) = 0; - /** - * Destroys an entity and its descendants only in slice mode (when prefabs are disabled). This request is only added as a stop-gap - * solution to prevent the editor from crashing when prefabs are enabled and must only be called through the BehaviorContext - * binding for 'DestroyGameEntityAndDescendants'.No code should be written to directly call this method. This will be removed soon. - * - * @param id The ID of the entity to destroy. - */ - virtual void DestroyGameEntityAndDescendantsOnlyInSliceMode(const AZ::EntityId& /*id*/) = 0; - /** * Activates the game entity. * @param id The ID of the entity to activate. diff --git a/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp b/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp index a97281b9c1..903bad91bf 100644 --- a/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp +++ b/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp @@ -11,9 +11,10 @@ #include #include #include +#include #include #include -#include +#include #include "GameEntityContextComponent.h" @@ -47,9 +48,9 @@ namespace AzFramework ->Attribute(AZ::Script::Attributes::Scope, AZ::Script::Attributes::ScopeFlags::Common) ->Event("CreateGameEntity", &GameEntityContextRequestBus::Events::CreateGameEntityForBehaviorContext) ->Attribute(AZ::Script::Attributes::ExcludeFrom, AZ::Script::Attributes::ExcludeFlags::All) - ->Event("DestroyGameEntity", &GameEntityContextRequestBus::Events::DestroyGameEntityOnlyInSliceMode) + ->Event("DestroyGameEntity", &GameEntityContextRequestBus::Events::DestroyGameEntity) ->Event( - "DestroyGameEntityAndDescendants", &GameEntityContextRequestBus::Events::DestroyGameEntityAndDescendantsOnlyInSliceMode) + "DestroyGameEntityAndDescendants", &GameEntityContextRequestBus::Events::DestroyGameEntityAndDescendants) ->Event("ActivateGameEntity", &GameEntityContextRequestBus::Events::ActivateGameEntity) ->Event("DeactivateGameEntity", &GameEntityContextRequestBus::Events::DeactivateGameEntity) ->Attribute(AZ::ScriptCanvasAttributes::DeactivatesInputEntity, true) @@ -249,23 +250,6 @@ namespace AzFramework DestroyGameEntityInternal(id, false); } - void GameEntityContextComponent::DestroyGameEntityOnlyInSliceMode(const AZ::EntityId& id) - { - bool isPrefabSystemEnabled = false; - AzFramework::ApplicationRequests::Bus::BroadcastResult( - isPrefabSystemEnabled, &AzFramework::ApplicationRequests::IsPrefabSystemEnabled); - if (!isPrefabSystemEnabled) - { - DestroyGameEntityInternal(id, false); - } - else - { - AZ_Error( - "GameEntityContextComponent", false, - "Destroying a game entity is temporarily disabled until the Spawnable system can support this."); - } - } - //========================================================================= // GameEntityContextComponent::DestroyGameEntityAndDescendantsById //========================================================================= @@ -274,24 +258,6 @@ namespace AzFramework DestroyGameEntityInternal(id, true); } - - void GameEntityContextComponent::DestroyGameEntityAndDescendantsOnlyInSliceMode(const AZ::EntityId& id) - { - bool isPrefabSystemEnabled = false; - AzFramework::ApplicationRequests::Bus::BroadcastResult( - isPrefabSystemEnabled, &AzFramework::ApplicationRequests::IsPrefabSystemEnabled); - if (!isPrefabSystemEnabled) - { - DestroyGameEntityInternal(id, true); - } - else - { - AZ_Error( - "GameEntityContextComponent", false, - "Destroying a game entity and its descendants is temporarily disabled until the Spawnable system can support this."); - } - } - //========================================================================= // GameEntityContextComponent::DestroyGameEntityInternal //========================================================================= @@ -319,6 +285,14 @@ namespace AzFramework EBUS_EVENT_RESULT(currentEntity, AZ::ComponentApplicationBus, FindEntity, *entityIdIter); if (currentEntity) { + bool isPrefabSystemEnabled = false; + AzFramework::ApplicationRequests::Bus::BroadcastResult( + isPrefabSystemEnabled, &AzFramework::ApplicationRequests::IsPrefabSystemEnabled); + if (isPrefabSystemEnabled) + { + AZ::Interface::Get()->RemoveSpawnedEntity(currentEntity->GetId()); + } + if (currentEntity->GetState() == AZ::Entity::State::Active) { // Deactivate the entity, we'll destroy it as soon as it is safe. diff --git a/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.h b/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.h index 831067d728..56a53f3c50 100644 --- a/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.h +++ b/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.h @@ -90,11 +90,6 @@ namespace AzFramework } private: - ////////////////////////////////////////////////////////////////////////// - // GameEntityContextRequestBus - void DestroyGameEntityOnlyInSliceMode(const AZ::EntityId&) override; - void DestroyGameEntityAndDescendantsOnlyInSliceMode(const AZ::EntityId&) override; - ///////////////////////////////////////////////////////////////////////// AzFramework::EntityVisibilityBoundsUnionSystem m_entityVisibilityBoundsUnionSystem; }; diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h index 4b09dcbc75..2ce4158229 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h @@ -235,6 +235,12 @@ namespace AzFramework SpawnablePriority m_priority{ SpawnablePriority_Default }; }; + struct ClaimEntityOptionalArgs final + { + //! The priority at which this call will be executed. + SpawnablePriority m_priority{ SpawnablePriority_Default }; + }; + struct BarrierOptionalArgs final { //! The priority at which this call will be executed. @@ -314,6 +320,8 @@ namespace AzFramework virtual void ClaimEntities( EntitySpawnTicket& ticket, ClaimEntitiesCallback listCallback, ClaimEntitiesOptionalArgs optionalArgs = {}) = 0; + virtual void ClaimEntity(AZ::EntityId entityId, void* ticket, ClaimEntityOptionalArgs optionalArgs = {}) = 0; + //! Blocks until all operations made on the provided ticket before the barrier call have completed. //! @param ticket The ticket to monitor. //! @param completionCallback Required callback that will be called as soon as the barrier has been reached. diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp index 5fa072a451..eae71b5ea0 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp @@ -17,17 +17,18 @@ #include #include #include +#include namespace AzFramework { template - void SpawnableEntitiesManager::QueueRequest(EntitySpawnTicket& ticket, SpawnablePriority priority, T&& request) + void SpawnableEntitiesManager::QueueRequest(Ticket* ticket, SpawnablePriority priority, T&& request) { - request.m_ticket = &GetTicketPayload(ticket); + request.m_ticket = ticket; Queue& queue = priority <= m_highPriorityThreshold ? m_highPriorityQueue : m_regularPriorityQueue; { AZStd::scoped_lock queueLock(queue.m_pendingRequestMutex); - request.m_requestId = GetTicketPayload(ticket).m_nextRequestId++; + request.m_requestId = ticket->m_nextRequestId++; queue.m_pendingRequest.push(AZStd::move(request)); } } @@ -56,7 +57,7 @@ namespace AzFramework optionalArgs.m_serializeContext == nullptr ? m_defaultSerializeContext : optionalArgs.m_serializeContext; queueEntry.m_completionCallback = AZStd::move(optionalArgs.m_completionCallback); queueEntry.m_preInsertionCallback = AZStd::move(optionalArgs.m_preInsertionCallback); - QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); } void SpawnableEntitiesManager::SpawnEntities( @@ -72,7 +73,7 @@ namespace AzFramework queueEntry.m_completionCallback = AZStd::move(optionalArgs.m_completionCallback); queueEntry.m_preInsertionCallback = AZStd::move(optionalArgs.m_preInsertionCallback); queueEntry.m_referencePreviouslySpawnedEntities = optionalArgs.m_referencePreviouslySpawnedEntities; - QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); } void SpawnableEntitiesManager::DespawnAllEntities(EntitySpawnTicket& ticket, DespawnAllEntitiesOptionalArgs optionalArgs) @@ -82,7 +83,7 @@ namespace AzFramework DespawnAllEntitiesCommand queueEntry; queueEntry.m_ticketId = ticket.GetId(); queueEntry.m_completionCallback = AZStd::move(optionalArgs.m_completionCallback); - QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); } void SpawnableEntitiesManager::ReloadSpawnable( @@ -96,7 +97,7 @@ namespace AzFramework queueEntry.m_serializeContext = optionalArgs.m_serializeContext == nullptr ? m_defaultSerializeContext : optionalArgs.m_serializeContext; queueEntry.m_completionCallback = AZStd::move(optionalArgs.m_completionCallback); - QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); } void SpawnableEntitiesManager::ListEntities( @@ -108,7 +109,7 @@ namespace AzFramework ListEntitiesCommand queueEntry; queueEntry.m_ticketId = ticket.GetId(); queueEntry.m_listCallback = AZStd::move(listCallback); - QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); } void SpawnableEntitiesManager::ListIndicesAndEntities( @@ -120,7 +121,7 @@ namespace AzFramework ListIndicesEntitiesCommand queueEntry; queueEntry.m_ticketId = ticket.GetId(); queueEntry.m_listCallback = AZStd::move(listCallback); - QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); } void SpawnableEntitiesManager::ClaimEntities( @@ -132,7 +133,18 @@ namespace AzFramework ClaimEntitiesCommand queueEntry; queueEntry.m_ticketId = ticket.GetId(); queueEntry.m_listCallback = AZStd::move(listCallback); - QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); + } + + void SpawnableEntitiesManager::ClaimEntity( + AZ::EntityId entityId, void* ticket, + ClaimEntityOptionalArgs optionalArgs) + { + AZ_Assert(ticket == nullptr, "Ticket provided to ClaimEntity is invalid."); + + ClaimEntityCommand queueEntry; + queueEntry.m_entityId = entityId; + QueueRequest(reinterpret_cast(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); } void SpawnableEntitiesManager::Barrier(EntitySpawnTicket& ticket, BarrierCallback completionCallback, BarrierOptionalArgs optionalArgs) @@ -143,7 +155,7 @@ namespace AzFramework BarrierCommand queueEntry; queueEntry.m_ticketId = ticket.GetId(); queueEntry.m_completionCallback = AZStd::move(completionCallback); - QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); } auto SpawnableEntitiesManager::ProcessQueue(CommandQueuePriority priority) -> CommandQueueStatus @@ -340,6 +352,7 @@ namespace AzFramework for (auto it = ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount; it != ticket.m_spawnedEntities.end(); ++it) { GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it); + AZ::Interface::Get()->AddSpawnedEntity((*it)->GetId(), &ticket); } // Let other systems know about newly spawned entities for any post-processing after adding to the scene/game context. @@ -421,6 +434,7 @@ namespace AzFramework for (auto it = ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount; it != ticket.m_spawnedEntities.end(); ++it) { GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it); + AZ::Interface::Get()->AddSpawnedEntity((*it)->GetId(), &ticket); } if (request.m_completionCallback) @@ -448,7 +462,7 @@ namespace AzFramework if (entity != nullptr) { GameEntityContextRequestBus::Broadcast( - &GameEntityContextRequestBus::Events::DestroyGameEntityAndDescendants, entity->GetId()); + &GameEntityContextRequestBus::Events::DestroyGameEntity, entity->GetId()); } } @@ -483,7 +497,7 @@ namespace AzFramework if (entity != nullptr) { GameEntityContextRequestBus::Broadcast( - &GameEntityContextRequestBus::Events::DestroyGameEntityAndDescendants, entity->GetId()); + &GameEntityContextRequestBus::Events::DestroyGameEntity, entity->GetId()); } } @@ -609,6 +623,31 @@ namespace AzFramework } } + bool SpawnableEntitiesManager::ProcessRequest(ClaimEntityCommand& request) + { + Ticket& ticket = *request.m_ticket; + if (request.m_requestId == ticket.m_currentRequestId) + { + AZStd::vector& spawnedEntities = ticket.m_spawnedEntities; + AZStd::vector::size_type entityIndex = 0; + for (entityIndex = 0; entityIndex < spawnedEntities.size(); entityIndex++) + { + if (spawnedEntities[entityIndex]->GetId() == request.m_entityId) + { + spawnedEntities.erase(spawnedEntities.begin() + entityIndex); + ticket.m_spawnedEntityIndices.erase(ticket.m_spawnedEntityIndices.begin() + entityIndex); + ticket.m_currentRequestId++; + return true; + } + } + return false; + } + else + { + return false; + } + } + bool SpawnableEntitiesManager::ProcessRequest(BarrierCommand& request) { Ticket& ticket = *request.m_ticket; @@ -637,7 +676,7 @@ namespace AzFramework if (entity != nullptr) { GameEntityContextRequestBus::Broadcast( - &GameEntityContextRequestBus::Events::DestroyGameEntityAndDescendants, entity->GetId()); + &GameEntityContextRequestBus::Events::DestroyGameEntity, entity->GetId()); } } delete request.m_ticket; diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h index 69985f8aa9..89181ea179 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h @@ -66,6 +66,8 @@ namespace AzFramework EntitySpawnTicket& ticket, ListIndicesEntitiesCallback listCallback, ListEntitiesOptionalArgs optionalArgs = {}) override; void ClaimEntities( EntitySpawnTicket& ticket, ClaimEntitiesCallback listCallback, ClaimEntitiesOptionalArgs optionalArgs = {}) override; + void ClaimEntity( + AZ::EntityId entityId, void* ticket, ClaimEntityOptionalArgs optionalArgs = {}) override; void Barrier(EntitySpawnTicket& spawnInfo, BarrierCallback completionCallback, BarrierOptionalArgs optionalArgs = {}) override; @@ -162,6 +164,13 @@ namespace AzFramework EntitySpawnTicket::Id m_ticketId; uint32_t m_requestId; }; + struct ClaimEntityCommand + { + Ticket* m_ticket; + EntitySpawnTicket::Id m_ticketId; + uint32_t m_requestId; + AZ::EntityId m_entityId; + }; struct BarrierCommand { BarrierCallback m_completionCallback; @@ -177,7 +186,7 @@ namespace AzFramework using Requests = AZStd::variant< SpawnAllEntitiesCommand, SpawnEntitiesCommand, DespawnAllEntitiesCommand, ReloadSpawnableCommand, ListEntitiesCommand, - ListIndicesEntitiesCommand, ClaimEntitiesCommand, BarrierCommand, DestroyTicketCommand>; + ListIndicesEntitiesCommand, ClaimEntitiesCommand, ClaimEntityCommand, BarrierCommand, DestroyTicketCommand>; struct Queue { @@ -187,7 +196,7 @@ namespace AzFramework }; template - void QueueRequest(EntitySpawnTicket& ticket, SpawnablePriority priority, T&& request); + void QueueRequest(Ticket* ticket, SpawnablePriority priority, T&& request); AZStd::pair CreateTicket(AZ::Data::Asset&& spawnable) override; void DestroyTicket(void* ticket) override; @@ -203,6 +212,7 @@ namespace AzFramework bool ProcessRequest(ListEntitiesCommand& request); bool ProcessRequest(ListIndicesEntitiesCommand& request); bool ProcessRequest(ClaimEntitiesCommand& request); + bool ProcessRequest(ClaimEntityCommand& request); bool ProcessRequest(BarrierCommand& request); bool ProcessRequest(DestroyTicketCommand& request); diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableSystemComponent.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableSystemComponent.h index 5b5fb1b7ee..33e0a29704 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableSystemComponent.h +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableSystemComponent.h @@ -18,6 +18,7 @@ #include #include #include +#include namespace AzFramework { @@ -90,6 +91,7 @@ namespace AzFramework void LoadRootSpawnableFromSettingsRegistry(); SpawnableAssetHandler m_assetHandler; + SpawnedEntityTicketMapper m_spawnedEntityTicketMapper; SpawnableEntitiesManager m_entitiesManager; SpawnableEntitiesContainer m_rootSpawnableContainer; AZ::SettingsRegistryInterface::NotifyEventHandler m_registryChangeHandler; diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.cpp new file mode 100644 index 0000000000..4179f3c111 --- /dev/null +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.cpp @@ -0,0 +1,43 @@ +/* + * 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 AzFramework +{ + SpawnedEntityTicketMapper::SpawnedEntityTicketMapper() + { + AZ::Interface::Register(this); + } + + SpawnedEntityTicketMapper::~SpawnedEntityTicketMapper() + { + AZ::Interface::Unregister(this); + } + + void SpawnedEntityTicketMapper::RemoveSpawnedEntity(AZ::EntityId entityId) + { + auto spawnedGameEntitiesIterator = m_spawnedEntities.find(entityId); + if (spawnedGameEntitiesIterator != m_spawnedEntities.end()) + { + SpawnableEntitiesDefinition* spawnableEntitiesInterface = SpawnableEntitiesInterface::Get(); + AZ_Assert(spawnableEntitiesInterface == nullptr, "SpawnableEntitiesInterface is not found."); + spawnableEntitiesInterface->ClaimEntity( + spawnedGameEntitiesIterator->first, spawnedGameEntitiesIterator->second); + m_spawnedEntities.erase(entityId); + } + } + + void SpawnedEntityTicketMapper::AddSpawnedEntity(AZ::EntityId entityId, void* ticket) + { + m_spawnedEntities.emplace(entityId, ticket); + } + +} + diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.h new file mode 100644 index 0000000000..36582a05b2 --- /dev/null +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.h @@ -0,0 +1,37 @@ +/* + * 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 + +namespace AzFramework +{ + class SpawnedEntityTicketMapper + : public SpawnedEntityTicketMapperInterface + { + public: + AZ_RTTI(SpawnedEntityTicketMapper, "{5C803604-E949-44F4-94A4-F835E5794C77}", SpawnedEntityTicketMapperInterface); + + SpawnedEntityTicketMapper(); + ~SpawnedEntityTicketMapper(); + + //! Removes the entityId from the spawned entities map. + //! @param entityId The id of the entity to remove. + void RemoveSpawnedEntity(AZ::EntityId entityId) override; + + //! Adds the entityId,ticket pair to the spawned entities map. + //! @param entityId The id of the entity to add. + //! @param ticket The ticket pointer to add. + void AddSpawnedEntity(AZ::EntityId entityId, void* ticket) override; + private: + AZStd::unordered_map m_spawnedEntities; + }; +} // namespace AzFramework diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapperInterface.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapperInterface.h new file mode 100644 index 0000000000..8f0bd96dbe --- /dev/null +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapperInterface.h @@ -0,0 +1,24 @@ +/* + * 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 + +namespace AzFramework +{ + class SpawnedEntityTicketMapperInterface + { + public: + AZ_RTTI(SpawnedEntityTicketMapperInterface, "{D407E96B-635C-44F2-B089-084EBAB8B036}"); + + virtual void RemoveSpawnedEntity(AZ::EntityId entityId) = 0; + virtual void AddSpawnedEntity(AZ::EntityId entityId, void* ticket) = 0; + }; +} diff --git a/Code/Framework/AzFramework/AzFramework/azframework_files.cmake b/Code/Framework/AzFramework/AzFramework/azframework_files.cmake index 87cf29ffec..b486596e2a 100644 --- a/Code/Framework/AzFramework/AzFramework/azframework_files.cmake +++ b/Code/Framework/AzFramework/AzFramework/azframework_files.cmake @@ -294,6 +294,9 @@ set(FILES Spawnable/SpawnableMonitor.cpp Spawnable/SpawnableSystemComponent.h Spawnable/SpawnableSystemComponent.cpp + Spawnable/SpawnedEntityTicketMapper.h + Spawnable/SpawnedEntityTicketMapper.cpp + Spawnable/SpawnedEntityTicketMapperInterface.h Terrain/TerrainDataRequestBus.h Terrain/TerrainDataRequestBus.cpp Thermal/ThermalInfo.h From 2cbd72e80132c5b0c8efce567a3317592ef0138c Mon Sep 17 00:00:00 2001 From: srikappa-amzn Date: Wed, 15 Sep 2021 14:57:54 -0700 Subject: [PATCH 02/20] Improved comments and minor changes to accessing interfaces Signed-off-by: srikappa-amzn --- .../Entity/GameEntityContextComponent.cpp | 5 ++++- .../Spawnable/SpawnableEntitiesManager.cpp | 15 +++++++-------- .../Spawnable/SpawnableEntitiesManager.h | 4 +++- .../Spawnable/SpawnableSystemComponent.h | 2 -- .../Spawnable/SpawnedEntityTicketMapper.cpp | 5 +++-- .../Spawnable/SpawnedEntityTicketMapper.h | 3 ++- 6 files changed, 19 insertions(+), 15 deletions(-) diff --git a/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp b/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp index 903bad91bf..694ed83765 100644 --- a/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp +++ b/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp @@ -290,7 +290,10 @@ namespace AzFramework isPrefabSystemEnabled, &AzFramework::ApplicationRequests::IsPrefabSystemEnabled); if (isPrefabSystemEnabled) { - AZ::Interface::Get()->RemoveSpawnedEntity(currentEntity->GetId()); + SpawnedEntityTicketMapperInterface* spawnedEntityTicketMapperInterface = + AZ::Interface::Get(); + AZ_Assert(spawnedEntityTicketMapperInterface != nullptr, "SpawnedEntityTicketMapperInterface is not found."); + spawnedEntityTicketMapperInterface->RemoveSpawnedEntity(currentEntity->GetId()); } if (currentEntity->GetState() == AZ::Entity::State::Active) diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp index eae71b5ea0..2c317bc33f 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp @@ -17,7 +17,6 @@ #include #include #include -#include namespace AzFramework { @@ -352,7 +351,7 @@ namespace AzFramework for (auto it = ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount; it != ticket.m_spawnedEntities.end(); ++it) { GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it); - AZ::Interface::Get()->AddSpawnedEntity((*it)->GetId(), &ticket); + m_spawnedEntityTicketMapper.AddSpawnedEntity((*it)->GetId(), &ticket); } // Let other systems know about newly spawned entities for any post-processing after adding to the scene/game context. @@ -434,7 +433,7 @@ namespace AzFramework for (auto it = ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount; it != ticket.m_spawnedEntities.end(); ++it) { GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it); - AZ::Interface::Get()->AddSpawnedEntity((*it)->GetId(), &ticket); + m_spawnedEntityTicketMapper.AddSpawnedEntity((*it)->GetId(), &ticket); } if (request.m_completionCallback) @@ -625,18 +624,18 @@ namespace AzFramework bool SpawnableEntitiesManager::ProcessRequest(ClaimEntityCommand& request) { - Ticket& ticket = *request.m_ticket; - if (request.m_requestId == ticket.m_currentRequestId) + Ticket* ticket = request.m_ticket; + if (request.m_requestId == ticket->m_currentRequestId) { - AZStd::vector& spawnedEntities = ticket.m_spawnedEntities; + AZStd::vector& spawnedEntities = ticket->m_spawnedEntities; AZStd::vector::size_type entityIndex = 0; for (entityIndex = 0; entityIndex < spawnedEntities.size(); entityIndex++) { if (spawnedEntities[entityIndex]->GetId() == request.m_entityId) { spawnedEntities.erase(spawnedEntities.begin() + entityIndex); - ticket.m_spawnedEntityIndices.erase(ticket.m_spawnedEntityIndices.begin() + entityIndex); - ticket.m_currentRequestId++; + ticket->m_spawnedEntityIndices.erase(ticket->m_spawnedEntityIndices.begin() + entityIndex); + ticket->m_currentRequestId++; return true; } } diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h index 89181ea179..e5a0760010 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h @@ -16,6 +16,7 @@ #include #include #include +#include namespace AZ { @@ -167,7 +168,6 @@ namespace AzFramework struct ClaimEntityCommand { Ticket* m_ticket; - EntitySpawnTicket::Id m_ticketId; uint32_t m_requestId; AZ::EntityId m_entityId; }; @@ -234,6 +234,8 @@ namespace AzFramework //! SpawnablePriority_Default which gives users a bit of room to fine tune the priorities as this value can be configured //! through the Settings Registry under the key "/O3DE/AzFramework/Spawnables/HighPriorityThreshold". SpawnablePriority m_highPriorityThreshold { 64 }; + private: + SpawnedEntityTicketMapper m_spawnedEntityTicketMapper; }; AZ_DEFINE_ENUM_BITWISE_OPERATORS(AzFramework::SpawnableEntitiesManager::CommandQueuePriority); diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableSystemComponent.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableSystemComponent.h index 33e0a29704..5b5fb1b7ee 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableSystemComponent.h +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableSystemComponent.h @@ -18,7 +18,6 @@ #include #include #include -#include namespace AzFramework { @@ -91,7 +90,6 @@ namespace AzFramework void LoadRootSpawnableFromSettingsRegistry(); SpawnableAssetHandler m_assetHandler; - SpawnedEntityTicketMapper m_spawnedEntityTicketMapper; SpawnableEntitiesManager m_entitiesManager; SpawnableEntitiesContainer m_rootSpawnableContainer; AZ::SettingsRegistryInterface::NotifyEventHandler m_registryChangeHandler; diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.cpp index 4179f3c111..3a6b40f5a9 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.cpp +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.cpp @@ -13,6 +13,9 @@ namespace AzFramework { SpawnedEntityTicketMapper::SpawnedEntityTicketMapper() { + spawnableEntitiesInterface = SpawnableEntitiesInterface::Get(); + AZ_Assert(spawnableEntitiesInterface != nullptr, "SpawnableEntitiesInterface is not found."); + AZ::Interface::Register(this); } @@ -26,8 +29,6 @@ namespace AzFramework auto spawnedGameEntitiesIterator = m_spawnedEntities.find(entityId); if (spawnedGameEntitiesIterator != m_spawnedEntities.end()) { - SpawnableEntitiesDefinition* spawnableEntitiesInterface = SpawnableEntitiesInterface::Get(); - AZ_Assert(spawnableEntitiesInterface == nullptr, "SpawnableEntitiesInterface is not found."); spawnableEntitiesInterface->ClaimEntity( spawnedGameEntitiesIterator->first, spawnedGameEntitiesIterator->second); m_spawnedEntities.erase(entityId); diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.h index 36582a05b2..45e701db4e 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.h +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.h @@ -23,7 +23,7 @@ namespace AzFramework SpawnedEntityTicketMapper(); ~SpawnedEntityTicketMapper(); - //! Removes the entityId from the spawned entities map. + //! Removes the entityId from the spawned entities map if present. //! @param entityId The id of the entity to remove. void RemoveSpawnedEntity(AZ::EntityId entityId) override; @@ -32,6 +32,7 @@ namespace AzFramework //! @param ticket The ticket pointer to add. void AddSpawnedEntity(AZ::EntityId entityId, void* ticket) override; private: + SpawnableEntitiesDefinition* spawnableEntitiesInterface = nullptr; AZStd::unordered_map m_spawnedEntities; }; } // namespace AzFramework From a4af34423a4b0e85ac4a6fc80408e9e06f120ba2 Mon Sep 17 00:00:00 2001 From: srikappa-amzn Date: Thu, 16 Sep 2021 00:03:00 -0700 Subject: [PATCH 03/20] Use iter_swap instead of erase Signed-off-by: srikappa-amzn --- .../Spawnable/SpawnableEntitiesManager.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp index 2c317bc33f..92d461819b 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp @@ -628,18 +628,16 @@ namespace AzFramework if (request.m_requestId == ticket->m_currentRequestId) { AZStd::vector& spawnedEntities = ticket->m_spawnedEntities; - AZStd::vector::size_type entityIndex = 0; - for (entityIndex = 0; entityIndex < spawnedEntities.size(); entityIndex++) + for (auto entityIterator = spawnedEntities.begin(); entityIterator != spawnedEntities.end(); ++entityIterator) { - if (spawnedEntities[entityIndex]->GetId() == request.m_entityId) + if ((*entityIterator)->GetId() == request.m_entityId) { - spawnedEntities.erase(spawnedEntities.begin() + entityIndex); - ticket->m_spawnedEntityIndices.erase(ticket->m_spawnedEntityIndices.begin() + entityIndex); - ticket->m_currentRequestId++; + AZStd::iter_swap(entityIterator, spawnedEntities.rbegin()); + spawnedEntities.pop_back(); return true; } } - return false; + return true; } else { From d11dbd37320c6121385ceee45e4fd5b7e60efd77 Mon Sep 17 00:00:00 2001 From: scspaldi Date: Tue, 21 Sep 2021 11:50:04 -0700 Subject: [PATCH 04/20] Updated Linux and Mac configs to show test results. Signed-off-by: scspaldi --- scripts/build/Platform/Linux/build_config.json | 12 ++++++++---- scripts/build/Platform/Mac/build_config.json | 6 ++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/scripts/build/Platform/Linux/build_config.json b/scripts/build/Platform/Linux/build_config.json index e70fa32878..ee4c27b0a9 100644 --- a/scripts/build/Platform/Linux/build_config.json +++ b/scripts/build/Platform/Linux/build_config.json @@ -83,7 +83,8 @@ "CMAKE_OPTIONS": "-G 'Ninja Multi-Config' -DCMAKE_C_COMPILER=clang-6.0 -DCMAKE_CXX_COMPILER=clang++-6.0 -DLY_UNITY_BUILD=TRUE -DLY_PARALLEL_LINK_JOBS=4", "CMAKE_LY_PROJECTS": "AutomatedTesting", "CMAKE_TARGET": "all", - "CTEST_OPTIONS": "-E Gem::EMotionFX.Editor.Tests -LE (SUITE_sandbox|SUITE_awsi) -L FRAMEWORK_googletest" + "CTEST_OPTIONS": "-E Gem::EMotionFX.Editor.Tests -LE (SUITE_sandbox|SUITE_awsi) -L FRAMEWORK_googletest", + "TEST_RESULTS": "True" } }, "test_profile_nounity": { @@ -95,7 +96,8 @@ "CMAKE_OPTIONS": "-G 'Ninja Multi-Config' -DCMAKE_C_COMPILER=clang-6.0 -DCMAKE_CXX_COMPILER=clang++-6.0 -DLY_UNITY_BUILD=FALSE -DLY_PARALLEL_LINK_JOBS=4", "CMAKE_LY_PROJECTS": "AutomatedTesting", "CMAKE_TARGET": "all", - "CTEST_OPTIONS": "-E Gem::EMotionFX.Editor.Tests -LE (SUITE_sandbox|SUITE_awsi) -L FRAMEWORK_googletest" + "CTEST_OPTIONS": "-E Gem::EMotionFX.Editor.Tests -LE (SUITE_sandbox|SUITE_awsi) -L FRAMEWORK_googletest", + "TEST_RESULTS": "True" } }, "asset_profile": { @@ -143,7 +145,8 @@ "CMAKE_OPTIONS": "-G 'Ninja Multi-Config' -DCMAKE_C_COMPILER=clang-6.0 -DCMAKE_CXX_COMPILER=clang++-6.0 -DLY_UNITY_BUILD=TRUE -DLY_PARALLEL_LINK_JOBS=4", "CMAKE_LY_PROJECTS": "AutomatedTesting", "CMAKE_TARGET": "TEST_SUITE_periodic", - "CTEST_OPTIONS": "-L (SUITE_periodic)" + "CTEST_OPTIONS": "-L (SUITE_periodic)", + "TEST_RESULTS": "True" } }, "sandbox_test_profile": { @@ -178,7 +181,8 @@ "CMAKE_OPTIONS": "-G 'Ninja Multi-Config' -DCMAKE_C_COMPILER=clang-6.0 -DCMAKE_CXX_COMPILER=clang++-6.0 -DLY_UNITY_BUILD=TRUE -DLY_PARALLEL_LINK_JOBS=4", "CMAKE_LY_PROJECTS": "AutomatedTesting", "CMAKE_TARGET": "TEST_SUITE_benchmark", - "CTEST_OPTIONS": "-L (SUITE_benchmark)" + "CTEST_OPTIONS": "-L (SUITE_benchmark)", + "TEST_RESULTS": "True" } }, "release": { diff --git a/scripts/build/Platform/Mac/build_config.json b/scripts/build/Platform/Mac/build_config.json index 2cf445a836..7eb3cb5699 100644 --- a/scripts/build/Platform/Mac/build_config.json +++ b/scripts/build/Platform/Mac/build_config.json @@ -102,7 +102,8 @@ "CMAKE_OPTIONS": "-G Xcode -DLY_UNITY_BUILD=TRUE", "CMAKE_LY_PROJECTS": "AutomatedTesting", "CMAKE_TARGET": "TEST_SUITE_periodic", - "CTEST_OPTIONS": "-L \"(SUITE_periodic)\"" + "CTEST_OPTIONS": "-L \"(SUITE_periodic)\"", + "TEST_RESULTS": "True" } }, "benchmark_test_profile": { @@ -118,7 +119,8 @@ "CMAKE_OPTIONS": "-G Xcode -DLY_UNITY_BUILD=TRUE", "CMAKE_LY_PROJECTS": "AutomatedTesting", "CMAKE_TARGET": "TEST_SUITE_benchmark", - "CTEST_OPTIONS": "-L \"(SUITE_benchmark)\"" + "CTEST_OPTIONS": "-L \"(SUITE_benchmark)\"", + "TEST_RESULTS": "True" } }, "release": { From 3c882230edd196e2803806cff201c94b3c5ae147 Mon Sep 17 00:00:00 2001 From: srikappa-amzn Date: Thu, 23 Sep 2021 15:25:15 -0700 Subject: [PATCH 05/20] Store the spawn ticket id inside entity instead of maintaining a map of entity to ticket Signed-off-by: srikappa-amzn --- .../AzCore/AzCore/Component/Entity.cpp | 10 ++ .../AzCore/AzCore/Component/Entity.h | 5 + .../Entity/GameEntityContextComponent.cpp | 21 ++- .../Spawnable/SpawnableEntitiesInterface.cpp | 22 ++- .../Spawnable/SpawnableEntitiesInterface.h | 36 +++-- .../Spawnable/SpawnableEntitiesManager.cpp | 130 +++++++++++------- .../Spawnable/SpawnableEntitiesManager.h | 38 +++-- .../Spawnable/SpawnedEntityTicketMapper.cpp | 44 ------ .../Spawnable/SpawnedEntityTicketMapper.h | 38 ----- .../SpawnedEntityTicketMapperInterface.h | 24 ---- .../AzFramework/azframework_files.cmake | 3 - 11 files changed, 179 insertions(+), 192 deletions(-) delete mode 100644 Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.cpp delete mode 100644 Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.h delete mode 100644 Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapperInterface.h diff --git a/Code/Framework/AzCore/AzCore/Component/Entity.cpp b/Code/Framework/AzCore/AzCore/Component/Entity.cpp index 00c1895261..4f55c360cd 100644 --- a/Code/Framework/AzCore/AzCore/Component/Entity.cpp +++ b/Code/Framework/AzCore/AzCore/Component/Entity.cpp @@ -655,6 +655,16 @@ namespace AZ m_stateEvent.Signal(oldState, m_state); } + void Entity::SetSpawnTicketId(u32 spawnTicketId) + { + m_spawnTicketId = AZStd::move(spawnTicketId); + } + + u32 Entity::GetSpawnTicketId() const + { + return m_spawnTicketId; + } + void Entity::OnNameChanged() const { EBUS_EVENT_ID(GetId(), EntityBus, OnEntityNameChanged, m_name); diff --git a/Code/Framework/AzCore/AzCore/Component/Entity.h b/Code/Framework/AzCore/AzCore/Component/Entity.h index 7ec63a56ac..97cec50946 100644 --- a/Code/Framework/AzCore/AzCore/Component/Entity.h +++ b/Code/Framework/AzCore/AzCore/Component/Entity.h @@ -133,6 +133,9 @@ namespace AZ //! @return The state of the entity. For example, the entity has been initialized, the entity is active, and so on. State GetState() const { return m_state; } + u32 GetSpawnTicketId() const; + void SetSpawnTicketId(u32); + //! Connects an entity state event handler to the entity. //! All state changes will be signaled through this event. //! @param handler reference to the EntityStateEvent handler to attach to the entities state event. @@ -411,6 +414,8 @@ namespace AZ //! A user-friendly name for the entity. This makes error messages easier to read. AZStd::string m_name; + u32 m_spawnTicketId = 0; + //! The state of the entity. State m_state; diff --git a/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp b/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp index 694ed83765..f2571c7549 100644 --- a/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp +++ b/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp @@ -14,7 +14,7 @@ #include #include #include -#include +#include #include "GameEntityContextComponent.h" @@ -290,10 +290,21 @@ namespace AzFramework isPrefabSystemEnabled, &AzFramework::ApplicationRequests::IsPrefabSystemEnabled); if (isPrefabSystemEnabled) { - SpawnedEntityTicketMapperInterface* spawnedEntityTicketMapperInterface = - AZ::Interface::Get(); - AZ_Assert(spawnedEntityTicketMapperInterface != nullptr, "SpawnedEntityTicketMapperInterface is not found."); - spawnedEntityTicketMapperInterface->RemoveSpawnedEntity(currentEntity->GetId()); + if (currentEntity->GetSpawnTicketId() > 0) + { + SpawnableEntitiesDefinition* spawnableEntitiesInterface = SpawnableEntitiesInterface::Get(); + AZ_Assert(spawnableEntitiesInterface != nullptr, "SpawnableEntitiesInterface is not found."); + spawnableEntitiesInterface->GetEntitySpawnTicket( + currentEntity->GetSpawnTicketId(), + [spawnableEntitiesInterface, currentEntity](EntitySpawnTicket* entitySpawnTicket) + { + if (entitySpawnTicket != nullptr) + { + spawnableEntitiesInterface->DespawnEntity(currentEntity->GetId(), *entitySpawnTicket); + } + }); + return; + } } if (currentEntity->GetState() == AZ::Entity::State::Active) diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.cpp index 617eb3a0b9..53bffd0d26 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.cpp +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.cpp @@ -227,10 +227,16 @@ namespace AzFramework EntitySpawnTicket::EntitySpawnTicket(EntitySpawnTicket&& rhs) : m_payload(rhs.m_payload) - , m_id(rhs.m_id) { + auto manager = SpawnableEntitiesInterface::Get(); + AZ_Assert(manager, "SpawnableEntitiesInterface has no implementation."); rhs.m_payload = nullptr; + Id previousId = m_id; + m_id = rhs.m_id; rhs.m_id = 0; + AZStd::scoped_lock lock(manager->m_entitySpawnTicketMapMutex); + manager->m_entitySpawnTicketMap.erase(previousId); + manager->m_entitySpawnTicketMap.insert_or_assign(rhs.m_id, this); } EntitySpawnTicket::EntitySpawnTicket(AZ::Data::Asset spawnable) @@ -240,6 +246,8 @@ namespace AzFramework AZStd::pair result = manager->CreateTicket(AZStd::move(spawnable)); m_id = result.first; m_payload = result.second; + AZStd::scoped_lock lock(manager->m_entitySpawnTicketMapMutex); + manager->m_entitySpawnTicketMap.insert_or_assign(m_id, this); } EntitySpawnTicket::~EntitySpawnTicket() @@ -250,6 +258,8 @@ namespace AzFramework AZ_Assert(manager, "Attempting to destroy an entity spawn ticket while the SpawnableEntitiesInterface has no implementation."); manager->DestroyTicket(m_payload); m_payload = nullptr; + AZStd::scoped_lock lock(manager->m_entitySpawnTicketMapMutex); + manager->m_entitySpawnTicketMap.erase(m_id); m_id = 0; } } @@ -258,17 +268,23 @@ namespace AzFramework { if (this != &rhs) { + auto manager = SpawnableEntitiesInterface::Get(); + AZ_Assert(manager, "Attempting to destroy an entity spawn ticket while the SpawnableEntitiesInterface has no implementation."); if (m_payload) { - auto manager = SpawnableEntitiesInterface::Get(); - AZ_Assert(manager, "Attempting to destroy an entity spawn ticket while the SpawnableEntitiesInterface has no implementation."); manager->DestroyTicket(m_payload); } + + Id previousId = m_id; m_id = rhs.m_id; rhs.m_id = 0; m_payload = rhs.m_payload; rhs.m_payload = nullptr; + + AZStd::scoped_lock lock(manager->m_entitySpawnTicketMapMutex); + manager->m_entitySpawnTicketMap.erase(previousId); + manager->m_entitySpawnTicketMap.insert_or_assign(m_id, this); } return *this; } diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h index 2ce4158229..91b2eafc54 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h @@ -140,7 +140,7 @@ namespace AzFramework public: friend class SpawnableEntitiesDefinition; - using Id = uint64_t; + using Id = uint32_t; EntitySpawnTicket() = default; EntitySpawnTicket(const EntitySpawnTicket&) = delete; @@ -162,6 +162,7 @@ namespace AzFramework using EntitySpawnCallback = AZStd::function; using EntityPreInsertionCallback = AZStd::function; using EntityDespawnCallback = AZStd::function; + using GetEntitySpawnTicketCallback = AZStd::function; using ReloadSpawnableCallback = AZStd::function; using ListEntitiesCallback = AZStd::function; using ListIndicesEntitiesCallback = AZStd::function; @@ -206,12 +207,21 @@ namespace AzFramework struct DespawnAllEntitiesOptionalArgs final { //! Callback that's called when despawning entities has completed. This can be triggered from a different thread than the one that - //! made the function call to despawn. The returned list of entities contains all the newly created entities. + //! made the function call to despawn. EntityDespawnCallback m_completionCallback; //! The priority at which this call will be executed. SpawnablePriority m_priority { SpawnablePriority_Default }; }; + struct DespawnEntityOptionalArgs final + { + //! Callback that's called when despawning entity has completed. This can be triggered from a different thread than the one that + //! made the function call to despawn. + EntityDespawnCallback m_completionCallback; + //! The priority at which this call will be executed. + SpawnablePriority m_priority{ SpawnablePriority_Default }; + }; + struct ReloadSpawnableOptionalArgs final { //! Callback that's called when respawning entities has completed. This can be triggered from a different thread than the one that @@ -235,12 +245,6 @@ namespace AzFramework SpawnablePriority m_priority{ SpawnablePriority_Default }; }; - struct ClaimEntityOptionalArgs final - { - //! The priority at which this call will be executed. - SpawnablePriority m_priority{ SpawnablePriority_Default }; - }; - struct BarrierOptionalArgs final { //! The priority at which this call will be executed. @@ -283,9 +287,18 @@ namespace AzFramework EntitySpawnTicket& ticket, AZStd::vector entityIndices, SpawnEntitiesOptionalArgs optionalArgs = {}) = 0; //! Removes all entities in the provided list from the environment. //! @param ticket The ticket previously used to spawn entities with. - //! @param priority The priority at which this call will be executed. //! @param optionalArgs Optional additional arguments, see DespawnAllEntitiesOptionalArgs. virtual void DespawnAllEntities(EntitySpawnTicket& ticket, DespawnAllEntitiesOptionalArgs optionalArgs = {}) = 0; + //! Removes the entity with the provided id from the spawned list of entities. + //! @param entityId the id of entity to despawn. + //! @param ticket The ticket previously used to spawn entities with. + //! @param optionalArgs Optional additional arguments, see DespawnEntityOptionalArgs. + virtual void DespawnEntity(AZ::EntityId entityId, EntitySpawnTicket& ticket, DespawnEntityOptionalArgs optionalArgs = {}) = 0; + //! Gets the EntitySpawnTicket associated with the entitySpawnTicketId. + //! @param entitySpawnTicketId the id of EntitySpawnTicket to get. + //! @param getEntitySpawnTicketCallback The callback to execute upon fetching the ticket. + virtual void GetEntitySpawnTicket( + EntitySpawnTicket::Id entitySpawnTicketId, GetEntitySpawnTicketCallback getEntitySpawnTicketCallback) = 0; //! Removes all entities in the provided list from the environment and reconstructs the entities from the provided spawnable. //! @param ticket Holds the information on the entities to reload. //! @param priority The priority at which this call will be executed. @@ -320,8 +333,6 @@ namespace AzFramework virtual void ClaimEntities( EntitySpawnTicket& ticket, ClaimEntitiesCallback listCallback, ClaimEntitiesOptionalArgs optionalArgs = {}) = 0; - virtual void ClaimEntity(AZ::EntityId entityId, void* ticket, ClaimEntityOptionalArgs optionalArgs = {}) = 0; - //! Blocks until all operations made on the provided ticket before the barrier call have completed. //! @param ticket The ticket to monitor. //! @param completionCallback Required callback that will be called as soon as the barrier has been reached. @@ -355,6 +366,9 @@ namespace AzFramework { return reinterpret_cast(ticket->m_payload); } + + AZStd::unordered_map m_entitySpawnTicketMap; + AZStd::mutex m_entitySpawnTicketMapMutex; }; using SpawnableEntitiesInterface = AZ::Interface; diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp index 92d461819b..c75a5fe834 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp @@ -21,13 +21,13 @@ namespace AzFramework { template - void SpawnableEntitiesManager::QueueRequest(Ticket* ticket, SpawnablePriority priority, T&& request) + void SpawnableEntitiesManager::QueueRequest(EntitySpawnTicket& ticket, SpawnablePriority priority, T&& request) { - request.m_ticket = ticket; + request.m_ticket = &GetTicketPayload(ticket); Queue& queue = priority <= m_highPriorityThreshold ? m_highPriorityQueue : m_regularPriorityQueue; { AZStd::scoped_lock queueLock(queue.m_pendingRequestMutex); - request.m_requestId = ticket->m_nextRequestId++; + request.m_requestId = GetTicketPayload(ticket).m_nextRequestId++; queue.m_pendingRequest.push(AZStd::move(request)); } } @@ -56,7 +56,7 @@ namespace AzFramework optionalArgs.m_serializeContext == nullptr ? m_defaultSerializeContext : optionalArgs.m_serializeContext; queueEntry.m_completionCallback = AZStd::move(optionalArgs.m_completionCallback); queueEntry.m_preInsertionCallback = AZStd::move(optionalArgs.m_preInsertionCallback); - QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); } void SpawnableEntitiesManager::SpawnEntities( @@ -72,7 +72,7 @@ namespace AzFramework queueEntry.m_completionCallback = AZStd::move(optionalArgs.m_completionCallback); queueEntry.m_preInsertionCallback = AZStd::move(optionalArgs.m_preInsertionCallback); queueEntry.m_referencePreviouslySpawnedEntities = optionalArgs.m_referencePreviouslySpawnedEntities; - QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); } void SpawnableEntitiesManager::DespawnAllEntities(EntitySpawnTicket& ticket, DespawnAllEntitiesOptionalArgs optionalArgs) @@ -82,7 +82,36 @@ namespace AzFramework DespawnAllEntitiesCommand queueEntry; queueEntry.m_ticketId = ticket.GetId(); queueEntry.m_completionCallback = AZStd::move(optionalArgs.m_completionCallback); - QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); + } + + void SpawnableEntitiesManager::DespawnEntity(AZ::EntityId entityId, EntitySpawnTicket& ticket, DespawnEntityOptionalArgs optionalArgs) + { + AZ_Assert(ticket.IsValid(), "Ticket provided to DespawnEntity hasn't been initialized."); + + DespawnEntityCommand queueEntry; + queueEntry.m_ticketId = ticket.GetId(); + queueEntry.m_entityId = entityId; + queueEntry.m_completionCallback = AZStd::move(optionalArgs.m_completionCallback); + QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); + } + + void SpawnableEntitiesManager::GetEntitySpawnTicket( + EntitySpawnTicket::Id entitySpawnTicketId, GetEntitySpawnTicketCallback getEntitySpawnTicketCallback) + { + if (entitySpawnTicketId == 0) + { + AZ_Error("Spawnable", false, "Ticket id provided to GetEntitySpawnTicket is invalid."); + return; + } + + auto entitySpawnTicketIterator = m_entitySpawnTicketMap.find(entitySpawnTicketId); + if (entitySpawnTicketIterator == m_entitySpawnTicketMap.end()) + { + AZ_Error("Spawnable", false, "The EntitySpawnTicket corresponding to id '%lu' cannot be found", entitySpawnTicketId); + return; + } + getEntitySpawnTicketCallback(entitySpawnTicketIterator->second); } void SpawnableEntitiesManager::ReloadSpawnable( @@ -96,7 +125,7 @@ namespace AzFramework queueEntry.m_serializeContext = optionalArgs.m_serializeContext == nullptr ? m_defaultSerializeContext : optionalArgs.m_serializeContext; queueEntry.m_completionCallback = AZStd::move(optionalArgs.m_completionCallback); - QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); } void SpawnableEntitiesManager::ListEntities( @@ -108,7 +137,7 @@ namespace AzFramework ListEntitiesCommand queueEntry; queueEntry.m_ticketId = ticket.GetId(); queueEntry.m_listCallback = AZStd::move(listCallback); - QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); } void SpawnableEntitiesManager::ListIndicesAndEntities( @@ -120,7 +149,7 @@ namespace AzFramework ListIndicesEntitiesCommand queueEntry; queueEntry.m_ticketId = ticket.GetId(); queueEntry.m_listCallback = AZStd::move(listCallback); - QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); } void SpawnableEntitiesManager::ClaimEntities( @@ -132,18 +161,7 @@ namespace AzFramework ClaimEntitiesCommand queueEntry; queueEntry.m_ticketId = ticket.GetId(); queueEntry.m_listCallback = AZStd::move(listCallback); - QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); - } - - void SpawnableEntitiesManager::ClaimEntity( - AZ::EntityId entityId, void* ticket, - ClaimEntityOptionalArgs optionalArgs) - { - AZ_Assert(ticket == nullptr, "Ticket provided to ClaimEntity is invalid."); - - ClaimEntityCommand queueEntry; - queueEntry.m_entityId = entityId; - QueueRequest(reinterpret_cast(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); } void SpawnableEntitiesManager::Barrier(EntitySpawnTicket& ticket, BarrierCallback completionCallback, BarrierOptionalArgs optionalArgs) @@ -154,7 +172,7 @@ namespace AzFramework BarrierCommand queueEntry; queueEntry.m_ticketId = ticket.GetId(); queueEntry.m_completionCallback = AZStd::move(completionCallback); - QueueRequest(&GetTicketPayload(ticket), optionalArgs.m_priority, AZStd::move(queueEntry)); + QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); } auto SpawnableEntitiesManager::ProcessQueue(CommandQueuePriority priority) -> CommandQueueStatus @@ -234,12 +252,13 @@ namespace AzFramework return queue.m_delayed.empty() ? CommandQueueStatus::NoCommandsLeft : CommandQueueStatus::HasCommandsLeft; } - AZStd::pair SpawnableEntitiesManager::CreateTicket(AZ::Data::Asset&& spawnable) + AZStd::pair SpawnableEntitiesManager::CreateTicket(AZ::Data::Asset&& spawnable) { - static AZStd::atomic_uint64_t idCounter { 1 }; + static AZStd::atomic_uint32_t idCounter { 1 }; auto result = aznew Ticket(); result->m_spawnable = AZStd::move(spawnable); + return AZStd::make_pair(idCounter++, result); } @@ -351,7 +370,7 @@ namespace AzFramework for (auto it = ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount; it != ticket.m_spawnedEntities.end(); ++it) { GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it); - m_spawnedEntityTicketMapper.AddSpawnedEntity((*it)->GetId(), &ticket); + (*it)->SetSpawnTicketId(request.m_ticketId); } // Let other systems know about newly spawned entities for any post-processing after adding to the scene/game context. @@ -433,7 +452,7 @@ namespace AzFramework for (auto it = ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount; it != ticket.m_spawnedEntities.end(); ++it) { GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it); - m_spawnedEntityTicketMapper.AddSpawnedEntity((*it)->GetId(), &ticket); + (*it)->SetSpawnTicketId(request.m_ticketId); } if (request.m_completionCallback) @@ -460,6 +479,7 @@ namespace AzFramework { if (entity != nullptr) { + entity->SetSpawnTicketId(0); GameEntityContextRequestBus::Broadcast( &GameEntityContextRequestBus::Events::DestroyGameEntity, entity->GetId()); } @@ -482,6 +502,39 @@ namespace AzFramework } } + bool SpawnableEntitiesManager::ProcessRequest(DespawnEntityCommand& request) + { + Ticket& ticket = *request.m_ticket; + if (request.m_requestId == ticket.m_currentRequestId) + { + AZStd::vector& spawnedEntities = request.m_ticket->m_spawnedEntities; + for (auto entityIterator = spawnedEntities.begin(); entityIterator != spawnedEntities.end(); ++entityIterator) + { + if (*entityIterator != nullptr && (*entityIterator)->GetId() == request.m_entityId) + { + (*entityIterator)->SetSpawnTicketId(0); + GameEntityContextRequestBus::Broadcast( + &GameEntityContextRequestBus::Events::DestroyGameEntity, (*entityIterator)->GetId()); + AZStd::iter_swap(entityIterator, spawnedEntities.rbegin()); + spawnedEntities.pop_back(); + break; + } + } + + if (request.m_completionCallback) + { + request.m_completionCallback(request.m_ticketId); + } + + ticket.m_currentRequestId++; + return true; + } + else + { + return false; + } + } + bool SpawnableEntitiesManager::ProcessRequest(ReloadSpawnableCommand& request) { Ticket& ticket = *request.m_ticket; @@ -495,6 +548,7 @@ namespace AzFramework { if (entity != nullptr) { + entity->SetSpawnTicketId(0); GameEntityContextRequestBus::Broadcast( &GameEntityContextRequestBus::Events::DestroyGameEntity, entity->GetId()); } @@ -622,29 +676,6 @@ namespace AzFramework } } - bool SpawnableEntitiesManager::ProcessRequest(ClaimEntityCommand& request) - { - Ticket* ticket = request.m_ticket; - if (request.m_requestId == ticket->m_currentRequestId) - { - AZStd::vector& spawnedEntities = ticket->m_spawnedEntities; - for (auto entityIterator = spawnedEntities.begin(); entityIterator != spawnedEntities.end(); ++entityIterator) - { - if ((*entityIterator)->GetId() == request.m_entityId) - { - AZStd::iter_swap(entityIterator, spawnedEntities.rbegin()); - spawnedEntities.pop_back(); - return true; - } - } - return true; - } - else - { - return false; - } - } - bool SpawnableEntitiesManager::ProcessRequest(BarrierCommand& request) { Ticket& ticket = *request.m_ticket; @@ -672,6 +703,7 @@ namespace AzFramework { if (entity != nullptr) { + entity->SetSpawnTicketId(0); GameEntityContextRequestBus::Broadcast( &GameEntityContextRequestBus::Events::DestroyGameEntity, entity->GetId()); } diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h index e5a0760010..5445360d64 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h @@ -16,7 +16,6 @@ #include #include #include -#include namespace AZ { @@ -58,6 +57,9 @@ namespace AzFramework void SpawnEntities( EntitySpawnTicket& ticket, AZStd::vector entityIndices, SpawnEntitiesOptionalArgs optionalArgs = {}) override; void DespawnAllEntities(EntitySpawnTicket& ticket, DespawnAllEntitiesOptionalArgs optionalArgs = {}) override; + void DespawnEntity(AZ::EntityId entityId, EntitySpawnTicket& ticket, DespawnEntityOptionalArgs optionalArgs = {}) override; + void GetEntitySpawnTicket( + EntitySpawnTicket::Id entitySpawnTicketId, GetEntitySpawnTicketCallback getEntitySpawnTicketCallback) override; void ReloadSpawnable( EntitySpawnTicket& ticket, AZ::Data::Asset spawnable, ReloadSpawnableOptionalArgs optionalArgs = {}) override; @@ -67,8 +69,6 @@ namespace AzFramework EntitySpawnTicket& ticket, ListIndicesEntitiesCallback listCallback, ListEntitiesOptionalArgs optionalArgs = {}) override; void ClaimEntities( EntitySpawnTicket& ticket, ClaimEntitiesCallback listCallback, ClaimEntitiesOptionalArgs optionalArgs = {}) override; - void ClaimEntity( - AZ::EntityId entityId, void* ticket, ClaimEntityOptionalArgs optionalArgs = {}) override; void Barrier(EntitySpawnTicket& spawnInfo, BarrierCallback completionCallback, BarrierOptionalArgs optionalArgs = {}) override; @@ -135,6 +135,14 @@ namespace AzFramework EntitySpawnTicket::Id m_ticketId; uint32_t m_requestId; }; + struct DespawnEntityCommand + { + EntityDespawnCallback m_completionCallback; + Ticket* m_ticket; + EntitySpawnTicket::Id m_ticketId; + uint32_t m_requestId; + AZ::EntityId m_entityId; + }; struct ReloadSpawnableCommand { AZ::Data::Asset m_spawnable; @@ -165,12 +173,6 @@ namespace AzFramework EntitySpawnTicket::Id m_ticketId; uint32_t m_requestId; }; - struct ClaimEntityCommand - { - Ticket* m_ticket; - uint32_t m_requestId; - AZ::EntityId m_entityId; - }; struct BarrierCommand { BarrierCallback m_completionCallback; @@ -185,8 +187,16 @@ namespace AzFramework }; using Requests = AZStd::variant< - SpawnAllEntitiesCommand, SpawnEntitiesCommand, DespawnAllEntitiesCommand, ReloadSpawnableCommand, ListEntitiesCommand, - ListIndicesEntitiesCommand, ClaimEntitiesCommand, ClaimEntityCommand, BarrierCommand, DestroyTicketCommand>; + SpawnAllEntitiesCommand, + SpawnEntitiesCommand, + DespawnAllEntitiesCommand, + DespawnEntityCommand, + ReloadSpawnableCommand, + ListEntitiesCommand, + ListIndicesEntitiesCommand, + ClaimEntitiesCommand, + BarrierCommand, + DestroyTicketCommand>; struct Queue { @@ -196,7 +206,7 @@ namespace AzFramework }; template - void QueueRequest(Ticket* ticket, SpawnablePriority priority, T&& request); + void QueueRequest(EntitySpawnTicket& ticket, SpawnablePriority priority, T&& request); AZStd::pair CreateTicket(AZ::Data::Asset&& spawnable) override; void DestroyTicket(void* ticket) override; @@ -208,11 +218,11 @@ namespace AzFramework bool ProcessRequest(SpawnAllEntitiesCommand& request); bool ProcessRequest(SpawnEntitiesCommand& request); bool ProcessRequest(DespawnAllEntitiesCommand& request); + bool ProcessRequest(DespawnEntityCommand& request); bool ProcessRequest(ReloadSpawnableCommand& request); bool ProcessRequest(ListEntitiesCommand& request); bool ProcessRequest(ListIndicesEntitiesCommand& request); bool ProcessRequest(ClaimEntitiesCommand& request); - bool ProcessRequest(ClaimEntityCommand& request); bool ProcessRequest(BarrierCommand& request); bool ProcessRequest(DestroyTicketCommand& request); @@ -234,8 +244,6 @@ namespace AzFramework //! SpawnablePriority_Default which gives users a bit of room to fine tune the priorities as this value can be configured //! through the Settings Registry under the key "/O3DE/AzFramework/Spawnables/HighPriorityThreshold". SpawnablePriority m_highPriorityThreshold { 64 }; - private: - SpawnedEntityTicketMapper m_spawnedEntityTicketMapper; }; AZ_DEFINE_ENUM_BITWISE_OPERATORS(AzFramework::SpawnableEntitiesManager::CommandQueuePriority); diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.cpp deleted file mode 100644 index 3a6b40f5a9..0000000000 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.cpp +++ /dev/null @@ -1,44 +0,0 @@ -/* - * 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 AzFramework -{ - SpawnedEntityTicketMapper::SpawnedEntityTicketMapper() - { - spawnableEntitiesInterface = SpawnableEntitiesInterface::Get(); - AZ_Assert(spawnableEntitiesInterface != nullptr, "SpawnableEntitiesInterface is not found."); - - AZ::Interface::Register(this); - } - - SpawnedEntityTicketMapper::~SpawnedEntityTicketMapper() - { - AZ::Interface::Unregister(this); - } - - void SpawnedEntityTicketMapper::RemoveSpawnedEntity(AZ::EntityId entityId) - { - auto spawnedGameEntitiesIterator = m_spawnedEntities.find(entityId); - if (spawnedGameEntitiesIterator != m_spawnedEntities.end()) - { - spawnableEntitiesInterface->ClaimEntity( - spawnedGameEntitiesIterator->first, spawnedGameEntitiesIterator->second); - m_spawnedEntities.erase(entityId); - } - } - - void SpawnedEntityTicketMapper::AddSpawnedEntity(AZ::EntityId entityId, void* ticket) - { - m_spawnedEntities.emplace(entityId, ticket); - } - -} - diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.h deleted file mode 100644 index 45e701db4e..0000000000 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapper.h +++ /dev/null @@ -1,38 +0,0 @@ -/* - * 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 - -namespace AzFramework -{ - class SpawnedEntityTicketMapper - : public SpawnedEntityTicketMapperInterface - { - public: - AZ_RTTI(SpawnedEntityTicketMapper, "{5C803604-E949-44F4-94A4-F835E5794C77}", SpawnedEntityTicketMapperInterface); - - SpawnedEntityTicketMapper(); - ~SpawnedEntityTicketMapper(); - - //! Removes the entityId from the spawned entities map if present. - //! @param entityId The id of the entity to remove. - void RemoveSpawnedEntity(AZ::EntityId entityId) override; - - //! Adds the entityId,ticket pair to the spawned entities map. - //! @param entityId The id of the entity to add. - //! @param ticket The ticket pointer to add. - void AddSpawnedEntity(AZ::EntityId entityId, void* ticket) override; - private: - SpawnableEntitiesDefinition* spawnableEntitiesInterface = nullptr; - AZStd::unordered_map m_spawnedEntities; - }; -} // namespace AzFramework diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapperInterface.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapperInterface.h deleted file mode 100644 index 8f0bd96dbe..0000000000 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnedEntityTicketMapperInterface.h +++ /dev/null @@ -1,24 +0,0 @@ -/* - * 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 - -namespace AzFramework -{ - class SpawnedEntityTicketMapperInterface - { - public: - AZ_RTTI(SpawnedEntityTicketMapperInterface, "{D407E96B-635C-44F2-B089-084EBAB8B036}"); - - virtual void RemoveSpawnedEntity(AZ::EntityId entityId) = 0; - virtual void AddSpawnedEntity(AZ::EntityId entityId, void* ticket) = 0; - }; -} diff --git a/Code/Framework/AzFramework/AzFramework/azframework_files.cmake b/Code/Framework/AzFramework/AzFramework/azframework_files.cmake index b486596e2a..87cf29ffec 100644 --- a/Code/Framework/AzFramework/AzFramework/azframework_files.cmake +++ b/Code/Framework/AzFramework/AzFramework/azframework_files.cmake @@ -294,9 +294,6 @@ set(FILES Spawnable/SpawnableMonitor.cpp Spawnable/SpawnableSystemComponent.h Spawnable/SpawnableSystemComponent.cpp - Spawnable/SpawnedEntityTicketMapper.h - Spawnable/SpawnedEntityTicketMapper.cpp - Spawnable/SpawnedEntityTicketMapperInterface.h Terrain/TerrainDataRequestBus.h Terrain/TerrainDataRequestBus.cpp Thermal/ThermalInfo.h From 28c056e4fdb9e11bbe22c3b186fd12b279370bc2 Mon Sep 17 00:00:00 2001 From: srikappa-amzn Date: Thu, 23 Sep 2021 18:26:39 -0700 Subject: [PATCH 06/20] Added comments and changed some function names Signed-off-by: srikappa-amzn --- .../AzCore/AzCore/Component/Entity.cpp | 2 +- .../Entity/GameEntityContextComponent.cpp | 2 +- .../Spawnable/SpawnableEntitiesInterface.cpp | 4 +--- .../Spawnable/SpawnableEntitiesInterface.h | 9 ++++----- .../Spawnable/SpawnableEntitiesManager.cpp | 18 +++++++++++------- .../Spawnable/SpawnableEntitiesManager.h | 5 ++--- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Component/Entity.cpp b/Code/Framework/AzCore/AzCore/Component/Entity.cpp index 4f55c360cd..4febd893b0 100644 --- a/Code/Framework/AzCore/AzCore/Component/Entity.cpp +++ b/Code/Framework/AzCore/AzCore/Component/Entity.cpp @@ -657,7 +657,7 @@ namespace AZ void Entity::SetSpawnTicketId(u32 spawnTicketId) { - m_spawnTicketId = AZStd::move(spawnTicketId); + m_spawnTicketId = spawnTicketId; } u32 Entity::GetSpawnTicketId() const diff --git a/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp b/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp index f2571c7549..aa7e03577a 100644 --- a/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp +++ b/Code/Framework/AzFramework/AzFramework/Entity/GameEntityContextComponent.cpp @@ -294,7 +294,7 @@ namespace AzFramework { SpawnableEntitiesDefinition* spawnableEntitiesInterface = SpawnableEntitiesInterface::Get(); AZ_Assert(spawnableEntitiesInterface != nullptr, "SpawnableEntitiesInterface is not found."); - spawnableEntitiesInterface->GetEntitySpawnTicket( + spawnableEntitiesInterface->RetrieveEntitySpawnTicket( currentEntity->GetSpawnTicketId(), [spawnableEntitiesInterface, currentEntity](EntitySpawnTicket* entitySpawnTicket) { diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.cpp index 53bffd0d26..d915680760 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.cpp +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.cpp @@ -227,15 +227,13 @@ namespace AzFramework EntitySpawnTicket::EntitySpawnTicket(EntitySpawnTicket&& rhs) : m_payload(rhs.m_payload) + , m_id(rhs.m_id) { auto manager = SpawnableEntitiesInterface::Get(); AZ_Assert(manager, "SpawnableEntitiesInterface has no implementation."); rhs.m_payload = nullptr; - Id previousId = m_id; - m_id = rhs.m_id; rhs.m_id = 0; AZStd::scoped_lock lock(manager->m_entitySpawnTicketMapMutex); - manager->m_entitySpawnTicketMap.erase(previousId); manager->m_entitySpawnTicketMap.insert_or_assign(rhs.m_id, this); } diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h index 91b2eafc54..3eea87b511 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesInterface.h @@ -162,7 +162,7 @@ namespace AzFramework using EntitySpawnCallback = AZStd::function; using EntityPreInsertionCallback = AZStd::function; using EntityDespawnCallback = AZStd::function; - using GetEntitySpawnTicketCallback = AZStd::function; + using RetrieveEntitySpawnTicketCallback = AZStd::function; using ReloadSpawnableCallback = AZStd::function; using ListEntitiesCallback = AZStd::function; using ListIndicesEntitiesCallback = AZStd::function; @@ -296,9 +296,8 @@ namespace AzFramework virtual void DespawnEntity(AZ::EntityId entityId, EntitySpawnTicket& ticket, DespawnEntityOptionalArgs optionalArgs = {}) = 0; //! Gets the EntitySpawnTicket associated with the entitySpawnTicketId. //! @param entitySpawnTicketId the id of EntitySpawnTicket to get. - //! @param getEntitySpawnTicketCallback The callback to execute upon fetching the ticket. - virtual void GetEntitySpawnTicket( - EntitySpawnTicket::Id entitySpawnTicketId, GetEntitySpawnTicketCallback getEntitySpawnTicketCallback) = 0; + //! @param callback The callback to execute upon retrieving the ticket. + virtual void RetrieveEntitySpawnTicket(EntitySpawnTicket::Id entitySpawnTicketId, RetrieveEntitySpawnTicketCallback callback) = 0; //! Removes all entities in the provided list from the environment and reconstructs the entities from the provided spawnable. //! @param ticket Holds the information on the entities to reload. //! @param priority The priority at which this call will be executed. @@ -368,7 +367,7 @@ namespace AzFramework } AZStd::unordered_map m_entitySpawnTicketMap; - AZStd::mutex m_entitySpawnTicketMapMutex; + AZStd::recursive_mutex m_entitySpawnTicketMapMutex; }; using SpawnableEntitiesInterface = AZ::Interface; diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp index c75a5fe834..ef7351aabb 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.cpp @@ -96,22 +96,22 @@ namespace AzFramework QueueRequest(ticket, optionalArgs.m_priority, AZStd::move(queueEntry)); } - void SpawnableEntitiesManager::GetEntitySpawnTicket( - EntitySpawnTicket::Id entitySpawnTicketId, GetEntitySpawnTicketCallback getEntitySpawnTicketCallback) + void SpawnableEntitiesManager::RetrieveEntitySpawnTicket(EntitySpawnTicket::Id entitySpawnTicketId, RetrieveEntitySpawnTicketCallback callback) { if (entitySpawnTicketId == 0) { - AZ_Error("Spawnable", false, "Ticket id provided to GetEntitySpawnTicket is invalid."); + AZ_Assert(false, "Ticket id provided to RetrieveEntitySpawnTicket is invalid."); return; } + AZStd::scoped_lock lock(m_entitySpawnTicketMapMutex); auto entitySpawnTicketIterator = m_entitySpawnTicketMap.find(entitySpawnTicketId); if (entitySpawnTicketIterator == m_entitySpawnTicketMap.end()) { - AZ_Error("Spawnable", false, "The EntitySpawnTicket corresponding to id '%lu' cannot be found", entitySpawnTicketId); + AZ_Assert(false, "The EntitySpawnTicket corresponding to id '%lu' cannot be found", entitySpawnTicketId); return; } - getEntitySpawnTicketCallback(entitySpawnTicketIterator->second); + callback(entitySpawnTicketIterator->second); } void SpawnableEntitiesManager::ReloadSpawnable( @@ -369,8 +369,8 @@ namespace AzFramework // Add to the game context, now the entities are active for (auto it = ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount; it != ticket.m_spawnedEntities.end(); ++it) { - GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it); (*it)->SetSpawnTicketId(request.m_ticketId); + GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it); } // Let other systems know about newly spawned entities for any post-processing after adding to the scene/game context. @@ -451,8 +451,8 @@ namespace AzFramework // Add to the game context, now the entities are active for (auto it = ticket.m_spawnedEntities.begin() + spawnedEntitiesInitialCount; it != ticket.m_spawnedEntities.end(); ++it) { - GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it); (*it)->SetSpawnTicketId(request.m_ticketId); + GameEntityContextRequestBus::Broadcast(&GameEntityContextRequestBus::Events::AddGameEntity, *it); } if (request.m_completionCallback) @@ -479,6 +479,7 @@ namespace AzFramework { if (entity != nullptr) { + // Setting it to 0 is needed to avoid the infite loop between GameEntityContext and SpawnableEntitiesManager. entity->SetSpawnTicketId(0); GameEntityContextRequestBus::Broadcast( &GameEntityContextRequestBus::Events::DestroyGameEntity, entity->GetId()); @@ -512,6 +513,7 @@ namespace AzFramework { if (*entityIterator != nullptr && (*entityIterator)->GetId() == request.m_entityId) { + // Setting it to 0 is needed to avoid the infite loop between GameEntityContext and SpawnableEntitiesManager. (*entityIterator)->SetSpawnTicketId(0); GameEntityContextRequestBus::Broadcast( &GameEntityContextRequestBus::Events::DestroyGameEntity, (*entityIterator)->GetId()); @@ -548,6 +550,7 @@ namespace AzFramework { if (entity != nullptr) { + // Setting it to 0 is needed to avoid the infite loop between GameEntityContext and SpawnableEntitiesManager. entity->SetSpawnTicketId(0); GameEntityContextRequestBus::Broadcast( &GameEntityContextRequestBus::Events::DestroyGameEntity, entity->GetId()); @@ -703,6 +706,7 @@ namespace AzFramework { if (entity != nullptr) { + // Setting it to 0 is needed to avoid the infite loop between GameEntityContext and SpawnableEntitiesManager. entity->SetSpawnTicketId(0); GameEntityContextRequestBus::Broadcast( &GameEntityContextRequestBus::Events::DestroyGameEntity, entity->GetId()); diff --git a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h index 5445360d64..c3de5be003 100644 --- a/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h +++ b/Code/Framework/AzFramework/AzFramework/Spawnable/SpawnableEntitiesManager.h @@ -58,8 +58,7 @@ namespace AzFramework EntitySpawnTicket& ticket, AZStd::vector entityIndices, SpawnEntitiesOptionalArgs optionalArgs = {}) override; void DespawnAllEntities(EntitySpawnTicket& ticket, DespawnAllEntitiesOptionalArgs optionalArgs = {}) override; void DespawnEntity(AZ::EntityId entityId, EntitySpawnTicket& ticket, DespawnEntityOptionalArgs optionalArgs = {}) override; - void GetEntitySpawnTicket( - EntitySpawnTicket::Id entitySpawnTicketId, GetEntitySpawnTicketCallback getEntitySpawnTicketCallback) override; + void RetrieveEntitySpawnTicket(EntitySpawnTicket::Id entitySpawnTicketId, RetrieveEntitySpawnTicketCallback callback) override; void ReloadSpawnable( EntitySpawnTicket& ticket, AZ::Data::Asset spawnable, ReloadSpawnableOptionalArgs optionalArgs = {}) override; @@ -139,9 +138,9 @@ namespace AzFramework { EntityDespawnCallback m_completionCallback; Ticket* m_ticket; + AZ::EntityId m_entityId; EntitySpawnTicket::Id m_ticketId; uint32_t m_requestId; - AZ::EntityId m_entityId; }; struct ReloadSpawnableCommand { From dcc325dfaa826b803d2d865fa0bf529dc5981471 Mon Sep 17 00:00:00 2001 From: srikappa-amzn Date: Fri, 24 Sep 2021 12:16:38 -0700 Subject: [PATCH 07/20] Added function comments for new entity functions Signed-off-by: srikappa-amzn --- Code/Framework/AzCore/AzCore/Component/Entity.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Code/Framework/AzCore/AzCore/Component/Entity.h b/Code/Framework/AzCore/AzCore/Component/Entity.h index 97cec50946..3887b2e03e 100644 --- a/Code/Framework/AzCore/AzCore/Component/Entity.h +++ b/Code/Framework/AzCore/AzCore/Component/Entity.h @@ -133,8 +133,13 @@ namespace AZ //! @return The state of the entity. For example, the entity has been initialized, the entity is active, and so on. State GetState() const { return m_state; } + //! Gets the ticket id used to spawn the entity. + //! @return the ticket id used to spawn the entity. If entity is not spawned, the id will be 0. u32 GetSpawnTicketId() const; - void SetSpawnTicketId(u32); + + //! Sets the ticket id used to spawn the entity. The ticket id in the entity will remain 0 unless it's set using this function. + //! @param spawnTicketId the ticket id used to spawn the entity. + void SetSpawnTicketId(u32 spawnTicketId); //! Connects an entity state event handler to the entity. //! All state changes will be signaled through this event. From 4998f44246841002b64ecadce9872f1eadb244f2 Mon Sep 17 00:00:00 2001 From: evanchia Date: Mon, 27 Sep 2021 14:25:43 -0700 Subject: [PATCH 08/20] adding process_utils linux integ test Signed-off-by: evanchia --- .../tests/integ/test_process_utils_linux.py | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 Tools/LyTestTools/tests/integ/test_process_utils_linux.py diff --git a/Tools/LyTestTools/tests/integ/test_process_utils_linux.py b/Tools/LyTestTools/tests/integ/test_process_utils_linux.py new file mode 100644 index 0000000000..23ad8f6653 --- /dev/null +++ b/Tools/LyTestTools/tests/integ/test_process_utils_linux.py @@ -0,0 +1,39 @@ +""" +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 +""" +import subprocess +import pytest + +import ly_test_tools.environment.process_utils as process_utils +import ly_test_tools.environment.waiter as waiter +from ly_test_tools import LINUX + +if LINUX: + pytestmark = pytest.mark.SUITE_smoke +else: + pytestmark = pytest.mark.skipif(not LINUX, reason="Only runs on Linux") + + +class TestProcessUtils(object): + + def test_KillLinuxProcess_ProcessStarted_KilledSuccessfully(self): + # Construct a simple timeout command + linux_executable = 'timeout' + command = [linux_executable, '5s', 'echo'] + + # Verification function for the waiter to call + def process_killed(): + return not process_utils.process_exists(linux_executable, ignore_extensions=True) + + # Create a new process with no output in a new session + with subprocess.Popen(command, stdout=subprocess.DEVNULL, start_new_session=True): + # Ensure that the process was started + assert process_utils.process_exists(linux_executable, ignore_extensions=True), \ + f"Process '{linux_executable}' was expected to exist, but could not be found." + # Kill the process using the process_utils module + process_utils.kill_processes_named(linux_executable, ignore_extensions=True) + # Verify that the process was killed + waiter.wait_for(process_killed, timeout=2) # Raises exception if the process is alive. From bcf024edf9eec8e42bfe6dec91b9fd8000321058 Mon Sep 17 00:00:00 2001 From: evanchia Date: Mon, 27 Sep 2021 14:33:08 -0700 Subject: [PATCH 09/20] registering linux integ test to cmake Signed-off-by: evanchia --- Tools/LyTestTools/tests/CMakeLists.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Tools/LyTestTools/tests/CMakeLists.txt b/Tools/LyTestTools/tests/CMakeLists.txt index a6d7a0734c..31766f3080 100644 --- a/Tools/LyTestTools/tests/CMakeLists.txt +++ b/Tools/LyTestTools/tests/CMakeLists.txt @@ -39,6 +39,14 @@ if(PAL_TRAIT_BUILD_HOST_TOOLS AND PAL_TRAIT_BUILD_TESTS_SUPPORTED AND AutomatedT TEST_SUITE smoke COMPONENT TestTools ) + + ly_add_pytest( + NAME LyTestTools_IntegTests_ProcessUtilsLinux_smoke_no_gpu + PATH ${CMAKE_CURRENT_LIST_DIR}/integ/test_process_utils_linux.py + TEST_SERIAL + TEST_SUITE smoke + COMPONENT TestTools + ) # Regression tests. ly_add_pytest( From 6c7cd2abf0a981f37708dd38d0e7085ed31db5cb Mon Sep 17 00:00:00 2001 From: Guthrie Adams Date: Mon, 27 Sep 2021 18:50:33 -0500 Subject: [PATCH 10/20] =?UTF-8?q?Atom=20Tools:=20Fixing=20problems=20with?= =?UTF-8?q?=20inspector=20property=20group=20layout=20and=20deletion=20?= =?UTF-8?q?=E2=80=A2=20A=20previous=20change=20introduced=20a=20heading=20?= =?UTF-8?q?section=20of=20the=20top=20of=20the=20inspector=20=E2=80=A2=20T?= =?UTF-8?q?he=20code=20was=20incorrectly=20attempting=20to=20delete=20chil?= =?UTF-8?q?d=20widgets=20and=20layouts=20from=20the=20layout=20of=20each?= =?UTF-8?q?=20section=20=E2=80=A2=20Size=20policies=20were=20also=20update?= =?UTF-8?q?d=20to=20give=20priority=20to=20the=20section=20containing=20th?= =?UTF-8?q?e=20property=20groups=20=E2=80=A2=20Tested=20material=20compone?= =?UTF-8?q?nt=20property=20editor=20and=20material=20editor=20to=20make=20?= =?UTF-8?q?sure=20that=20widgets=20were=20laid=20out=20correctly=20at=20th?= =?UTF-8?q?e=20top=20of=20the=20inspector?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Guthrie Adams --- .../Code/Source/Inspector/InspectorWidget.cpp | 18 +- .../Code/Source/Inspector/InspectorWidget.ui | 169 ++++++++---------- 2 files changed, 87 insertions(+), 100 deletions(-) diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Inspector/InspectorWidget.cpp b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Inspector/InspectorWidget.cpp index 5eda93ca59..5ef2dbb2c5 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Inspector/InspectorWidget.cpp +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Inspector/InspectorWidget.cpp @@ -31,20 +31,26 @@ namespace AtomToolsFramework void InspectorWidget::AddHeading(QWidget* headingWidget) { - headingWidget->setParent(m_ui->m_headingSection); - m_ui->m_headingSectionLayout->addWidget(headingWidget); + headingWidget->setParent(this); + m_ui->m_headingContentsLayout->addWidget(headingWidget); } void InspectorWidget::ClearHeading() { - qDeleteAll(m_ui->m_headingSection->findChildren(QString(), Qt::FindDirectChildrenOnly)); - qDeleteAll(m_ui->m_headingSectionLayout->children()); + while (QLayoutItem* child = m_ui->m_headingContentsLayout->takeAt(0)) + { + delete child->widget(); + delete child; + } } void InspectorWidget::Reset() { - qDeleteAll(m_ui->m_groupContents->findChildren(QString(), Qt::FindDirectChildrenOnly)); - qDeleteAll(m_ui->m_groupContentsLayout->children()); + while (QLayoutItem* child = m_ui->m_groupContentsLayout->takeAt(0)) + { + delete child->widget(); + delete child; + } m_groups.clear(); } diff --git a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Inspector/InspectorWidget.ui b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Inspector/InspectorWidget.ui index 13f9c9e41c..c197ee172e 100644 --- a/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Inspector/InspectorWidget.ui +++ b/Gems/Atom/Tools/AtomToolsFramework/Code/Source/Inspector/InspectorWidget.ui @@ -30,104 +30,85 @@ 0 - - - - 0 - - - 0 - - - 0 - - - 0 - - - 0 - - - - - - 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + + + + + 0 + + + 0 + + + 0 + + + 0 + + + 0 + + + + + Qt::ScrollBarAsNeeded - - 0 + + true - - 0 - - - 0 - - - 0 - - - - - - - - - 0 - - - 0 - - - 0 - - - 0 - - - 0 - - - - - Qt::ScrollBarAsNeeded + + + + 0 + 0 + 679 + 767 + + + + + 0 + 0 + + + + + 0 - - true + + 0 - - - - 0 - 0 - 683 - 763 - - - - - 0 - - - 0 - - - 0 - - - 0 - - - 0 - - - - - - - - - - + + 0 + + + 0 + + + 0 + + + + + + From 96d99c2814ce8da8d5fbd322b1e8456caaac601b Mon Sep 17 00:00:00 2001 From: AMZN-Igarri <82394219+AMZN-Igarri@users.noreply.github.com> Date: Tue, 28 Sep 2021 10:11:15 +0200 Subject: [PATCH 11/20] Switched over from Error to Warning - Restore viewport when exiting game mode error. (#4343) Signed-off-by: igarri --- Code/Editor/EditorViewportWidget.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Code/Editor/EditorViewportWidget.cpp b/Code/Editor/EditorViewportWidget.cpp index 5b5b52d28f..dad6734428 100644 --- a/Code/Editor/EditorViewportWidget.cpp +++ b/Code/Editor/EditorViewportWidget.cpp @@ -2428,7 +2428,7 @@ void EditorViewportWidget::RestoreViewportAfterGameMode() } else { - AZ_Error("CryLegacy", false, "Not restoring the editor viewport camera is currently unsupported"); + AZ_Warning("CryLegacy", false, "Not restoring the editor viewport camera is currently unsupported"); SetViewTM(preGameModeViewTM); } } From 7da866e81a8cc547e17e93462173cd2ab4c534e2 Mon Sep 17 00:00:00 2001 From: hultonha <82228511+hultonha@users.noreply.github.com> Date: Tue, 28 Sep 2021 09:26:20 +0100 Subject: [PATCH 12/20] Update to how entity space is treated in the viewport (#4263) * first pass fixes for how entity space is handled in the viewport interaction model Signed-off-by: hultonha * small updates to simplify space handling Signed-off-by: hultonha * fix for influence group with one entity selected Signed-off-by: hultonha * some tidy-up and fix for scale manipulator snapping Signed-off-by: hultonha * couple of small fixes for scale manipulator Signed-off-by: hultonha * small comment update Signed-off-by: hultonha * fixes for integration test failures Signed-off-by: hultonha * add test for rotation manipulator Signed-off-by: hultonha * add test coverage for rotation manipulators Signed-off-by: hultonha * add tests for translation manipulators and some other tidy-up changes Signed-off-by: hultonha * add tests for translating a group of entities Signed-off-by: hultonha * add some tests for scale manipulators Signed-off-by: hultonha * some updates and formatting changes (clang-format) to ViewportScreen Signed-off-by: hultonha * simplify usage of lround code Signed-off-by: hultonha * update missed name updates Signed-off-by: hultonha * update usage of WorldToScreenNdc Signed-off-by: hultonha * updates following review feedback Signed-off-by: hultonha * some more small tidy-up changes Signed-off-by: hultonha * move static variables to be marked inline Signed-off-by: hultonha * small formatting fixes Signed-off-by: hultonha --- .../AzFramework/Viewport/ScreenGeometry.h | 53 +- .../AzFramework/Viewport/ViewportScreen.cpp | 50 +- .../AzFramework/Viewport/ViewportScreen.h | 41 +- .../Manipulators/ScaleManipulators.cpp | 1 + .../Manipulators/TranslationManipulators.cpp | 27 +- .../EditorTransformComponentSelection.cpp | 179 +++--- .../EditorTransformComponentSelection.h | 11 +- ...EditorTransformComponentSelectionTests.cpp | 600 +++++++++++++++++- .../Tests/Viewport/ViewportScreenTests.cpp | 128 ++-- .../AtomFont/Code/Source/FFont.cpp | 2 +- 10 files changed, 868 insertions(+), 224 deletions(-) diff --git a/Code/Framework/AzFramework/AzFramework/Viewport/ScreenGeometry.h b/Code/Framework/AzFramework/AzFramework/Viewport/ScreenGeometry.h index c0fa1ad631..1529d760e5 100644 --- a/Code/Framework/AzFramework/AzFramework/Viewport/ScreenGeometry.h +++ b/Code/Framework/AzFramework/AzFramework/Viewport/ScreenGeometry.h @@ -8,11 +8,10 @@ #pragma once -#include -#include -#include -#include #include +#include +#include +#include namespace AZ { @@ -27,7 +26,11 @@ namespace AzFramework AZ_TYPE_INFO(ScreenPoint, "{8472B6C2-527F-44FC-87F8-C226B1A57A97}"); ScreenPoint() = default; - ScreenPoint(int x, int y) : m_x(x), m_y(y) {} + ScreenPoint(int x, int y) + : m_x(x) + , m_y(y) + { + } int m_x; //!< X screen position. int m_y; //!< Y screen position. @@ -42,7 +45,11 @@ namespace AzFramework AZ_TYPE_INFO(ScreenVector, "{1EAA2C62-8FDB-4A28-9FE3-1FA4F1418894}"); ScreenVector() = default; - ScreenVector(int x, int y) : m_x(x), m_y(y) {} + ScreenVector(int x, int y) + : m_x(x) + , m_y(y) + { + } int m_x; //!< X screen delta. int m_y; //!< Y screen delta. @@ -71,14 +78,14 @@ namespace AzFramework inline const ScreenPoint operator+(const ScreenPoint& lhs, const ScreenVector& rhs) { - ScreenPoint result{lhs}; + ScreenPoint result{ lhs }; result += rhs; return result; } inline const ScreenPoint operator-(const ScreenPoint& lhs, const ScreenVector& rhs) { - ScreenPoint result{lhs}; + ScreenPoint result{ lhs }; result -= rhs; return result; } @@ -99,14 +106,14 @@ namespace AzFramework inline const ScreenVector operator+(const ScreenVector& lhs, const ScreenVector& rhs) { - ScreenVector result{lhs}; + ScreenVector result{ lhs }; result += rhs; return result; } inline const ScreenVector operator-(const ScreenVector& lhs, const ScreenVector& rhs) { - ScreenVector result{lhs}; + ScreenVector result{ lhs }; result -= rhs; return result; } @@ -131,23 +138,25 @@ namespace AzFramework return !operator==(lhs, rhs); } + inline ScreenVector& operator*=(ScreenVector& lhs, const float rhs) + { + lhs.m_x = aznumeric_cast(AZStd::lround(aznumeric_cast(lhs.m_x) * rhs)); + lhs.m_y = aznumeric_cast(AZStd::lround(aznumeric_cast(lhs.m_y) * rhs)); + return lhs; + } + + inline const ScreenVector operator*(const ScreenVector& lhs, const float rhs) + { + ScreenVector result{ lhs }; + result *= rhs; + return result; + } + inline float ScreenVectorLength(const ScreenVector& screenVector) { return aznumeric_cast(AZStd::sqrt(screenVector.m_x * screenVector.m_x + screenVector.m_y * screenVector.m_y)); } - inline ScreenPoint ScreenPointFromNDC(const AZ::Vector3& screenNDC, const AZ::Vector2& viewportSize) - { - return ScreenPoint( - aznumeric_caster(AZStd::round(screenNDC.GetX() * viewportSize.GetX())), - aznumeric_caster(AZStd::round((1.0f - screenNDC.GetY()) * viewportSize.GetY()))); - } - - inline AZ::Vector2 NDCFromScreenPoint(const ScreenPoint& screenPoint, const AZ::Vector2& viewportSize) - { - return AZ::Vector2(aznumeric_cast(screenPoint.m_x), viewportSize.GetY() - aznumeric_cast(screenPoint.m_y)) / viewportSize; - } - //! Return an AZ::Vector2 from a ScreenPoint. inline AZ::Vector2 Vector2FromScreenPoint(const ScreenPoint& screenPoint) { diff --git a/Code/Framework/AzFramework/AzFramework/Viewport/ViewportScreen.cpp b/Code/Framework/AzFramework/AzFramework/Viewport/ViewportScreen.cpp index a94b806ad3..afd16a6c23 100644 --- a/Code/Framework/AzFramework/AzFramework/Viewport/ViewportScreen.cpp +++ b/Code/Framework/AzFramework/AzFramework/Viewport/ViewportScreen.cpp @@ -23,7 +23,7 @@ namespace AzFramework // y-up, z into the screen, x-left // // x -> -x - // y -> z + // y -> z // z -> y // // the same transform can be used to go to/from z-up - the only difference is the order of @@ -37,15 +37,15 @@ namespace AzFramework // yaw = AZ::Matrix4x4::CreateRotationZ(AZ::DegToRad(180.0f)); // conversion = pitch * yaw return AZ::Matrix4x4::CreateFromColumns( - AZ::Vector4(-1.0f, 0.0f, 0.0f, 0.0f), AZ::Vector4(0.0f, 0.0f, 1.0f, .0f), - AZ::Vector4(0.0f, 1.0f, 0.0f, 0.0f), AZ::Vector4(0.0f, 0.0f, 0.0f, 1.0f)); + AZ::Vector4(-1.0f, 0.0f, 0.0f, 0.0f), AZ::Vector4(0.0f, 0.0f, 1.0f, 0.0f), AZ::Vector4(0.0f, 1.0f, 0.0f, 0.0f), + AZ::Vector4(0.0f, 0.0f, 0.0f, 1.0f)); } AZ::Matrix4x4 CameraTransform(const CameraState& cameraState) { return AZ::Matrix4x4::CreateFromColumns( - AZ::Vector3ToVector4(cameraState.m_side), AZ::Vector3ToVector4(cameraState.m_forward), - AZ::Vector3ToVector4(cameraState.m_up), AZ::Vector3ToVector4(cameraState.m_position, 1.0f)); + AZ::Vector3ToVector4(cameraState.m_side), AZ::Vector3ToVector4(cameraState.m_forward), AZ::Vector3ToVector4(cameraState.m_up), + AZ::Vector3ToVector4(cameraState.m_position, 1.0f)); } AZ::Matrix4x4 CameraView(const CameraState& cameraState) @@ -63,8 +63,7 @@ namespace AzFramework AZ::Matrix4x4 CameraProjection(const CameraState& cameraState) { return AZ::Matrix4x4::CreateProjection( - cameraState.VerticalFovRadian(), AspectRatio(cameraState.m_viewportSize), cameraState.m_nearClip, - cameraState.m_farClip); + cameraState.VerticalFovRadian(), AspectRatio(cameraState.m_viewportSize), cameraState.m_nearClip, cameraState.m_farClip); } AZ::Matrix4x4 InverseCameraProjection(const CameraState& cameraState) @@ -93,12 +92,11 @@ namespace AzFramework const auto cameraWorldTransform = AZ::Transform::CreateFromMatrix3x3AndTranslation( AZ::Matrix3x3::CreateFromMatrix4x4(worldFromView), worldFromView.GetTranslation()); return AZ::ViewFrustumAttributes( - cameraWorldTransform, AspectRatio(cameraState.m_viewportSize), cameraState.m_fovOrZoom, - cameraState.m_nearClip, cameraState.m_farClip); + cameraWorldTransform, AspectRatio(cameraState.m_viewportSize), cameraState.m_fovOrZoom, cameraState.m_nearClip, + cameraState.m_farClip); } - AZ::Vector3 WorldToScreenNDC( - const AZ::Vector3& worldPosition, const AZ::Matrix4x4& cameraView, const AZ::Matrix4x4& cameraProjection) + AZ::Vector3 WorldToScreenNdc(const AZ::Vector3& worldPosition, const AZ::Matrix4x4& cameraView, const AZ::Matrix4x4& cameraProjection) { // transform the world space position to clip space const auto clipSpacePosition = cameraProjection * cameraView * AZ::Vector3ToVector4(worldPosition, 1.0f); @@ -108,25 +106,24 @@ namespace AzFramework return (AZ::Vector4ToVector3(ndcPosition) + AZ::Vector3::CreateOne()) * 0.5f; } - ScreenPoint WorldToScreen( - const AZ::Vector3& worldPosition, const AZ::Matrix4x4& cameraView, const AZ::Matrix4x4& cameraProjection, + const AZ::Vector3& worldPosition, + const AZ::Matrix4x4& cameraView, + const AZ::Matrix4x4& cameraProjection, const AZ::Vector2& viewportSize) { - const auto ndcNormalizedPosition = WorldToScreenNDC(worldPosition, cameraView, cameraProjection); + const auto ndcNormalizedPosition = WorldToScreenNdc(worldPosition, cameraView, cameraProjection); // scale ndc position by screen dimensions to return screen position - return ScreenPointFromNDC(ndcNormalizedPosition, viewportSize); + return ScreenPointFromNdc(AZ::Vector3ToVector2(ndcNormalizedPosition), viewportSize); } ScreenPoint WorldToScreen(const AZ::Vector3& worldPosition, const CameraState& cameraState) { - return WorldToScreen( - worldPosition, CameraView(cameraState), CameraProjection(cameraState), cameraState.m_viewportSize); + return WorldToScreen(worldPosition, CameraView(cameraState), CameraProjection(cameraState), cameraState.m_viewportSize); } - AZ::Vector3 ScreenNDCToWorld( - const AZ::Vector2& normalizedScreenPosition, const AZ::Matrix4x4& inverseCameraView, - const AZ::Matrix4x4& inverseCameraProjection) + AZ::Vector3 ScreenNdcToWorld( + const AZ::Vector2& normalizedScreenPosition, const AZ::Matrix4x4& inverseCameraView, const AZ::Matrix4x4& inverseCameraProjection) { // convert screen space coordinates from <0, 1> to <-1,1> range const auto ndcPosition = normalizedScreenPosition * 2.0f - AZ::Vector2::CreateOne(); @@ -142,18 +139,19 @@ namespace AzFramework } AZ::Vector3 ScreenToWorld( - const ScreenPoint& screenPosition, const AZ::Matrix4x4& inverseCameraView, - const AZ::Matrix4x4& inverseCameraProjection, const AZ::Vector2& viewportSize) + const ScreenPoint& screenPosition, + const AZ::Matrix4x4& inverseCameraView, + const AZ::Matrix4x4& inverseCameraProjection, + const AZ::Vector2& viewportSize) { - const auto normalizedScreenPosition = NDCFromScreenPoint(screenPosition, viewportSize); + const auto normalizedScreenPosition = NdcFromScreenPoint(screenPosition, viewportSize); - return ScreenNDCToWorld(normalizedScreenPosition, inverseCameraView, inverseCameraProjection); + return ScreenNdcToWorld(normalizedScreenPosition, inverseCameraView, inverseCameraProjection); } AZ::Vector3 ScreenToWorld(const ScreenPoint& screenPosition, const CameraState& cameraState) { return ScreenToWorld( - screenPosition, InverseCameraView(cameraState), InverseCameraProjection(cameraState), - cameraState.m_viewportSize); + screenPosition, InverseCameraView(cameraState), InverseCameraProjection(cameraState), cameraState.m_viewportSize); } } // namespace AzFramework diff --git a/Code/Framework/AzFramework/AzFramework/Viewport/ViewportScreen.h b/Code/Framework/AzFramework/AzFramework/Viewport/ViewportScreen.h index 87bad66eab..8a6c0249b2 100644 --- a/Code/Framework/AzFramework/AzFramework/Viewport/ViewportScreen.h +++ b/Code/Framework/AzFramework/AzFramework/Viewport/ViewportScreen.h @@ -8,26 +8,42 @@ #pragma once +#include #include +#include +#include namespace AZ { class Frustum; class Matrix4x4; - class Vector3; struct ViewFrustumAttributes; } // namespace AZ namespace AzFramework { struct CameraState; - struct ScreenPoint; struct ViewportInfo; - //! Projects a position in world space to screen space normalized device coordinates for the given camera. - AZ::Vector3 WorldToScreenNDC( - const AZ::Vector3& worldPosition, const AZ::Matrix4x4& cameraView, const AZ::Matrix4x4& cameraProjection); + //! Returns a position in screen space (in the range [0-viewportSize.x, 0-viewportSize.y]) from normalized device + //! coordinates (in the range 0.0-1.0). + inline ScreenPoint ScreenPointFromNdc(const AZ::Vector2& screenNdc, const AZ::Vector2& viewportSize) + { + return ScreenPoint( + aznumeric_cast(AZStd::lround(screenNdc.GetX() * viewportSize.GetX())), + aznumeric_cast(AZStd::lround((1.0f - screenNdc.GetY()) * viewportSize.GetY()))); + } + //! Returns a position in normalized device coordinates (in the range [0.0-1.0, 0.0-1.0]) from a + //! screen space position (in the range [0-viewportSize.x, 0-viewportSize.y]). + inline AZ::Vector2 NdcFromScreenPoint(const ScreenPoint& screenPoint, const AZ::Vector2& viewportSize) + { + return AZ::Vector2(aznumeric_cast(screenPoint.m_x), viewportSize.GetY() - aznumeric_cast(screenPoint.m_y)) / + viewportSize; + } + + //! Projects a position in world space to screen space normalized device coordinates for the given camera. + AZ::Vector3 WorldToScreenNdc(const AZ::Vector3& worldPosition, const AZ::Matrix4x4& cameraView, const AZ::Matrix4x4& cameraProjection); //! Projects a position in world space to screen space for the given camera. ScreenPoint WorldToScreen(const AZ::Vector3& worldPosition, const CameraState& cameraState); @@ -35,7 +51,9 @@ namespace AzFramework //! Overload of WorldToScreen that accepts camera values that can be precomputed if this function //! is called many times in a loop. ScreenPoint WorldToScreen( - const AZ::Vector3& worldPosition, const AZ::Matrix4x4& cameraView, const AZ::Matrix4x4& cameraProjection, + const AZ::Vector3& worldPosition, + const AZ::Matrix4x4& cameraView, + const AZ::Matrix4x4& cameraProjection, const AZ::Vector2& viewportSize); //! Unprojects a position in screen space pixel coordinates to world space. @@ -45,14 +63,15 @@ namespace AzFramework //! Overload of ScreenToWorld that accepts camera values that can be precomputed if this function //! is called many times in a loop. AZ::Vector3 ScreenToWorld( - const ScreenPoint& screenPosition, const AZ::Matrix4x4& inverseCameraView, - const AZ::Matrix4x4& inverseCameraProjection, const AZ::Vector2& viewportSize); + const ScreenPoint& screenPosition, + const AZ::Matrix4x4& inverseCameraView, + const AZ::Matrix4x4& inverseCameraProjection, + const AZ::Vector2& viewportSize); //! Unprojects a position in screen space normalized device coordinates to world space. //! Note: The position returned will be on the near clip plane of the camera in world space. - AZ::Vector3 ScreenNDCToWorld( - const AZ::Vector2& ndcPosition, const AZ::Matrix4x4& inverseCameraView, - const AZ::Matrix4x4& inverseCameraProjection); + AZ::Vector3 ScreenNdcToWorld( + const AZ::Vector2& ndcPosition, const AZ::Matrix4x4& inverseCameraView, const AZ::Matrix4x4& inverseCameraProjection); //! Returns the camera projection for the current camera state. AZ::Matrix4x4 CameraProjection(const CameraState& cameraState); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Manipulators/ScaleManipulators.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Manipulators/ScaleManipulators.cpp index 1867f346ec..d8c43ace9e 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Manipulators/ScaleManipulators.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Manipulators/ScaleManipulators.cpp @@ -71,6 +71,7 @@ namespace AzToolsFramework } m_uniformScaleManipulator->SetVisualOrientationOverride(QuaternionFromTransformNoScaling(localTransform)); + m_uniformScaleManipulator->SetLocalPosition(localTransform.GetTranslation()); m_uniformScaleManipulator->SetLocalOrientation(AZ::Quaternion::CreateIdentity()); } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Manipulators/TranslationManipulators.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Manipulators/TranslationManipulators.cpp index 049cab4a13..0efe310250 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Manipulators/TranslationManipulators.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Manipulators/TranslationManipulators.cpp @@ -13,14 +13,14 @@ namespace AzToolsFramework { - static const float s_surfaceManipulatorTransparency = 0.75f; - static const float s_axisLength = 2.0f; - static const float s_surfaceManipulatorRadius = 0.1f; + static const float SurfaceManipulatorTransparency = 0.75f; + static const float LinearManipulatorAxisLength = 2.0f; + static const float SurfaceManipulatorRadius = 0.1f; - static const AZ::Color s_xAxisColor = AZ::Color(1.0f, 0.0f, 0.0f, 1.0f); - static const AZ::Color s_yAxisColor = AZ::Color(0.0f, 1.0f, 0.0f, 1.0f); - static const AZ::Color s_zAxisColor = AZ::Color(0.0f, 0.0f, 1.0f, 1.0f); - static const AZ::Color s_surfaceManipulatorColor = AZ::Color(1.0f, 1.0f, 0.0f, 0.5f); + static const AZ::Color LinearManipulatorXAxisColor = AZ::Color(1.0f, 0.0f, 0.0f, 1.0f); + static const AZ::Color LinearManipulatorYAxisColor = AZ::Color(0.0f, 1.0f, 0.0f, 1.0f); + static const AZ::Color LinearManipulatorZAxisColor = AZ::Color(0.0f, 0.0f, 1.0f, 1.0f); + static const AZ::Color SurfaceManipulatorColor = AZ::Color(1.0f, 1.0f, 0.0f, 0.5f); TranslationManipulators::TranslationManipulators( const Dimensions dimensions, const AZ::Transform& worldFromLocal, const AZ::Vector3& nonUniformScale) @@ -291,7 +291,7 @@ namespace AzToolsFramework { const AZ::Color color[2] = { defaultColor, - Vector3ToVector4(BaseManipulator::s_defaultMouseOverColor.GetAsVector3(), s_surfaceManipulatorTransparency) + Vector3ToVector4(BaseManipulator::s_defaultMouseOverColor.GetAsVector3(), SurfaceManipulatorTransparency) }; return color[mouseOver]; @@ -325,15 +325,16 @@ namespace AzToolsFramework void ConfigureTranslationManipulatorAppearance3d(TranslationManipulators* translationManipulators) { translationManipulators->SetAxes(AZ::Vector3::CreateAxisX(), AZ::Vector3::CreateAxisY(), AZ::Vector3::CreateAxisZ()); - translationManipulators->ConfigurePlanarView(s_xAxisColor, s_yAxisColor, s_zAxisColor); - translationManipulators->ConfigureLinearView(s_axisLength, s_xAxisColor, s_yAxisColor, s_zAxisColor); - translationManipulators->ConfigureSurfaceView(s_surfaceManipulatorRadius, s_surfaceManipulatorColor); + translationManipulators->ConfigurePlanarView(LinearManipulatorXAxisColor, LinearManipulatorYAxisColor, LinearManipulatorZAxisColor); + translationManipulators->ConfigureLinearView( + LinearManipulatorAxisLength, LinearManipulatorXAxisColor, LinearManipulatorYAxisColor, LinearManipulatorZAxisColor); + translationManipulators->ConfigureSurfaceView(SurfaceManipulatorRadius, SurfaceManipulatorColor); } void ConfigureTranslationManipulatorAppearance2d(TranslationManipulators* translationManipulators) { translationManipulators->SetAxes(AZ::Vector3::CreateAxisX(), AZ::Vector3::CreateAxisY()); - translationManipulators->ConfigurePlanarView(s_xAxisColor); - translationManipulators->ConfigureLinearView(s_axisLength, s_xAxisColor, s_yAxisColor); + translationManipulators->ConfigurePlanarView(LinearManipulatorXAxisColor); + translationManipulators->ConfigureLinearView(LinearManipulatorAxisLength, LinearManipulatorXAxisColor, LinearManipulatorYAxisColor); } } // namespace AzToolsFramework diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp index 8157d369d2..ccb72e1d50 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.cpp @@ -700,13 +700,6 @@ namespace AzToolsFramework switch (referenceFrame) { case ReferenceFrame::Local: - // if we have a group selection, always use the pivot override if one - // is set when moving to local space (can't pick individual local space) - if (entityIdMap.size() > 1) - { - pivot.m_worldOrientation = pivotOverrideFrame.m_orientationOverride.value(); - } - break; case ReferenceFrame::Parent: pivot.m_worldOrientation = pivotOverrideFrame.m_orientationOverride.value(); break; @@ -784,7 +777,7 @@ namespace AzToolsFramework SortEntitiesByLocationInHierarchy(sortedEntityIdsOut); } - static void UpdateInitialRotation(EntityIdManipulators& entityManipulators) + static void UpdateInitialTransform(EntityIdManipulators& entityManipulators) { // save new start orientation (if moving rotation axes separate from object // or switching type of rotation (modifier keys change)) @@ -797,22 +790,17 @@ namespace AzToolsFramework } } - // utility function to immediately return the current reference frame - // based on the state of the modifiers + // utility function to immediately return the current reference frame based on the state of the modifiers static ReferenceFrame ReferenceFrameFromModifiers(const ViewportInteraction::KeyboardModifiers modifiers) { - if (modifiers.Shift() && !modifiers.Alt()) - { - return ReferenceFrame::World; - } - else if (modifiers.Alt() && !modifiers.Shift()) - { - return ReferenceFrame::Local; - } - else - { - return ReferenceFrame::Parent; - } + return modifiers.Shift() ? ReferenceFrame::World : ReferenceFrame::Local; + } + + // utility function to immediately return the current sphere of influence of the manipulators based on the + // state of the modifiers + static Influence InfluenceFromModifiers(const ViewportInteraction::KeyboardModifiers modifiers) + { + return modifiers.Alt() ? Influence::Individual : Influence::Group; } template @@ -838,6 +826,7 @@ namespace AzToolsFramework else { const ReferenceFrame referenceFrame = spaceLock.value_or(ReferenceFrameFromModifiers(action.m_modifiers)); + const Influence influence = InfluenceFromModifiers(action.m_modifiers); // note: used for parent and world depending on the current reference frame const auto pivotOrientation = @@ -854,9 +843,9 @@ namespace AzToolsFramework const AZ::Vector3 worldTranslation = GetWorldTranslation(entityId); - switch (referenceFrame) + switch (influence) { - case ReferenceFrame::Local: + case Influence::Individual: { // move in each entities local space at once AZ::Quaternion worldOrientation = AZ::Quaternion::CreateIdentity(); @@ -876,10 +865,9 @@ namespace AzToolsFramework entityId, entityItLookupIt->second.m_initial.GetTranslation() + localOffset, transformChangedInternally); } break; - case ReferenceFrame::Parent: - case ReferenceFrame::World: + case Influence::Group: { - AZ::Quaternion offsetRotation = pivotOrientation.m_worldOrientation * + const AZ::Quaternion offsetRotation = pivotOrientation.m_worldOrientation * QuaternionFromTransformNoScaling(entityIdManipulators.m_manipulators->GetLocalTransform().GetInverse()); const AZ::Vector3 localOffset = offsetRotation.TransformVector(action.LocalPositionOffset()); @@ -1062,7 +1050,8 @@ namespace AzToolsFramework { AZStd::chrono::milliseconds timeNow; AzToolsFramework::ViewportInteraction::EditorViewportInputTimeNowRequestBus::BroadcastResult( - timeNow, &AzToolsFramework::ViewportInteraction::EditorViewportInputTimeNowRequestBus::Events::EditorViewportInputTimeNow); + timeNow, + &AzToolsFramework::ViewportInteraction::EditorViewportInputTimeNowRequestBus::Events::EditorViewportInputTimeNow); return timeNow; }); } @@ -1263,7 +1252,7 @@ namespace AzToolsFramework AZStd::unique_ptr translationManipulators = AZStd::make_unique( TranslationManipulators::Dimensions::Three, AZ::Transform::CreateIdentity(), AZ::Vector3::CreateOne()); - translationManipulators->SetLineBoundWidth(ManipulatorLineBoundWidth(ViewportUi::DefaultViewportId)); + translationManipulators->SetLineBoundWidth(ManipulatorLineBoundWidth()); InitializeManipulators(*translationManipulators); @@ -1297,7 +1286,7 @@ namespace AzToolsFramework ViewportInteraction::KeyboardModifiers prevModifiers{}; translationManipulators->InstallLinearManipulatorMouseMoveCallback( - [this, prevModifiers, manipulatorEntityIds](const LinearManipulator::Action& action) mutable -> void + [this, prevModifiers, manipulatorEntityIds](const LinearManipulator::Action& action) mutable { UpdateTranslationManipulator( action, manipulatorEntityIds->m_entityIds, m_entityIdManipulators, m_pivotOverrideFrame, prevModifiers, @@ -1331,7 +1320,7 @@ namespace AzToolsFramework }); translationManipulators->InstallPlanarManipulatorMouseMoveCallback( - [this, prevModifiers, manipulatorEntityIds](const PlanarManipulator::Action& action) mutable -> void + [this, prevModifiers, manipulatorEntityIds](const PlanarManipulator::Action& action) mutable { UpdateTranslationManipulator( action, manipulatorEntityIds->m_entityIds, m_entityIdManipulators, m_pivotOverrideFrame, prevModifiers, @@ -1364,7 +1353,7 @@ namespace AzToolsFramework }); translationManipulators->InstallSurfaceManipulatorMouseMoveCallback( - [this, prevModifiers, manipulatorEntityIds](const SurfaceManipulator::Action& action) mutable -> void + [this, prevModifiers, manipulatorEntityIds](const SurfaceManipulator::Action& action) mutable { UpdateTranslationManipulator( action, manipulatorEntityIds->m_entityIds, m_entityIdManipulators, m_pivotOverrideFrame, prevModifiers, @@ -1391,7 +1380,7 @@ namespace AzToolsFramework AZStd::unique_ptr rotationManipulators = AZStd::make_unique(AZ::Transform::CreateIdentity()); - rotationManipulators->SetCircleBoundWidth(ManipulatorCicleBoundWidth(ViewportUi::DefaultViewportId)); + rotationManipulators->SetCircleBoundWidth(ManipulatorCicleBoundWidth()); InitializeManipulators(*rotationManipulators); @@ -1415,7 +1404,7 @@ namespace AzToolsFramework AZStd::shared_ptr sharedRotationState = AZStd::make_shared(); rotationManipulators->InstallLeftMouseDownCallback( - [this, sharedRotationState]([[maybe_unused]] const AngularManipulator::Action& action) mutable -> void + [this, sharedRotationState]([[maybe_unused]] const AngularManipulator::Action& action) mutable { sharedRotationState->m_savedOrientation = AZ::Quaternion::CreateIdentity(); sharedRotationState->m_referenceFrameAtMouseDown = m_referenceFrame; @@ -1437,11 +1426,13 @@ namespace AzToolsFramework BeginRecordManipulatorCommand(); }); - ViewportInteraction::KeyboardModifiers prevModifiers{}; rotationManipulators->InstallMouseMoveCallback( - [this, prevModifiers, sharedRotationState](const AngularManipulator::Action& action) mutable -> void + [this, prevModifiers = ViewportInteraction::KeyboardModifiers(), + sharedRotationState](const AngularManipulator::Action& action) mutable { const ReferenceFrame referenceFrame = m_spaceCluster.m_spaceLock.value_or(ReferenceFrameFromModifiers(action.m_modifiers)); + const Influence influence = InfluenceFromModifiers(action.m_modifiers); + const AZ::Quaternion manipulatorOrientation = action.m_start.m_rotation * action.m_current.m_delta; // store the pivot override frame when positioning the manipulator manually (ctrl) // so we don't lose the orientation when adding/removing entities from the selection @@ -1452,9 +1443,7 @@ namespace AzToolsFramework // only update the manipulator orientation if we're rotating in a local reference frame or we're // manually modifying the manipulator orientation independent of the entity by holding ctrl - if ((sharedRotationState->m_referenceFrameAtMouseDown == ReferenceFrame::Local && - m_entityIdManipulators.m_lookups.size() == 1) || - action.m_modifiers.Ctrl()) + if (sharedRotationState->m_referenceFrameAtMouseDown == ReferenceFrame::Local || action.m_modifiers.Ctrl()) { m_entityIdManipulators.m_manipulators->SetLocalTransform(AZ::Transform::CreateFromQuaternionAndTranslation( manipulatorOrientation, m_entityIdManipulators.m_manipulators->GetLocalTransform().GetTranslation())); @@ -1463,20 +1452,24 @@ namespace AzToolsFramework // save state if we change the type of rotation we're doing to to prevent snapping if (prevModifiers != action.m_modifiers) { - UpdateInitialRotation(m_entityIdManipulators); + UpdateInitialTransform(m_entityIdManipulators); sharedRotationState->m_savedOrientation = action.m_current.m_delta.GetInverseFull(); } // allow the user to modify the orientation without moving the object if ctrl is held if (action.m_modifiers.Ctrl()) { - UpdateInitialRotation(m_entityIdManipulators); + UpdateInitialTransform(m_entityIdManipulators); sharedRotationState->m_savedOrientation = action.m_current.m_delta.GetInverseFull(); } else { - const auto pivotOrientation = ETCS::CalculateSelectionPivotOrientation( - m_entityIdManipulators.m_lookups, m_pivotOverrideFrame, ReferenceFrame::Parent); + // only update the pivot override if the orientation is being modified in local space and we have + // more than one entity selected (so rotating a single entity does not set the orientation override) + if (referenceFrame == ReferenceFrame::Local && sharedRotationState->m_entityIds.size() > 1) + { + m_pivotOverrideFrame.m_orientationOverride = manipulatorOrientation; + } // note: must use sorted entityIds based on hierarchy order when updating transforms for (AZ::EntityId entityId : sharedRotationState->m_entityIds) @@ -1492,9 +1485,9 @@ namespace AzToolsFramework const AZ::Transform offsetRotation = AZ::Transform::CreateFromQuaternion(sharedRotationState->m_savedOrientation * action.m_current.m_delta); - switch (referenceFrame) + switch (influence) { - case ReferenceFrame::Local: + case Influence::Individual: { const AZ::Quaternion rotation = entityIdLookupIt->second.m_initial.GetRotation().GetNormalized(); const AZ::Vector3 position = entityIdLookupIt->second.m_initial.GetTranslation(); @@ -1510,23 +1503,10 @@ namespace AzToolsFramework AZ::Transform::CreateTranslation(-centerOffset) * AZ::Transform::CreateUniformScale(scale)); } break; - case ReferenceFrame::Parent: + case Influence::Group: { const AZ::Transform pivotTransform = AZ::Transform::CreateFromQuaternionAndTranslation( - pivotOrientation.m_worldOrientation, - m_entityIdManipulators.m_manipulators->GetLocalTransform().GetTranslation()); - - const AZ::Transform transformInPivotSpace = - pivotTransform.GetInverse() * entityIdLookupIt->second.m_initial; - - SetEntityWorldTransform(entityId, pivotTransform * offsetRotation * transformInPivotSpace); - } - break; - case ReferenceFrame::World: - { - const AZ::Transform pivotTransform = AZ::Transform::CreateFromQuaternionAndTranslation( - AZ::Quaternion::CreateIdentity(), - m_entityIdManipulators.m_manipulators->GetLocalTransform().GetTranslation()); + manipulatorOrientation, m_entityIdManipulators.m_manipulators->GetLocalTransform().GetTranslation()); const AZ::Transform transformInPivotSpace = pivotTransform.GetInverse() * entityIdLookupIt->second.m_initial; @@ -1561,7 +1541,7 @@ namespace AzToolsFramework AZ_PROFILE_FUNCTION(AzToolsFramework); AZStd::unique_ptr scaleManipulators = AZStd::make_unique(AZ::Transform::CreateIdentity()); - scaleManipulators->SetLineBoundWidth(ManipulatorLineBoundWidth(ViewportUi::DefaultViewportId)); + scaleManipulators->SetLineBoundWidth(ManipulatorLineBoundWidth()); InitializeManipulators(*scaleManipulators); @@ -1571,13 +1551,20 @@ namespace AzToolsFramework scaleManipulators->SetAxes(AZ::Vector3::CreateAxisX(), AZ::Vector3::CreateAxisY(), AZ::Vector3::CreateAxisZ()); scaleManipulators->ConfigureView(2.0f, AZ::Color::CreateOne(), AZ::Color::CreateOne(), AZ::Color::CreateOne()); - // lambdas capture shared_ptr by value to increment ref count - auto manipulatorEntityIds = AZStd::make_shared(); - - auto uniformLeftMouseDownCallback = [this, manipulatorEntityIds]([[maybe_unused]] const LinearManipulator::Action& action) + struct SharedScaleState { + AZ::Vector3 m_savedScaleOffset = AZ::Vector3::CreateZero(); + EntityIdList m_entityIds; + }; + + // lambdas capture shared_ptr by value to increment ref count + auto sharedScaleState = AZStd::make_shared(); + + auto uniformLeftMouseDownCallback = [this, sharedScaleState]([[maybe_unused]] const LinearManipulator::Action& action) + { + sharedScaleState->m_savedScaleOffset = AZ::Vector3::CreateZero(); // important to sort entityIds based on hierarchy order when updating transforms - BuildSortedEntityIdVectorFromEntityIdMap(m_entityIdManipulators.m_lookups, manipulatorEntityIds->m_entityIds); + BuildSortedEntityIdVectorFromEntityIdMap(m_entityIdManipulators.m_lookups, sharedScaleState->m_entityIds); for (auto& entityIdLookup : m_entityIdManipulators.m_lookups) { @@ -1591,20 +1578,32 @@ namespace AzToolsFramework m_axisPreview.m_orientation = QuaternionFromTransformNoScaling(m_entityIdManipulators.m_manipulators->GetLocalTransform()); }; - auto uniformLeftMouseUpCallback = [this, manipulatorEntityIds]([[maybe_unused]] const LinearManipulator::Action& action) + auto uniformLeftMouseUpCallback = [this, sharedScaleState]([[maybe_unused]] const LinearManipulator::Action& action) { AzToolsFramework::EditorTransformChangeNotificationBus::Broadcast( - &AzToolsFramework::EditorTransformChangeNotificationBus::Events::OnEntityTransformChanged, - manipulatorEntityIds->m_entityIds); + &AzToolsFramework::EditorTransformChangeNotificationBus::Events::OnEntityTransformChanged, sharedScaleState->m_entityIds); m_entityIdManipulators.m_manipulators->SetLocalTransform(RecalculateAverageManipulatorTransform( m_entityIdManipulators.m_lookups, m_pivotOverrideFrame, m_pivotMode, m_referenceFrame)); }; - auto uniformLeftMouseMoveCallback = [this, manipulatorEntityIds](const LinearManipulator::Action& action) + auto uniformLeftMouseMoveCallback = [this, sharedScaleState, prevModifiers = ViewportInteraction::KeyboardModifiers()]( + const LinearManipulator::Action& action) mutable { + // do nothing to modify the manipulator + if (action.m_modifiers.Ctrl()) + { + return; + } + + if (prevModifiers != action.m_modifiers) + { + UpdateInitialTransform(m_entityIdManipulators); + sharedScaleState->m_savedScaleOffset = action.LocalScaleOffset(); + } + // note: must use sorted entityIds based on hierarchy order when updating transforms - for (AZ::EntityId entityId : manipulatorEntityIds->m_entityIds) + for (AZ::EntityId entityId : sharedScaleState->m_entityIds) { auto entityIdLookupIt = m_entityIdManipulators.m_lookups.find(entityId); if (entityIdLookupIt == m_entityIdManipulators.m_lookups.end()) @@ -1620,26 +1619,33 @@ namespace AzToolsFramework return vec.GetX() + vec.GetY() + vec.GetZ(); }; - const float uniformScale = action.m_start.m_sign * sumVectorElements(action.LocalScaleOffset()); + const float uniformScale = + action.m_start.m_sign * sumVectorElements(action.LocalScaleOffset() - sharedScaleState->m_savedScaleOffset); const float scale = AZ::GetClamp(1.0f + uniformScale / initialScale, AZ::MinTransformScale, AZ::MaxTransformScale); const AZ::Transform scaleTransform = AZ::Transform::CreateUniformScale(scale); - if (action.m_modifiers.Alt()) + switch (InfluenceFromModifiers(action.m_modifiers)) { - const AZ::Transform pivotTransform = TransformNormalizedScale(entityIdLookupIt->second.m_initial); - const AZ::Transform transformInPivotSpace = pivotTransform.GetInverse() * initial; + case Influence::Individual: + { + const AZ::Transform pivotTransform = TransformNormalizedScale(entityIdLookupIt->second.m_initial); + const AZ::Transform transformInPivotSpace = pivotTransform.GetInverse() * initial; - SetEntityWorldTransform(entityId, pivotTransform * scaleTransform * transformInPivotSpace); - } - else - { - const AZ::Transform pivotTransform = - TransformNormalizedScale(m_entityIdManipulators.m_manipulators->GetLocalTransform()); - const AZ::Transform transformInPivotSpace = pivotTransform.GetInverse() * initial; + SetEntityWorldTransform(entityId, pivotTransform * scaleTransform * transformInPivotSpace); + } + break; + case Influence::Group: + { + const AZ::Transform pivotTransform = + TransformNormalizedScale(m_entityIdManipulators.m_manipulators->GetLocalTransform()); + const AZ::Transform transformInPivotSpace = pivotTransform.GetInverse() * initial; - SetEntityWorldTransform(entityId, pivotTransform * scaleTransform * transformInPivotSpace); + SetEntityWorldTransform(entityId, pivotTransform * scaleTransform * transformInPivotSpace); + } } } + + prevModifiers = action.m_modifiers; }; scaleManipulators->InstallAxisLeftMouseDownCallback(uniformLeftMouseDownCallback); @@ -2729,8 +2735,7 @@ namespace AzToolsFramework if (m_pivotOverrideFrame.m_orientationOverride && m_entityIdManipulators.m_manipulators) { - m_pivotOverrideFrame.m_orientationOverride = - QuaternionFromTransformNoScaling(m_entityIdManipulators.m_manipulators->GetLocalTransform()); + m_pivotOverrideFrame.m_orientationOverride = m_entityIdManipulators.m_manipulators->GetLocalTransform().GetRotation(); } if (m_pivotOverrideFrame.m_translationOverride && m_entityIdManipulators.m_manipulators) @@ -3337,7 +3342,7 @@ namespace AzToolsFramework display.SetLineWidth(4.0f); - const auto axisFlip = [&transform, &cameraState](const AZ::Vector3& axis) -> float + const auto axisFlip = [&transform, &cameraState](const AZ::Vector3& axis) { return ShouldFlipCameraAxis( AZ::Transform::CreateIdentity(), transform.GetTranslation(), TransformDirectionNoScaling(transform, axis), @@ -3554,7 +3559,7 @@ namespace AzToolsFramework // screen space const auto calculateGizmoAxis = [&cameraView, &cameraProjection, &screenOffset](const AZ::Vector3& axis) { - auto result = AZ::Vector2(AzFramework::WorldToScreenNDC(axis, cameraView, cameraProjection)); + auto result = AZ::Vector2(AzFramework::WorldToScreenNdc(axis, cameraView, cameraProjection)); result.SetY(1.0f - result.GetY()); return result + screenOffset; }; diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.h b/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.h index 6cc89b2d10..a0e87d9d6f 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/ViewportSelection/EditorTransformComponentSelection.h @@ -96,6 +96,14 @@ namespace AzToolsFramework AZ::u8 m_pickTypes = PickType::None; //!< What mode(s) were we in when picking an EntityId override. }; + //! How a manipulator should treat an adjustment. + //! @note Determines if a transform is applied to an individual entity or the whole group. + enum class Influence + { + Group, + Individual + }; + //! What frame/space is the manipulator currently operating in. enum class ReferenceFrame { @@ -328,7 +336,8 @@ namespace AzToolsFramework OptionalFrame m_pivotOverrideFrame; //!< Has a pivot override been set. Mode m_mode = Mode::Translation; //!< Manipulator mode - default to translation. Pivot m_pivotMode = Pivot::Object; //!< Entity pivot mode - default to object (authored root). - ReferenceFrame m_referenceFrame = ReferenceFrame::Parent; //!< What reference frame is the Manipulator currently operating in. + ReferenceFrame m_referenceFrame = ReferenceFrame::Local; //!< What reference frame is the Manipulator currently operating in. + Influence m_influence = Influence::Group; //!< What sphere of influence does the Manipulator have. Frame m_axisPreview; //!< Axes of entity at the time of mouse down to indicate delta of translation. bool m_triedToRefresh = false; //!< Did a refresh event occur to recalculate the current Manipulator transform. //! Was EditorTransformComponentSelection responsible for the most recent entity selection change. diff --git a/Code/Framework/AzToolsFramework/Tests/EditorTransformComponentSelectionTests.cpp b/Code/Framework/AzToolsFramework/Tests/EditorTransformComponentSelectionTests.cpp index 449c4549be..52470f41e3 100644 --- a/Code/Framework/AzToolsFramework/Tests/EditorTransformComponentSelectionTests.cpp +++ b/Code/Framework/AzToolsFramework/Tests/EditorTransformComponentSelectionTests.cpp @@ -242,11 +242,11 @@ namespace UnitTest { // the initial starting position of the entities AZ::TransformBus::Event( - m_entityId1, &AZ::TransformBus::Events::SetWorldTM, AZ::Transform::CreateTranslation(m_entity1WorldTranslation)); + m_entityId1, &AZ::TransformBus::Events::SetWorldTM, AZ::Transform::CreateTranslation(Entity1WorldTranslation)); AZ::TransformBus::Event( - m_entityId2, &AZ::TransformBus::Events::SetWorldTM, AZ::Transform::CreateTranslation(m_entity2WorldTranslation)); + m_entityId2, &AZ::TransformBus::Events::SetWorldTM, AZ::Transform::CreateTranslation(Entity2WorldTranslation)); AZ::TransformBus::Event( - m_entityId3, &AZ::TransformBus::Events::SetWorldTM, AZ::Transform::CreateTranslation(m_entity3WorldTranslation)); + m_entityId3, &AZ::TransformBus::Events::SetWorldTM, AZ::Transform::CreateTranslation(Entity3WorldTranslation)); } static void PositionCamera(AzFramework::CameraState& cameraState) @@ -261,9 +261,10 @@ namespace UnitTest AZ::EntityId m_entityId1; AZ::EntityId m_entityId2; AZ::EntityId m_entityId3; - AZ::Vector3 m_entity1WorldTranslation = AZ::Vector3(5.0f, 15.0f, 10.0f); - AZ::Vector3 m_entity2WorldTranslation = AZ::Vector3(5.0f, 14.0f, 10.0f); - AZ::Vector3 m_entity3WorldTranslation = AZ::Vector3(5.0f, 16.0f, 10.0f); + + static inline const AZ::Vector3 Entity1WorldTranslation = AZ::Vector3(5.0f, 15.0f, 10.0f); + static inline const AZ::Vector3 Entity2WorldTranslation = AZ::Vector3(5.0f, 14.0f, 10.0f); + static inline const AZ::Vector3 Entity3WorldTranslation = AZ::Vector3(5.0f, 16.0f, 10.0f); }; void ArrangeIndividualRotatedEntitySelection(const AzToolsFramework::EntityIdList& entityIds, const AZ::Quaternion& orientation) @@ -371,16 +372,16 @@ namespace UnitTest // Given AzToolsFramework::SelectEntity(m_entityId1); - ArrangeIndividualRotatedEntitySelection(m_entityIds, AZ::Quaternion::CreateRotationX(AZ::DegToRad(90.0f))); + const auto entityTransform = AZ::Transform::CreateFromQuaternion(AZ::Quaternion::CreateRotationX(AZ::DegToRad(90.0f))); + ArrangeIndividualRotatedEntitySelection(m_entityIds, entityTransform.GetRotation()); RefreshManipulators(EditorTransformComponentSelectionRequestBus::Events::RefreshType::All); SetTransformMode(EditorTransformComponentSelectionRequestBus::Events::Mode::Rotation); const AZ::Transform manipulatorTransformBefore = GetManipulatorTransform().value_or(AZ::Transform::CreateIdentity()); - // check preconditions - manipulator transform matches parent/world transform (identity) - EXPECT_THAT(manipulatorTransformBefore.GetBasisY(), IsClose(AZ::Vector3::CreateAxisY())); - EXPECT_THAT(manipulatorTransformBefore.GetBasisZ(), IsClose(AZ::Vector3::CreateAxisZ())); + // check preconditions - manipulator transform matches the entity transform + EXPECT_THAT(manipulatorTransformBefore, IsClose(entityTransform)); /////////////////////////////////////////////////////////////////////////////////////////////////////////////// /////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -624,7 +625,7 @@ namespace UnitTest EXPECT_TRUE(selectedEntitiesBefore.empty()); // calculate the position in screen space of the initial entity position - const auto entity1ScreenPosition = AzFramework::WorldToScreen(m_entity1WorldTranslation, m_cameraState); + const auto entity1ScreenPosition = AzFramework::WorldToScreen(Entity1WorldTranslation, m_cameraState); // click the entity in the viewport m_actionDispatcher->SetStickySelect(true) @@ -649,7 +650,7 @@ namespace UnitTest EXPECT_TRUE(selectedEntitiesBefore.empty()); // calculate the position in screen space of the initial entity position - const auto entity1ScreenPosition = AzFramework::WorldToScreen(m_entity1WorldTranslation, m_cameraState); + const auto entity1ScreenPosition = AzFramework::WorldToScreen(Entity1WorldTranslation, m_cameraState); // click the entity in the viewport m_actionDispatcher->SetStickySelect(false) @@ -728,7 +729,7 @@ namespace UnitTest AzToolsFramework::SelectEntity(m_entityId1); // calculate the position in screen space of the second entity - const auto entity2ScreenPosition = AzFramework::WorldToScreen(m_entity2WorldTranslation, m_cameraState); + const auto entity2ScreenPosition = AzFramework::WorldToScreen(Entity2WorldTranslation, m_cameraState); // click the entity in the viewport m_actionDispatcher->SetStickySelect(true) @@ -754,7 +755,7 @@ namespace UnitTest AzToolsFramework::SelectEntity(m_entityId1); // calculate the position in screen space of the second entity - const auto entity2ScreenPosition = AzFramework::WorldToScreen(m_entity2WorldTranslation, m_cameraState); + const auto entity2ScreenPosition = AzFramework::WorldToScreen(Entity2WorldTranslation, m_cameraState); // click the entity in the viewport m_actionDispatcher->SetStickySelect(false) @@ -780,7 +781,7 @@ namespace UnitTest AzToolsFramework::SelectEntity(m_entityId1); // calculate the position in screen space of the second entity - const auto entity2ScreenPosition = AzFramework::WorldToScreen(m_entity2WorldTranslation, m_cameraState); + const auto entity2ScreenPosition = AzFramework::WorldToScreen(Entity2WorldTranslation, m_cameraState); // click the entity in the viewport m_actionDispatcher->SetStickySelect(true) @@ -806,7 +807,7 @@ namespace UnitTest AzToolsFramework::SelectEntity(m_entityId1); // calculate the position in screen space of the second entity - const auto entity2ScreenPosition = AzFramework::WorldToScreen(m_entity2WorldTranslation, m_cameraState); + const auto entity2ScreenPosition = AzFramework::WorldToScreen(Entity2WorldTranslation, m_cameraState); // click the entity in the viewport m_actionDispatcher->SetStickySelect(false) @@ -832,7 +833,7 @@ namespace UnitTest AzToolsFramework::SelectEntities({ m_entityId1, m_entityId2 }); // calculate the position in screen space of the second entity - const auto entity2ScreenPosition = AzFramework::WorldToScreen(m_entity2WorldTranslation, m_cameraState); + const auto entity2ScreenPosition = AzFramework::WorldToScreen(Entity2WorldTranslation, m_cameraState); // click the entity in the viewport m_actionDispatcher->SetStickySelect(true) @@ -858,7 +859,7 @@ namespace UnitTest AzToolsFramework::SelectEntities({ m_entityId1, m_entityId2 }); // calculate the position in screen space of the second entity - const auto entity2ScreenPosition = AzFramework::WorldToScreen(m_entity2WorldTranslation, m_cameraState); + const auto entity2ScreenPosition = AzFramework::WorldToScreen(Entity2WorldTranslation, m_cameraState); // click the entity in the viewport m_actionDispatcher->SetStickySelect(false) @@ -1001,7 +1002,7 @@ namespace UnitTest AzToolsFramework::SelectEntity(m_entityId1); // calculate the position in screen space of the second entity - const auto entity2ScreenPosition = AzFramework::WorldToScreen(m_entity2WorldTranslation, m_cameraState); + const auto entity2ScreenPosition = AzFramework::WorldToScreen(Entity2WorldTranslation, m_cameraState); // single click select entity2 m_actionDispatcher->SetStickySelect(false) @@ -1035,7 +1036,7 @@ namespace UnitTest AzToolsFramework::SelectEntity(m_entityId1); // calculate the position in screen space of the second entity - const auto entity2ScreenPosition = AzFramework::WorldToScreen(m_entity2WorldTranslation, m_cameraState); + const auto entity2ScreenPosition = AzFramework::WorldToScreen(Entity2WorldTranslation, m_cameraState); // single click select entity2 m_actionDispatcher->SetStickySelect(GetParam()) @@ -1056,7 +1057,7 @@ namespace UnitTest manipulatorTransform, AzToolsFramework::GetEntityContextId(), &AzToolsFramework::EditorTransformComponentSelectionRequestBus::Events::GetManipulatorTransform); - EXPECT_THAT(manipulatorTransform->GetTranslation(), IsClose(m_entity2WorldTranslation)); + EXPECT_THAT(manipulatorTransform->GetTranslation(), IsClose(Entity2WorldTranslation)); } TEST_P( @@ -1069,7 +1070,7 @@ namespace UnitTest AzToolsFramework::SelectEntity(m_entityId1); // calculate the position in screen space of the second entity - const auto entity2ScreenPosition = AzFramework::WorldToScreen(m_entity2WorldTranslation, m_cameraState); + const auto entity2ScreenPosition = AzFramework::WorldToScreen(Entity2WorldTranslation, m_cameraState); // position in space above the entities const auto clickOffPositionWorld = AZ::Vector3(5.0f, 15.0f, 12.0f); @@ -1096,7 +1097,7 @@ namespace UnitTest manipulatorTransform, AzToolsFramework::GetEntityContextId(), &AzToolsFramework::EditorTransformComponentSelectionRequestBus::Events::GetManipulatorTransform); - EXPECT_THAT(manipulatorTransform->GetTranslation(), IsClose(m_entity2WorldTranslation)); + EXPECT_THAT(manipulatorTransform->GetTranslation(), IsClose(Entity2WorldTranslation)); }) ->MousePosition(clickOffPositionScreen) ->KeyboardModifierDown(AzToolsFramework::ViewportInteraction::KeyboardModifier::Control) @@ -1113,11 +1114,560 @@ namespace UnitTest &AzToolsFramework::EditorTransformComponentSelectionRequestBus::Events::GetManipulatorTransform); // manipulator transform is reset - EXPECT_THAT(manipulatorTransform->GetTranslation(), IsClose(m_entity1WorldTranslation)); + EXPECT_THAT(manipulatorTransform->GetTranslation(), IsClose(Entity1WorldTranslation)); } INSTANTIATE_TEST_CASE_P(All, EditorTransformComponentSelectionViewportPickingManipulatorTestFixtureParam, testing::Values(true, false)); + // create alias for EditorTransformComponentSelectionViewportPickingManipulatorTestFixture to help group tests + using EditorTransformComponentSelectionManipulatorInteractionTestFixture = + EditorTransformComponentSelectionViewportPickingManipulatorTestFixture; + + // type to group related inputs and outcomes for parameterized tests (single entity) + struct ManipulatorOptionsSingle + { + AzToolsFramework::ViewportInteraction::KeyboardModifier m_keyboardModifier; + AZ::Transform m_expectedManipulatorTransformAfter; + AZ::Transform m_expectedEntityTransformAfter; + }; + + class EditorTransformComponentSelectionRotationManipulatorSingleEntityTestFixtureParam + : public EditorTransformComponentSelectionManipulatorInteractionTestFixture + , public ::testing::WithParamInterface + { + }; + + TEST_P( + EditorTransformComponentSelectionRotationManipulatorSingleEntityTestFixtureParam, + RotatingASingleEntityWithDifferentModifierCombinations) + { + using AzToolsFramework::EditorTransformComponentSelectionRequestBus; + + PositionEntities(); + PositionCamera(m_cameraState); + + SetTransformMode(EditorTransformComponentSelectionRequestBus::Events::Mode::Rotation); + + AzToolsFramework::SelectEntity(m_entityId1); + + const float screenToWorldMultiplier = AzToolsFramework::CalculateScreenToWorldMultiplier(Entity1WorldTranslation, m_cameraState); + const float manipulatorRadius = 2.0f * screenToWorldMultiplier; + + const auto rotationManipulatorStartHoldWorldPosition = Entity1WorldTranslation + + AZ::Quaternion::CreateRotationX(AZ::DegToRad(-45.0f)).TransformVector(AZ::Vector3::CreateAxisY(-manipulatorRadius)); + const auto rotationManipulatorEndHoldWorldPosition = Entity1WorldTranslation + + AZ::Quaternion::CreateRotationX(AZ::DegToRad(-135.0f)).TransformVector(AZ::Vector3::CreateAxisY(-manipulatorRadius)); + + // calculate screen space positions + const auto rotationManipulatorHoldScreenPosition = + AzFramework::WorldToScreen(rotationManipulatorStartHoldWorldPosition, m_cameraState); + const auto rotationManipulatorEndHoldScreenPosition = + AzFramework::WorldToScreen(rotationManipulatorEndHoldWorldPosition, m_cameraState); + + m_actionDispatcher->CameraState(m_cameraState) + ->MousePosition(rotationManipulatorHoldScreenPosition) + ->KeyboardModifierDown(GetParam().m_keyboardModifier) + ->MouseLButtonDown() + ->MousePosition(rotationManipulatorEndHoldScreenPosition) + ->MouseLButtonUp(); + + const auto expectedEntityTransform = GetParam().m_expectedEntityTransformAfter; + const auto expectedManipulatorTransform = GetParam().m_expectedManipulatorTransformAfter; + + const auto manipulatorTransform = GetManipulatorTransform(); + const auto entityTransform = AzToolsFramework::GetWorldTransform(m_entityId1); + + EXPECT_THAT(*manipulatorTransform, IsClose(expectedManipulatorTransform)); + EXPECT_THAT(entityTransform, IsClose(expectedEntityTransform)); + } + + static const AZ::Transform ExpectedTransformAfterLocalRotationManipulatorMotion = AZ::Transform::CreateFromQuaternionAndTranslation( + AZ::Quaternion::CreateRotationX(AZ::DegToRad(-90.0f)), + EditorTransformComponentSelectionViewportPickingFixture::Entity1WorldTranslation); + + INSTANTIATE_TEST_CASE_P( + All, + EditorTransformComponentSelectionRotationManipulatorSingleEntityTestFixtureParam, + testing::Values( + // this replicates rotating an entity in local space with no modifiers held + // manipulator and entity rotate + ManipulatorOptionsSingle{ AzToolsFramework::ViewportInteraction::KeyboardModifier::None, + ExpectedTransformAfterLocalRotationManipulatorMotion, + ExpectedTransformAfterLocalRotationManipulatorMotion }, + // this replicates rotating an entity in local space with the alt modifier held + // manipulator and entity rotate + ManipulatorOptionsSingle{ AzToolsFramework::ViewportInteraction::KeyboardModifier::Alt, + ExpectedTransformAfterLocalRotationManipulatorMotion, + ExpectedTransformAfterLocalRotationManipulatorMotion }, + // this replicates rotating an entity in world space with the shift modifier held + // entity rotates, manipulator remains aligned to world + ManipulatorOptionsSingle{ + AzToolsFramework::ViewportInteraction::KeyboardModifier::Shift, + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity1WorldTranslation), + ExpectedTransformAfterLocalRotationManipulatorMotion }, + // this replicates rotating the manipulator in local space with the ctrl modifier held (entity is unchanged) + ManipulatorOptionsSingle{ + AzToolsFramework::ViewportInteraction::KeyboardModifier::Ctrl, ExpectedTransformAfterLocalRotationManipulatorMotion, + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity1WorldTranslation) })); + + // type to group related inputs and outcomes for parameterized tests (two entities) + struct ManipulatorOptionsMultiple + { + AzToolsFramework::ViewportInteraction::KeyboardModifier m_keyboardModifier; + AZ::Transform m_expectedManipulatorTransformAfter; + AZ::Transform m_firstExpectedEntityTransformAfter; + AZ::Transform m_secondExpectedEntityTransformAfter; + }; + + class EditorTransformComponentSelectionRotationManipulatorMultipleEntityTestFixtureParam + : public EditorTransformComponentSelectionManipulatorInteractionTestFixture + , public ::testing::WithParamInterface + { + }; + + TEST_P( + EditorTransformComponentSelectionRotationManipulatorMultipleEntityTestFixtureParam, + RotatingMultipleEntitiesWithDifferentModifierCombinations) + { + using AzToolsFramework::EditorTransformComponentSelectionRequestBus; + + PositionEntities(); + PositionCamera(m_cameraState); + + SetTransformMode(EditorTransformComponentSelectionRequestBus::Events::Mode::Rotation); + + AzToolsFramework::SelectEntities({ m_entityId2, m_entityId3 }); + + // manipulator should be centered between the two entities + const auto initialManipulatorTransform = GetManipulatorTransform(); + + const float screenToWorldMultiplier = + AzToolsFramework::CalculateScreenToWorldMultiplier(initialManipulatorTransform->GetTranslation(), m_cameraState); + const float manipulatorRadius = 2.0f * screenToWorldMultiplier; + + const auto rotationManipulatorStartHoldWorldPosition = initialManipulatorTransform->GetTranslation() + + AZ::Quaternion::CreateRotationX(AZ::DegToRad(-45.0f)).TransformVector(AZ::Vector3::CreateAxisY(-manipulatorRadius)); + const auto rotationManipulatorEndHoldWorldPosition = initialManipulatorTransform->GetTranslation() + + AZ::Quaternion::CreateRotationX(AZ::DegToRad(-135.0f)).TransformVector(AZ::Vector3::CreateAxisY(-manipulatorRadius)); + + // calculate screen space positions + const auto rotationManipulatorHoldScreenPosition = + AzFramework::WorldToScreen(rotationManipulatorStartHoldWorldPosition, m_cameraState); + const auto rotationManipulatorEndHoldScreenPosition = + AzFramework::WorldToScreen(rotationManipulatorEndHoldWorldPosition, m_cameraState); + + m_actionDispatcher->CameraState(m_cameraState) + ->MousePosition(rotationManipulatorHoldScreenPosition) + ->KeyboardModifierDown(GetParam().m_keyboardModifier) + ->MouseLButtonDown() + ->MousePosition(rotationManipulatorEndHoldScreenPosition) + ->MouseLButtonUp(); + + const auto expectedEntity2Transform = GetParam().m_firstExpectedEntityTransformAfter; + const auto expectedEntity3Transform = GetParam().m_secondExpectedEntityTransformAfter; + const auto expectedManipulatorTransform = GetParam().m_expectedManipulatorTransformAfter; + + const auto manipulatorTransformAfter = GetManipulatorTransform(); + const auto entity2Transform = AzToolsFramework::GetWorldTransform(m_entityId2); + const auto entity3Transform = AzToolsFramework::GetWorldTransform(m_entityId3); + + EXPECT_THAT(*manipulatorTransformAfter, IsClose(expectedManipulatorTransform)); + EXPECT_THAT(entity2Transform, IsClose(expectedEntity2Transform)); + EXPECT_THAT(entity3Transform, IsClose(expectedEntity3Transform)); + } + + // note: The aggregate manipulator position will be the average of entity 2 and 3 combined which + // winds up being the same as entity 1 + static const AZ::Vector3 AggregateManipulatorPositionWithEntity2and3Selected = + EditorTransformComponentSelectionViewportPickingFixture::Entity1WorldTranslation; + + static const AZ::Transform ExpectedEntity2TransformAfterLocalGroupRotationManipulatorMotion = + AZ::Transform::CreateTranslation(AggregateManipulatorPositionWithEntity2and3Selected) * + AZ::Transform::CreateFromQuaternion(AZ::Quaternion::CreateRotationX(AZ::DegToRad(-90.0f))) * + AZ::Transform::CreateTranslation(AZ::Vector3::CreateAxisY(-1.0f)); + static const AZ::Transform ExpectedEntity3TransformAfterLocalGroupRotationManipulatorMotion = + AZ::Transform::CreateTranslation(AggregateManipulatorPositionWithEntity2and3Selected) * + AZ::Transform::CreateFromQuaternion(AZ::Quaternion::CreateRotationX(AZ::DegToRad(-90.0f))) * + AZ::Transform::CreateTranslation(AZ::Vector3::CreateAxisY(1.0f)); + static const AZ::Transform ExpectedEntity2TransformAfterLocalIndividualRotationManipulatorMotion = + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity2WorldTranslation) * + AZ::Transform::CreateFromQuaternion(AZ::Quaternion::CreateRotationX(AZ::DegToRad(-90.0f))); + static const AZ::Transform ExpectedEntity3TransformAfterLocalIndividualRotationManipulatorMotion = + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity3WorldTranslation) * + AZ::Transform::CreateFromQuaternion(AZ::Quaternion::CreateRotationX(AZ::DegToRad(-90.0f))); + + INSTANTIATE_TEST_CASE_P( + All, + EditorTransformComponentSelectionRotationManipulatorMultipleEntityTestFixtureParam, + testing::Values( + // this replicates rotating a group of entities in local space with no modifiers held + // manipulator and entity rotate + ManipulatorOptionsMultiple{ AzToolsFramework::ViewportInteraction::KeyboardModifier::None, + ExpectedTransformAfterLocalRotationManipulatorMotion, + ExpectedEntity2TransformAfterLocalGroupRotationManipulatorMotion, + ExpectedEntity3TransformAfterLocalGroupRotationManipulatorMotion }, + // this replicates rotating a group of entities in local space with the alt modifier held + // manipulator and entity rotate + ManipulatorOptionsMultiple{ AzToolsFramework::ViewportInteraction::KeyboardModifier::Alt, + ExpectedTransformAfterLocalRotationManipulatorMotion, + ExpectedEntity2TransformAfterLocalIndividualRotationManipulatorMotion, + ExpectedEntity3TransformAfterLocalIndividualRotationManipulatorMotion }, + // this replicates rotating a group of entities in world space with the shift modifier held + // entity rotates, manipulator remains aligned to world + ManipulatorOptionsMultiple{ + AzToolsFramework::ViewportInteraction::KeyboardModifier::Shift, + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity1WorldTranslation), + ExpectedEntity2TransformAfterLocalGroupRotationManipulatorMotion, + ExpectedEntity3TransformAfterLocalGroupRotationManipulatorMotion }, + // this replicates rotating the manipulator in local space with the ctrl modifier held (entity is unchanged) + ManipulatorOptionsMultiple{ + AzToolsFramework::ViewportInteraction::KeyboardModifier::Ctrl, ExpectedTransformAfterLocalRotationManipulatorMotion, + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity2WorldTranslation), + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity3WorldTranslation) })); + + class EditorTransformComponentSelectionTranslationManipulatorSingleEntityTestFixtureParam + : public EditorTransformComponentSelectionManipulatorInteractionTestFixture + , public ::testing::WithParamInterface + { + }; + + static const float LinearManipulatorYAxisMovement = -3.0f; + static const float LinearManipulatorZAxisMovement = 2.0f; + + TEST_P( + EditorTransformComponentSelectionTranslationManipulatorSingleEntityTestFixtureParam, + TranslatingASingleEntityWithDifferentModifierCombinations) + { + using AzToolsFramework::EditorTransformComponentSelectionRequestBus; + + PositionEntities(); + + // move camera up and to the left so it's just above the normal row of entities + AzFramework::SetCameraTransform( + m_cameraState, + AZ::Transform::CreateFromQuaternionAndTranslation( + AZ::Quaternion::CreateFromEulerAnglesDegrees(AZ::Vector3(0.0f, 0.0f, 90.0f)), AZ::Vector3(10.0f, 14.5, 11.0f))); + + SetTransformMode(EditorTransformComponentSelectionRequestBus::Events::Mode::Translation); + + AzToolsFramework::SelectEntity(m_entityId1); + const auto entity1Transform = AzToolsFramework::GetWorldTransform(m_entityId1); + + const float screenToWorldMultiplier = AzToolsFramework::CalculateScreenToWorldMultiplier( + AzToolsFramework::GetWorldTransform(m_entityId1).GetTranslation(), m_cameraState); + + // calculate positions for two click and drag motions (moving a linear manipulator) + // begin each click in the center of the line of the linear manipulators + const auto translationManipulatorStartHoldWorldPosition1 = + AzToolsFramework::GetWorldTransform(m_entityId1).GetTranslation() + entity1Transform.GetBasisZ() * screenToWorldMultiplier; + const auto translationManipulatorEndHoldWorldPosition1 = + translationManipulatorStartHoldWorldPosition1 + AZ::Vector3::CreateAxisZ(LinearManipulatorZAxisMovement); + const auto translationManipulatorStartHoldWorldPosition2 = AzToolsFramework::GetWorldTransform(m_entityId1).GetTranslation() + + AZ::Vector3::CreateAxisZ(LinearManipulatorZAxisMovement) - entity1Transform.GetBasisY() * screenToWorldMultiplier; + const auto translationManipulatorEndHoldWorldPosition2 = + translationManipulatorStartHoldWorldPosition2 + AZ::Vector3::CreateAxisY(LinearManipulatorYAxisMovement); + + // transform to screen space + const auto translationManipulatorStartHoldScreenPosition1 = + AzFramework::WorldToScreen(translationManipulatorStartHoldWorldPosition1, m_cameraState); + const auto translationManipulatorEndHoldScreenPosition1 = + AzFramework::WorldToScreen(translationManipulatorEndHoldWorldPosition1, m_cameraState); + const auto translationManipulatorStartHoldScreenPosition2 = + AzFramework::WorldToScreen(translationManipulatorStartHoldWorldPosition2, m_cameraState); + const auto translationManipulatorEndHoldScreenPosition2 = + AzFramework::WorldToScreen(translationManipulatorEndHoldWorldPosition2, m_cameraState); + + m_actionDispatcher->CameraState(m_cameraState) + ->MousePosition(translationManipulatorStartHoldScreenPosition1) + ->KeyboardModifierDown(GetParam().m_keyboardModifier) + ->MouseLButtonDown() + ->MousePosition(translationManipulatorEndHoldScreenPosition1) + ->MouseLButtonUp() + ->MousePosition(translationManipulatorStartHoldScreenPosition2) + ->MouseLButtonDown() + ->MousePosition(translationManipulatorEndHoldScreenPosition2) + ->MouseLButtonUp(); + + const auto expectedEntityTransform = GetParam().m_expectedEntityTransformAfter; + const auto expectedManipulatorTransform = GetParam().m_expectedManipulatorTransformAfter; + + const auto manipulatorTransform = GetManipulatorTransform(); + const auto entityTransform = AzToolsFramework::GetWorldTransform(m_entityId1); + + EXPECT_THAT(*manipulatorTransform, IsCloseTolerance(expectedManipulatorTransform, 0.01f)); + EXPECT_THAT(entityTransform, IsCloseTolerance(expectedEntityTransform, 0.01f)); + } + + static const AZ::Transform ExpectedTransformAfterLocalTranslationManipulatorMotion = AZ::Transform::CreateTranslation( + EditorTransformComponentSelectionViewportPickingFixture::Entity1WorldTranslation + + AZ::Vector3(0.0f, LinearManipulatorYAxisMovement, LinearManipulatorZAxisMovement)); + + // where the manipulator should end up after the input from TranslatingMultipleEntitiesWithDifferentModifierCombinations + static const AZ::Transform ExpectedManipulatorTransformAfterGroupTranslationManipulatorMotion = AZ::Transform::CreateTranslation( + AggregateManipulatorPositionWithEntity2and3Selected + + AZ::Vector3(0.0f, LinearManipulatorYAxisMovement, LinearManipulatorZAxisMovement)); + + INSTANTIATE_TEST_CASE_P( + All, + EditorTransformComponentSelectionTranslationManipulatorSingleEntityTestFixtureParam, + testing::Values( + // this replicates translating an entity in local space with no modifiers held + // manipulator and entity translate + ManipulatorOptionsSingle{ AzToolsFramework::ViewportInteraction::KeyboardModifier::None, + ExpectedTransformAfterLocalTranslationManipulatorMotion, + ExpectedTransformAfterLocalTranslationManipulatorMotion }, + // this replicates translating an entity in local space with the alt modifier held + // manipulator and entity translate (to the user, equivalent to no modifiers with one entity selected) + ManipulatorOptionsSingle{ AzToolsFramework::ViewportInteraction::KeyboardModifier::Alt, + ExpectedTransformAfterLocalTranslationManipulatorMotion, + ExpectedTransformAfterLocalTranslationManipulatorMotion }, + // this replicates translating an entity in world space with the shift modifier held + // manipulator and entity translate + ManipulatorOptionsSingle{ AzToolsFramework::ViewportInteraction::KeyboardModifier::Shift, + ExpectedTransformAfterLocalTranslationManipulatorMotion, + ExpectedTransformAfterLocalTranslationManipulatorMotion }, + // this replicates translating the manipulator in local space with the ctrl modifier held + // entity is unchanged, manipulator moves + ManipulatorOptionsSingle{ + AzToolsFramework::ViewportInteraction::KeyboardModifier::Ctrl, ExpectedTransformAfterLocalTranslationManipulatorMotion, + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity1WorldTranslation) })); + + class EditorTransformComponentSelectionTranslationManipulatorMultipleEntityTestFixtureParam + : public EditorTransformComponentSelectionManipulatorInteractionTestFixture + , public ::testing::WithParamInterface + { + }; + + static const AZ::Transform Entity2RotationForLocalTranslation = + AZ::Transform::CreateFromQuaternion(AZ::Quaternion::CreateRotationZ(AZ::DegToRad(90.0f))); + + TEST_P( + EditorTransformComponentSelectionTranslationManipulatorMultipleEntityTestFixtureParam, + TranslatingMultipleEntitiesWithDifferentModifierCombinations) + { + using AzToolsFramework::EditorTransformComponentSelectionRequestBus; + + PositionEntities(); + + // move camera up and to the left so it's just above the normal row of entities + AzFramework::SetCameraTransform( + m_cameraState, + AZ::Transform::CreateFromQuaternionAndTranslation( + AZ::Quaternion::CreateFromEulerAnglesDegrees(AZ::Vector3(0.0f, 0.0f, 90.0f)), AZ::Vector3(10.0f, 14.5, 11.0f))); + + SetTransformMode(EditorTransformComponentSelectionRequestBus::Events::Mode::Translation); + + // give entity 2 a different orientation to entity 3 so when moving in local space their translation vectors will be different + AZ::TransformBus::Event( + m_entityId2, &AZ::TransformBus::Events::SetWorldRotationQuaternion, Entity2RotationForLocalTranslation.GetRotation()); + + AzToolsFramework::SelectEntities({ m_entityId2, m_entityId3 }); + + const auto initialManipulatorTransform = GetManipulatorTransform(); + + const float screenToWorldMultiplier = AzToolsFramework::CalculateScreenToWorldMultiplier( + AzToolsFramework::GetWorldTransform(m_entityId1).GetTranslation(), m_cameraState); + + // calculate positions for two click and drag motions (moving a linear manipulator) + // begin each click in the center of the line of the linear manipulators + const auto translationManipulatorStartHoldWorldPosition1 = AzToolsFramework::GetWorldTransform(m_entityId1).GetTranslation() + + initialManipulatorTransform->GetBasisZ() * screenToWorldMultiplier; + const auto translationManipulatorEndHoldWorldPosition1 = + translationManipulatorStartHoldWorldPosition1 + AZ::Vector3::CreateAxisZ(LinearManipulatorZAxisMovement); + const auto translationManipulatorStartHoldWorldPosition2 = AzToolsFramework::GetWorldTransform(m_entityId1).GetTranslation() + + AZ::Vector3::CreateAxisZ(LinearManipulatorZAxisMovement) - initialManipulatorTransform->GetBasisY() * screenToWorldMultiplier; + const auto translationManipulatorEndHoldWorldPosition2 = + translationManipulatorStartHoldWorldPosition2 + AZ::Vector3::CreateAxisY(LinearManipulatorYAxisMovement); + + // transform to screen space + const auto translationManipulatorStartHoldScreenPosition1 = + AzFramework::WorldToScreen(translationManipulatorStartHoldWorldPosition1, m_cameraState); + const auto translationManipulatorEndHoldScreenPosition1 = + AzFramework::WorldToScreen(translationManipulatorEndHoldWorldPosition1, m_cameraState); + const auto translationManipulatorStartHoldScreenPosition2 = + AzFramework::WorldToScreen(translationManipulatorStartHoldWorldPosition2, m_cameraState); + const auto translationManipulatorEndHoldScreenPosition2 = + AzFramework::WorldToScreen(translationManipulatorEndHoldWorldPosition2, m_cameraState); + + m_actionDispatcher->CameraState(m_cameraState) + ->MousePosition(translationManipulatorStartHoldScreenPosition1) + ->KeyboardModifierDown(GetParam().m_keyboardModifier) + ->MouseLButtonDown() + ->MousePosition(translationManipulatorEndHoldScreenPosition1) + ->MouseLButtonUp() + ->MousePosition(translationManipulatorStartHoldScreenPosition2) + ->MouseLButtonDown() + ->MousePosition(translationManipulatorEndHoldScreenPosition2) + ->MouseLButtonUp(); + + const auto expectedEntity2Transform = GetParam().m_firstExpectedEntityTransformAfter; + const auto expectedEntity3Transform = GetParam().m_secondExpectedEntityTransformAfter; + const auto expectedManipulatorTransform = GetParam().m_expectedManipulatorTransformAfter; + + const auto manipulatorTransformAfter = GetManipulatorTransform(); + const auto entity2Transform = AzToolsFramework::GetWorldTransform(m_entityId2); + const auto entity3Transform = AzToolsFramework::GetWorldTransform(m_entityId3); + + EXPECT_THAT(*manipulatorTransformAfter, IsCloseTolerance(expectedManipulatorTransform, 0.01f)); + EXPECT_THAT(entity2Transform, IsCloseTolerance(expectedEntity2Transform, 0.01f)); + EXPECT_THAT(entity3Transform, IsCloseTolerance(expectedEntity3Transform, 0.01f)); + } + + static const AZ::Transform ExpectedEntity2TransformAfterLocalGroupTranslationManipulatorMotion = + AZ::Transform::CreateTranslation( + EditorTransformComponentSelectionViewportPickingFixture::Entity2WorldTranslation + + AZ::Vector3(0.0f, LinearManipulatorYAxisMovement, LinearManipulatorZAxisMovement)) * + Entity2RotationForLocalTranslation; + static const AZ::Transform ExpectedEntity3TransformAfterLocalGroupTranslationManipulatorMotion = AZ::Transform::CreateTranslation( + EditorTransformComponentSelectionViewportPickingFixture::Entity3WorldTranslation + + AZ::Vector3(0.0f, LinearManipulatorYAxisMovement, LinearManipulatorZAxisMovement)); + // note: as entity has been rotated by 90 degrees about Z in TranslatingMultipleEntitiesWithDifferentModifierCombinations then + // LinearManipulatorYAxisMovement is now aligned to the world x-axis + static const AZ::Transform ExpectedEntity2TransformAfterLocalIndividualTranslationManipulatorMotion = + AZ::Transform::CreateTranslation( + EditorTransformComponentSelectionViewportPickingFixture::Entity2WorldTranslation + + AZ::Vector3(-LinearManipulatorYAxisMovement, 0.0f, LinearManipulatorZAxisMovement)) * + Entity2RotationForLocalTranslation; + static const AZ::Transform ExpectedEntity3TransformAfterLocalIndividualTranslationManipulatorMotion = AZ::Transform::CreateTranslation( + EditorTransformComponentSelectionViewportPickingFixture::Entity3WorldTranslation + + AZ::Vector3(0.0f, LinearManipulatorYAxisMovement, LinearManipulatorZAxisMovement)); + + INSTANTIATE_TEST_CASE_P( + All, + EditorTransformComponentSelectionTranslationManipulatorMultipleEntityTestFixtureParam, + testing::Values( + // this replicates translating a group of entities in local space with no modifiers held (group influence) + // manipulator and entity translate + ManipulatorOptionsMultiple{ AzToolsFramework::ViewportInteraction::KeyboardModifier::None, + ExpectedManipulatorTransformAfterGroupTranslationManipulatorMotion, + ExpectedEntity2TransformAfterLocalGroupTranslationManipulatorMotion, + ExpectedEntity3TransformAfterLocalGroupTranslationManipulatorMotion }, + // this replicates translating a group of entities in local space with the alt modifier held + // entities move in their own local space (individual influence) + ManipulatorOptionsMultiple{ AzToolsFramework::ViewportInteraction::KeyboardModifier::Alt, + ExpectedManipulatorTransformAfterGroupTranslationManipulatorMotion, + ExpectedEntity2TransformAfterLocalIndividualTranslationManipulatorMotion, + ExpectedEntity3TransformAfterLocalIndividualTranslationManipulatorMotion }, + // this replicates translating a group of entities in world space with the shift modifier held + // entities and manipulator move in world space + ManipulatorOptionsMultiple{ AzToolsFramework::ViewportInteraction::KeyboardModifier::Shift, + ExpectedManipulatorTransformAfterGroupTranslationManipulatorMotion, + ExpectedEntity2TransformAfterLocalGroupTranslationManipulatorMotion, + ExpectedEntity3TransformAfterLocalGroupTranslationManipulatorMotion }, + // this replicates translating the manipulator in local space with the ctrl modifier held (entities are unchanged) + ManipulatorOptionsMultiple{ + AzToolsFramework::ViewportInteraction::KeyboardModifier::Ctrl, + ExpectedManipulatorTransformAfterGroupTranslationManipulatorMotion, + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity2WorldTranslation) * + Entity2RotationForLocalTranslation, + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity3WorldTranslation) })); + + class EditorTransformComponentSelectionScaleManipulatorMultipleEntityTestFixtureParam + : public EditorTransformComponentSelectionManipulatorInteractionTestFixture + , public ::testing::WithParamInterface + { + }; + + static const float LinearManipulatorZAxisMovementScale = 0.5f; + + TEST_P( + EditorTransformComponentSelectionScaleManipulatorMultipleEntityTestFixtureParam, + ScalingMultipleEntitiesWithDifferentModifierCombinations) + { + using AzToolsFramework::EditorTransformComponentSelectionRequestBus; + + PositionEntities(); + + // move camera up and to the left so it's just above the normal row of entities + AzFramework::SetCameraTransform( + m_cameraState, + AZ::Transform::CreateFromQuaternionAndTranslation( + AZ::Quaternion::CreateFromEulerAnglesDegrees(AZ::Vector3(0.0f, 0.0f, 90.0f)), AZ::Vector3(10.0f, 15.0f, 10.1f))); + + SetTransformMode(EditorTransformComponentSelectionRequestBus::Events::Mode::Scale); + + AzToolsFramework::SelectEntities({ m_entityId2, m_entityId3 }); + + // manipulator should be centered between the two entities + const auto initialManipulatorTransform = GetManipulatorTransform(); + + const float screenToWorldMultiplier = + AzToolsFramework::CalculateScreenToWorldMultiplier(initialManipulatorTransform->GetTranslation(), m_cameraState); + + const auto translationManipulatorStartHoldWorldPosition1 = AzToolsFramework::GetWorldTransform(m_entityId1).GetTranslation() + + initialManipulatorTransform->GetBasisZ() * screenToWorldMultiplier; + const auto translationManipulatorEndHoldWorldPosition1 = + translationManipulatorStartHoldWorldPosition1 + AZ::Vector3::CreateAxisZ(LinearManipulatorZAxisMovementScale); + + // calculate screen space positions + const auto scaleManipulatorHoldScreenPosition = + AzFramework::WorldToScreen(translationManipulatorStartHoldWorldPosition1, m_cameraState); + const auto scaleManipulatorEndHoldScreenPosition = + AzFramework::WorldToScreen(translationManipulatorEndHoldWorldPosition1, m_cameraState); + + m_actionDispatcher->CameraState(m_cameraState) + ->MousePosition(scaleManipulatorHoldScreenPosition) + ->KeyboardModifierDown(GetParam().m_keyboardModifier) + ->MouseLButtonDown() + ->MousePosition(scaleManipulatorEndHoldScreenPosition) + ->MouseLButtonUp(); + + const auto expectedEntity2Transform = GetParam().m_firstExpectedEntityTransformAfter; + const auto expectedEntity3Transform = GetParam().m_secondExpectedEntityTransformAfter; + const auto expectedManipulatorTransform = GetParam().m_expectedManipulatorTransformAfter; + + const auto manipulatorTransformAfter = GetManipulatorTransform(); + const auto entity2Transform = AzToolsFramework::GetWorldTransform(m_entityId2); + const auto entity3Transform = AzToolsFramework::GetWorldTransform(m_entityId3); + + EXPECT_THAT(*manipulatorTransformAfter, IsCloseTolerance(expectedManipulatorTransform, 0.01f)); + EXPECT_THAT(entity2Transform, IsCloseTolerance(expectedEntity2Transform, 0.01f)); + EXPECT_THAT(entity3Transform, IsCloseTolerance(expectedEntity3Transform, 0.01f)); + } + + static const AZ::Transform ExpectedEntity2TransformAfterLocalGroupScaleManipulatorMotion = + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity2WorldTranslation) * + AZ::Transform::CreateTranslation(AZ::Vector3(0.0f, -1.0f, 0.0f)) * + AZ::Transform::CreateUniformScale(LinearManipulatorZAxisMovement); + static const AZ::Transform ExpectedEntity3TransformAfterLocalGroupScaleManipulatorMotion = + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity3WorldTranslation) * + AZ::Transform::CreateTranslation(AZ::Vector3(0.0f, 1.0f, 0.0f)) * AZ::Transform::CreateUniformScale(LinearManipulatorZAxisMovement); + static const AZ::Transform ExpectedEntity2TransformAfterLocalIndividualScaleManipulatorMotion = + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity2WorldTranslation) * + AZ::Transform::CreateUniformScale(LinearManipulatorZAxisMovement); + static const AZ::Transform ExpectedEntity3TransformAfterLocalIndividualScaleManipulatorMotion = + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity3WorldTranslation) * + AZ::Transform::CreateUniformScale(LinearManipulatorZAxisMovement); + + INSTANTIATE_TEST_CASE_P( + All, + EditorTransformComponentSelectionScaleManipulatorMultipleEntityTestFixtureParam, + testing::Values( + // this replicates scaling a group of entities in local space with no modifiers held + // entities scale relative to manipulator pivot + ManipulatorOptionsMultiple{ AzToolsFramework::ViewportInteraction::KeyboardModifier::None, + AZ::Transform::CreateTranslation(AggregateManipulatorPositionWithEntity2and3Selected), + ExpectedEntity2TransformAfterLocalGroupScaleManipulatorMotion, + ExpectedEntity3TransformAfterLocalGroupScaleManipulatorMotion }, + // this replicates scaling a group of entities in local space with the alt modifier held + // entities scale about their own pivot + ManipulatorOptionsMultiple{ AzToolsFramework::ViewportInteraction::KeyboardModifier::Alt, + AZ::Transform::CreateTranslation(AggregateManipulatorPositionWithEntity2and3Selected), + ExpectedEntity2TransformAfterLocalIndividualScaleManipulatorMotion, + ExpectedEntity3TransformAfterLocalIndividualScaleManipulatorMotion }, + // this replicates scaling a group of entities in world space with the shift modifier held + // entities scale relative to manipulator pivot in world space + ManipulatorOptionsMultiple{ AzToolsFramework::ViewportInteraction::KeyboardModifier::Shift, + AZ::Transform::CreateTranslation(AggregateManipulatorPositionWithEntity2and3Selected), + ExpectedEntity2TransformAfterLocalGroupScaleManipulatorMotion, + ExpectedEntity3TransformAfterLocalGroupScaleManipulatorMotion }, + // this has no effect (entities and manipulator are unchanged) + ManipulatorOptionsMultiple{ + AzToolsFramework::ViewportInteraction::KeyboardModifier::Ctrl, + AZ::Transform::CreateTranslation(AggregateManipulatorPositionWithEntity2and3Selected), + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity2WorldTranslation), + AZ::Transform::CreateTranslation(EditorTransformComponentSelectionViewportPickingFixture::Entity3WorldTranslation) })); + using EditorTransformComponentSelectionManipulatorTestFixture = IndirectCallManipulatorViewportInteractionFixtureMixin; @@ -1661,7 +2211,7 @@ namespace UnitTest All, EditorTransformComponentSelectionSingleEntityPivotAndOverrideFixture, testing::Values( - ReferenceFrameWithOrientation{ AzToolsFramework::ReferenceFrame::Local, ChildExpectedPivotLocalOrientationInWorldSpace }, + ReferenceFrameWithOrientation{ AzToolsFramework::ReferenceFrame::Local, PivotOverrideLocalOrientationInWorldSpace }, ReferenceFrameWithOrientation{ AzToolsFramework::ReferenceFrame::Parent, PivotOverrideLocalOrientationInWorldSpace }, ReferenceFrameWithOrientation{ AzToolsFramework::ReferenceFrame::World, AZ::Quaternion::CreateIdentity() })); diff --git a/Code/Framework/AzToolsFramework/Tests/Viewport/ViewportScreenTests.cpp b/Code/Framework/AzToolsFramework/Tests/Viewport/ViewportScreenTests.cpp index 77058038e9..e8fce6653f 100644 --- a/Code/Framework/AzToolsFramework/Tests/Viewport/ViewportScreenTests.cpp +++ b/Code/Framework/AzToolsFramework/Tests/Viewport/ViewportScreenTests.cpp @@ -6,8 +6,8 @@ * */ -#include #include +#include #include #include #include @@ -20,18 +20,17 @@ namespace UnitTest { - // transform a point from normalized device coordinates to world space, and then from world space back to normalized device coordinates - AZ::Vector2 ScreenNDCToWorldToScreenNDC( - const AZ::Vector2& ndcPoint, const AzFramework::CameraState& cameraState) + // transform a point from normalized device coordinates to world space, and then from world space back to normalized device coordinates + AZ::Vector2 ScreenNdcToWorldToScreenNdc(const AZ::Vector2& ndcPoint, const AzFramework::CameraState& cameraState) { - const auto worldResult = AzFramework::ScreenNDCToWorld(ndcPoint, InverseCameraView(cameraState), InverseCameraProjection(cameraState)); - const auto ndcResult = AzFramework::WorldToScreenNDC(worldResult, CameraView(cameraState), CameraProjection(cameraState)); + const auto worldResult = + AzFramework::ScreenNdcToWorld(ndcPoint, InverseCameraView(cameraState), InverseCameraProjection(cameraState)); + const auto ndcResult = AzFramework::WorldToScreenNdc(worldResult, CameraView(cameraState), CameraProjection(cameraState)); return AZ::Vector3ToVector2(ndcResult); } // transform a point from screen space to world space, and then from world space back to screen space - AzFramework::ScreenPoint ScreenToWorldToScreen( - const AzFramework::ScreenPoint& screenPoint, const AzFramework::CameraState& cameraState) + AzFramework::ScreenPoint ScreenToWorldToScreen(const AzFramework::ScreenPoint& screenPoint, const AzFramework::CameraState& cameraState) { const auto worldResult = AzFramework::ScreenToWorld(screenPoint, cameraState); return AzFramework::WorldToScreen(worldResult, cameraState); @@ -47,25 +46,25 @@ namespace UnitTest const auto cameraState = AzFramework::CreateIdentityDefaultCamera(cameraPosition, screenDimensions); { - const auto expectedScreenPoint = ScreenPoint{600, 450}; + const auto expectedScreenPoint = ScreenPoint{ 600, 450 }; const auto resultScreenPoint = ScreenToWorldToScreen(expectedScreenPoint, cameraState); EXPECT_EQ(resultScreenPoint, expectedScreenPoint); } { - const auto expectedScreenPoint = ScreenPoint{400, 300}; + const auto expectedScreenPoint = ScreenPoint{ 400, 300 }; const auto resultScreenPoint = ScreenToWorldToScreen(expectedScreenPoint, cameraState); EXPECT_EQ(resultScreenPoint, expectedScreenPoint); } { - const auto expectedScreenPoint = ScreenPoint{0, 0}; + const auto expectedScreenPoint = ScreenPoint{ 0, 0 }; const auto resultScreenPoint = ScreenToWorldToScreen(expectedScreenPoint, cameraState); EXPECT_EQ(resultScreenPoint, expectedScreenPoint); } { - const auto expectedScreenPoint = ScreenPoint{800, 600}; + const auto expectedScreenPoint = ScreenPoint{ 800, 600 }; const auto resultScreenPoint = ScreenToWorldToScreen(expectedScreenPoint, cameraState); EXPECT_EQ(resultScreenPoint, expectedScreenPoint); } @@ -81,7 +80,7 @@ namespace UnitTest const auto cameraState = AzFramework::CreateDefaultCamera(cameraTransform, screenDimensions); - const auto expectedScreenPoint = ScreenPoint{200, 300}; + const auto expectedScreenPoint = ScreenPoint{ 200, 300 }; const auto resultScreenPoint = ScreenToWorldToScreen(expectedScreenPoint, cameraState); EXPECT_EQ(resultScreenPoint, expectedScreenPoint); } @@ -93,46 +92,46 @@ namespace UnitTest using AzFramework::ScreenPoint; const auto screenDimensions = AZ::Vector2(800.0f, 600.0f); - const auto cameraTransform = AZ::Transform::CreateTranslation(AZ::Vector3(10.0f, 0.0f, 0.0f)) * - AZ::Transform::CreateRotationZ(AZ::DegToRad(-90.0f)); + const auto cameraTransform = + AZ::Transform::CreateTranslation(AZ::Vector3(10.0f, 0.0f, 0.0f)) * AZ::Transform::CreateRotationZ(AZ::DegToRad(-90.0f)); const auto cameraState = AzFramework::CreateDefaultCamera(cameraTransform, screenDimensions); - const auto worldResult = AzFramework::ScreenToWorld(ScreenPoint{400, 300}, cameraState); + const auto worldResult = AzFramework::ScreenToWorld(ScreenPoint{ 400, 300 }, cameraState); EXPECT_THAT(worldResult, IsClose(AZ::Vector3(10.1f, 0.0f, 0.0f))); } - + //////////////////////////////////////////////////////////////////////////////////////////////////////// // NDC tests TEST(ViewportScreen, WorldToScreenNDCAndScreenNDCToWorldReturnsTheSameValueIdentityCameraOffsetFromOrigin) { using NdcPoint = AZ::Vector2; - + const auto screenDimensions = AZ::Vector2(800.0f, 600.0f); const auto cameraPosition = AZ::Vector3::CreateAxisY(-10.0f); const auto cameraState = AzFramework::CreateIdentityDefaultCamera(cameraPosition, screenDimensions); { - const auto expectedNdcPoint = NdcPoint{0.75f, 0.75f}; - const auto resultNdcPoint = ScreenNDCToWorldToScreenNDC(expectedNdcPoint, cameraState); + const auto expectedNdcPoint = NdcPoint{ 0.75f, 0.75f }; + const auto resultNdcPoint = ScreenNdcToWorldToScreenNdc(expectedNdcPoint, cameraState); EXPECT_THAT(resultNdcPoint, IsClose(expectedNdcPoint)); } { - const auto expectedNdcPoint = NdcPoint{0.5f, 0.5f}; - const auto resultNdcPoint = ScreenNDCToWorldToScreenNDC(expectedNdcPoint, cameraState); + const auto expectedNdcPoint = NdcPoint{ 0.5f, 0.5f }; + const auto resultNdcPoint = ScreenNdcToWorldToScreenNdc(expectedNdcPoint, cameraState); EXPECT_THAT(resultNdcPoint, IsClose(expectedNdcPoint)); } { - const auto expectedNdcPoint = NdcPoint{0.0f, 0.0f}; - const auto resultNdcPoint = ScreenNDCToWorldToScreenNDC(expectedNdcPoint, cameraState); + const auto expectedNdcPoint = NdcPoint{ 0.0f, 0.0f }; + const auto resultNdcPoint = ScreenNdcToWorldToScreenNdc(expectedNdcPoint, cameraState); EXPECT_THAT(resultNdcPoint, IsClose(expectedNdcPoint)); } { - const auto expectedNdcPoint = NdcPoint{1.0f, 1.0f}; - const auto resultNdcPoint = ScreenNDCToWorldToScreenNDC(expectedNdcPoint, cameraState); + const auto expectedNdcPoint = NdcPoint{ 1.0f, 1.0f }; + const auto resultNdcPoint = ScreenNdcToWorldToScreenNdc(expectedNdcPoint, cameraState); EXPECT_THAT(resultNdcPoint, IsClose(expectedNdcPoint)); } } @@ -147,8 +146,8 @@ namespace UnitTest const auto cameraState = AzFramework::CreateDefaultCamera(cameraTransform, screenDimensions); - const auto expectedNdcPoint = NdcPoint{0.25f, 0.5f}; - const auto resultNdcPoint = ScreenNDCToWorldToScreenNDC(expectedNdcPoint, cameraState); + const auto expectedNdcPoint = NdcPoint{ 0.25f, 0.5f }; + const auto resultNdcPoint = ScreenNdcToWorldToScreenNdc(expectedNdcPoint, cameraState); EXPECT_THAT(resultNdcPoint, IsClose(expectedNdcPoint)); } @@ -159,12 +158,13 @@ namespace UnitTest using NdcPoint = AZ::Vector2; const auto screenDimensions = AZ::Vector2(800.0f, 600.0f); - const auto cameraTransform = AZ::Transform::CreateTranslation(AZ::Vector3(10.0f, 0.0f, 0.0f)) * - AZ::Transform::CreateRotationZ(AZ::DegToRad(-90.0f)); + const auto cameraTransform = + AZ::Transform::CreateTranslation(AZ::Vector3(10.0f, 0.0f, 0.0f)) * AZ::Transform::CreateRotationZ(AZ::DegToRad(-90.0f)); const auto cameraState = AzFramework::CreateDefaultCamera(cameraTransform, screenDimensions); - const auto worldResult = AzFramework::ScreenNDCToWorld(NdcPoint{0.5f, 0.5f}, InverseCameraView(cameraState), InverseCameraProjection(cameraState)); + const auto worldResult = + AzFramework::ScreenNdcToWorld(NdcPoint{ 0.5f, 0.5f }, InverseCameraView(cameraState), InverseCameraProjection(cameraState)); EXPECT_THAT(worldResult, IsClose(AZ::Vector3(10.1f, 0.0f, 0.0f))); } @@ -175,7 +175,7 @@ namespace UnitTest using AzFramework::ScreenPoint; using AzFramework::ScreenVector; - const ScreenVector screenVector = ScreenPoint{100, 200} - ScreenPoint{10, 20}; + const ScreenVector screenVector = ScreenPoint{ 100, 200 } - ScreenPoint{ 10, 20 }; EXPECT_EQ(screenVector, ScreenVector(90, 180)); } @@ -184,7 +184,7 @@ namespace UnitTest using AzFramework::ScreenPoint; using AzFramework::ScreenVector; - const ScreenPoint screenPoint = ScreenPoint{100, 200} + ScreenVector{50, 25}; + const ScreenPoint screenPoint = ScreenPoint{ 100, 200 } + ScreenVector{ 50, 25 }; EXPECT_EQ(screenPoint, ScreenPoint(150, 225)); } @@ -193,7 +193,7 @@ namespace UnitTest using AzFramework::ScreenPoint; using AzFramework::ScreenVector; - const ScreenPoint screenPoint = ScreenPoint{120, 200} - ScreenVector{50, 20}; + const ScreenPoint screenPoint = ScreenPoint{ 120, 200 } - ScreenVector{ 50, 20 }; EXPECT_EQ(screenPoint, ScreenPoint(70, 180)); } @@ -202,7 +202,7 @@ namespace UnitTest using AzFramework::ScreenPoint; using AzFramework::ScreenVector; - const ScreenVector screenVector = ScreenVector{100, 200} + ScreenVector{50, 25}; + const ScreenVector screenVector = ScreenVector{ 100, 200 } + ScreenVector{ 50, 25 }; EXPECT_EQ(screenVector, ScreenVector(150, 225)); } @@ -211,7 +211,7 @@ namespace UnitTest using AzFramework::ScreenPoint; using AzFramework::ScreenVector; - const ScreenVector screenVector = ScreenVector{100, 200} - ScreenVector{50, 25}; + const ScreenVector screenVector = ScreenVector{ 100, 200 } - ScreenVector{ 50, 25 }; EXPECT_EQ(screenVector, ScreenVector(50, 175)); } @@ -220,8 +220,8 @@ namespace UnitTest using AzFramework::ScreenPoint; using AzFramework::ScreenVector; - const ScreenPoint screenPoint = ScreenPoint{100, 200}; - const ScreenVector screenVector = ScreenVector{50, 25}; + const ScreenPoint screenPoint = ScreenPoint{ 100, 200 }; + const ScreenVector screenVector = ScreenVector{ 50, 25 }; const AZ::Vector2 fromScreenPoint = AzFramework::Vector2FromScreenPoint(screenPoint); const AZ::Vector2 fromScreenVector = AzFramework::Vector2FromScreenVector(screenVector); @@ -295,6 +295,58 @@ namespace UnitTest EXPECT_NEAR(AzFramework::ScreenVectorLength(ScreenVector(12, 15)), 19.20937f, 0.001f); } + TEST(ViewportScreen, ScreenVectorTransformedByScalarUpwards) + { + using AzFramework::ScreenVector; + + auto screenVector = ScreenVector(5, 10); + auto scaledScreenVector = screenVector * 2.0f; + + EXPECT_EQ(scaledScreenVector, ScreenVector(10, 20)); + } + + TEST(ViewportScreen, ScreenVectorTransformedByScalarWithRounding) + { + using AzFramework::ScreenVector; + + auto screenVector = ScreenVector(1, 6); + auto scaledScreenVector = screenVector * 0.1f; + + // value less than 0.5 rounds down, greater than or equal to 0.5 rounds up + EXPECT_EQ(scaledScreenVector, ScreenVector(0, 1)); + } + + TEST(ViewportScreen, ScreenVectorTransformedByScalarWithRoundingAtHalfwayBoundary) + { + using AzFramework::ScreenVector; + + auto screenVector = ScreenVector(5, 10); + auto scaledScreenVector = screenVector * 0.1f; + + // value less than 0.5 rounds down, greater than or equal to 0.5 rounds up + EXPECT_EQ(scaledScreenVector, ScreenVector(1, 1)); + } + + TEST(ViewportScreen, ScreenVectorTransformedByScalarDownwards) + { + using AzFramework::ScreenVector; + + auto screenVector = ScreenVector(6, 12); + auto scaledScreenVector = screenVector * 0.5f; + + EXPECT_EQ(scaledScreenVector, ScreenVector(3, 6)); + } + + TEST(ViewportScreen, ScreenVectorTransformedByScalarInplace) + { + using AzFramework::ScreenVector; + + auto screenVector = ScreenVector(13, 37); + screenVector *= 10.0f; + + EXPECT_EQ(screenVector, ScreenVector(130, 370)); + } + //////////////////////////////////////////////////////////////////////////////////////////////////////// // Other tests TEST(ViewportScreen, CanGetCameraTransformFromCameraViewAndBack) diff --git a/Gems/AtomLyIntegration/AtomFont/Code/Source/FFont.cpp b/Gems/AtomLyIntegration/AtomFont/Code/Source/FFont.cpp index c5268b427f..86b3011836 100644 --- a/Gems/AtomLyIntegration/AtomFont/Code/Source/FFont.cpp +++ b/Gems/AtomLyIntegration/AtomFont/Code/Source/FFont.cpp @@ -1726,7 +1726,7 @@ void AZ::FFont::DrawScreenAlignedText3d( { return; } - AZ::Vector3 positionNDC = AzFramework::WorldToScreenNDC( + AZ::Vector3 positionNDC = AzFramework::WorldToScreenNdc( params.m_position, currentView->GetWorldToViewMatrix(), currentView->GetViewToClipMatrix() From 1f6dfd0194d84ef5f126e970936fffe77e2c7772 Mon Sep 17 00:00:00 2001 From: AMZN-Igarri <82394219+AMZN-Igarri@users.noreply.github.com> Date: Tue, 28 Sep 2021 13:47:21 +0200 Subject: [PATCH 13/20] Asset Browser Search View Column Resizing (#4087) * Added resizing Signed-off-by: igarri * Added Signals and Slots for resizing columns Signed-off-by: igarri * Cleaned up code Signed-off-by: igarri * Fixed Column Resize when docking Signed-off-by: igarri * Added constants Signed-off-by: igarri * Applied Code review feedback Signed-off-by: igarri * aplied more feedback Signed-off-by: igarri * Addressed Code Review Comments Signed-off-by: igarri * Fixing bad merge Signed-off-by: igarri * Added fix in initialization Signed-off-by: igarri * Changed header setup sequence Signed-off-by: igarri --- .../AzAssetBrowser/AzAssetBrowserWindow.cpp | 23 +++++++++++++++++++ .../AzAssetBrowser/AzAssetBrowserWindow.h | 8 +++++++ .../AssetPicker/AssetPickerDialog.cpp | 10 ++++++++ .../AssetPicker/AssetPickerDialog.h | 4 ++++ .../AssetPicker/AssetPickerDialog.ui | 6 ++--- .../Views/AssetBrowserTableView.cpp | 21 ++++++++++++++--- .../Views/AssetBrowserTableView.h | 4 +++- 7 files changed, 69 insertions(+), 7 deletions(-) diff --git a/Code/Editor/AzAssetBrowser/AzAssetBrowserWindow.cpp b/Code/Editor/AzAssetBrowser/AzAssetBrowserWindow.cpp index 18946c7874..9fe4cfd0d2 100644 --- a/Code/Editor/AzAssetBrowser/AzAssetBrowserWindow.cpp +++ b/Code/Editor/AzAssetBrowser/AzAssetBrowserWindow.cpp @@ -141,6 +141,10 @@ AzAssetBrowserWindow::AzAssetBrowserWindow(QWidget* parent) m_ui->m_assetBrowserTreeViewWidget, &AzAssetBrowser::AssetBrowserTreeView::ClearTypeFilter, m_ui->m_searchWidget, &AzAssetBrowser::SearchWidget::ClearTypeFilter); + connect( + this, &AzAssetBrowserWindow::SizeChangedSignal, m_ui->m_assetBrowserTableViewWidget, + &AzAssetBrowser::AssetBrowserTableView::UpdateSizeSlot); + m_ui->m_assetBrowserTreeViewWidget->SetName("AssetBrowserTreeView_main"); } @@ -164,6 +168,25 @@ QObject* AzAssetBrowserWindow::createListenerForShowAssetEditorEvent(QObject* pa return listener; } +void AzAssetBrowserWindow::resizeEvent(QResizeEvent* resizeEvent) +{ + // leftLayout is the parent of the tableView + // rightLayout is the parent of the preview window. + // Workaround: When docking windows this event keeps holding the old size of the widgets instead of the new one + // but the resizeEvent holds the new size of the whole widget + // So we have to save the proportions somehow + const QWidget* leftLayout = m_ui->m_leftLayout; + const QVBoxLayout* rightLayout = m_ui->m_rightLayout; + + const float oldLeftLayoutWidth = aznumeric_cast(leftLayout->geometry().width()); + const float oldWidth = aznumeric_cast(leftLayout->geometry().width() + rightLayout->geometry().width()); + + const float newWidth = oldLeftLayoutWidth * aznumeric_cast(resizeEvent->size().width()) / oldWidth; + + emit SizeChangedSignal(aznumeric_cast(newWidth)); + QWidget::resizeEvent(resizeEvent); +} + void AzAssetBrowserWindow::OnInitViewToggleButton() { CreateSwitchViewMenu(); diff --git a/Code/Editor/AzAssetBrowser/AzAssetBrowserWindow.h b/Code/Editor/AzAssetBrowser/AzAssetBrowserWindow.h index bf742b0cf2..753f000300 100644 --- a/Code/Editor/AzAssetBrowser/AzAssetBrowserWindow.h +++ b/Code/Editor/AzAssetBrowser/AzAssetBrowserWindow.h @@ -53,9 +53,17 @@ public: static QObject* createListenerForShowAssetEditorEvent(QObject* parent); + +Q_SIGNALS: + void SizeChangedSignal(int newWidth); + +protected: + void resizeEvent(QResizeEvent* resizeEvent) override; + private: void OnInitViewToggleButton(); void UpdateDisplayInfo(); + protected slots: void CreateSwitchViewMenu(); void SetExpandedAssetBrowserMode(); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetPicker/AssetPickerDialog.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetPicker/AssetPickerDialog.cpp index 76b634b51e..966cd5a43a 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetPicker/AssetPickerDialog.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetPicker/AssetPickerDialog.cpp @@ -151,6 +151,10 @@ namespace AzToolsFramework m_ui->m_assetBrowserTableViewWidget, &AssetBrowserTableView::ClearTypeFilter, m_ui->m_searchWidget, &SearchWidget::ClearTypeFilter); + connect( + this, &AssetPickerDialog::SizeChangedSignal, m_ui->m_assetBrowserTableViewWidget, + &AssetBrowserTableView::UpdateSizeSlot); + m_ui->m_assetBrowserTableViewWidget->SetName("AssetBrowserTableView_main"); m_tableModel->UpdateTableModelMaps(); } @@ -206,6 +210,12 @@ namespace AzToolsFramework } } + void AssetPickerDialog::resizeEvent(QResizeEvent* resizeEvent) + { + emit SizeChangedSignal(m_ui->verticalLayout_4->geometry().width()); + QDialog::resizeEvent(resizeEvent); + } + void AssetPickerDialog::keyPressEvent(QKeyEvent* e) { // Until search widget is revised, Return key should not close the dialog, diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetPicker/AssetPickerDialog.h b/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetPicker/AssetPickerDialog.h index bab265c134..f8ec6077ce 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetPicker/AssetPickerDialog.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetPicker/AssetPickerDialog.h @@ -46,6 +46,9 @@ namespace AzToolsFramework explicit AssetPickerDialog(AssetSelectionModel& selection, QWidget* parent = nullptr); virtual ~AssetPickerDialog(); + Q_SIGNALS: + void SizeChangedSignal(int newWidth); + protected: ////////////////////////////////////////////////////////////////////////// // QDialog @@ -53,6 +56,7 @@ namespace AzToolsFramework void accept() override; void reject() override; void keyPressEvent(QKeyEvent* e) override; + void resizeEvent(QResizeEvent* resizeEvent) override; private Q_SLOTS: void DoubleClickedSlot(const QModelIndex& index); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetPicker/AssetPickerDialog.ui b/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetPicker/AssetPickerDialog.ui index b11ffb3990..df7d27d3c8 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetPicker/AssetPickerDialog.ui +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/AssetPicker/AssetPickerDialog.ui @@ -117,6 +117,9 @@ 0 + + + @@ -142,9 +145,6 @@ - - - diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/Views/AssetBrowserTableView.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/Views/AssetBrowserTableView.cpp index 217e282a37..feab9a9e5a 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/Views/AssetBrowserTableView.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/Views/AssetBrowserTableView.cpp @@ -20,13 +20,17 @@ AZ_PUSH_DISABLE_WARNING( #include #include #include - +#include #include AZ_POP_DISABLE_WARNING namespace AzToolsFramework { namespace AssetBrowser { + const float MinHeaderResizeProportion = .25f; + const float MaxHeaderResizeProportion = .75f; + const float DefaultHeaderResizeProportion = .5f; + AssetBrowserTableView::AssetBrowserTableView(QWidget* parent) : AzQtComponents::TableView(parent) , m_delegate(new SearchEntryDelegate(this)) @@ -65,8 +69,10 @@ namespace AzToolsFramework AzQtComponents::TableView::setModel(model); connect(m_tableModel, &AssetBrowserTableModel::layoutChanged, this, &AssetBrowserTableView::layoutChangedSlot); - header()->setSectionResizeMode(0, QHeaderView::ResizeMode::Stretch); - header()->setSectionResizeMode(1, QHeaderView::ResizeMode::Stretch); + header()->setStretchLastSection(true); + header()->setSectionResizeMode(0, QHeaderView::ResizeMode::Interactive); + header()->setSectionResizeMode(1, QHeaderView::ResizeMode::Interactive); + UpdateSizeSlot(parentWidget()->width()); header()->setSortIndicatorShown(false); header()->setSectionsClickable(false); } @@ -148,8 +154,17 @@ namespace AzToolsFramework void AssetBrowserTableView::OnAssetBrowserComponentReady() { + UpdateSizeSlot(parentWidget()->width()); } + void AssetBrowserTableView::UpdateSizeSlot(int newWidth) + { + setColumnWidth(0, aznumeric_cast(newWidth * DefaultHeaderResizeProportion)); + header()->setMinimumSectionSize(aznumeric_cast(newWidth * MinHeaderResizeProportion)); + header()->setMaximumSectionSize(aznumeric_cast(newWidth * MaxHeaderResizeProportion)); + } + + void AssetBrowserTableView::OnContextMenu([[maybe_unused]] const QPoint& point) { const auto& selectedAssets = GetSelectedAssets(); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/Views/AssetBrowserTableView.h b/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/Views/AssetBrowserTableView.h index b4eca59cef..b93500ba37 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/Views/AssetBrowserTableView.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/AssetBrowser/Views/AssetBrowserTableView.h @@ -59,12 +59,14 @@ namespace AzToolsFramework void ClearStringFilter(); void ClearTypeFilter(); + public Q_SLOTS: + void UpdateSizeSlot(int newWidth); + protected Q_SLOTS: void selectionChanged(const QItemSelection& selected, const QItemSelection& deselected) override; void rowsAboutToBeRemoved(const QModelIndex& parent, int start, int end) override; void layoutChangedSlot(const QList &parents = QList(), QAbstractItemModel::LayoutChangeHint hint = QAbstractItemModel::NoLayoutChangeHint); - private: QString m_name; QPointer m_tableModel; From 9078d1925aafc50c72e40f99da0f3f83b05f354c Mon Sep 17 00:00:00 2001 From: srikappa-amzn Date: Tue, 28 Sep 2021 06:42:53 -0700 Subject: [PATCH 14/20] Mock newly added methods to SpawnableEntitiesInterface Signed-off-by: srikappa-amzn --- .../AzFramework/Tests/Mocks/MockSpawnableEntitiesInterface.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Code/Framework/AzFramework/Tests/Mocks/MockSpawnableEntitiesInterface.h b/Code/Framework/AzFramework/Tests/Mocks/MockSpawnableEntitiesInterface.h index a5df7af4c3..a437545adf 100644 --- a/Code/Framework/AzFramework/Tests/Mocks/MockSpawnableEntitiesInterface.h +++ b/Code/Framework/AzFramework/Tests/Mocks/MockSpawnableEntitiesInterface.h @@ -40,6 +40,11 @@ namespace AzFramework MOCK_METHOD2(DespawnAllEntities, void(EntitySpawnTicket& ticket, DespawnAllEntitiesOptionalArgs optionalArgs)); + MOCK_METHOD3(DespawnEntity, void(AZ::EntityId entityId, EntitySpawnTicket& ticket, DespawnEntityOptionalArgs optionalArgs)); + + MOCK_METHOD2( + RetrieveEntitySpawnTicket, void(EntitySpawnTicket::Id entitySpawnTicketId, RetrieveEntitySpawnTicketCallback callback)); + MOCK_METHOD3( ReloadSpawnable, void(EntitySpawnTicket& ticket, AZ::Data::Asset spawnable, ReloadSpawnableOptionalArgs optionalArgs)); From a641586df28d4d62f5ba08c28296ff19746c2118 Mon Sep 17 00:00:00 2001 From: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Date: Tue, 28 Sep 2021 10:02:47 -0700 Subject: [PATCH 15/20] Move FocusModeInterface retrieval to EntityOutlinerListModel constructor to prevent crash on widget refresh. Also force expansion of root on construction for the same reason. Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> --- .../UI/Outliner/EntityOutlinerListModel.cpp | 7 ++----- .../AzToolsFramework/UI/Outliner/EntityOutlinerWidget.cpp | 3 +++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerListModel.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerListModel.cpp index 90f8e1d121..b3ac8b4c73 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerListModel.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerListModel.cpp @@ -82,6 +82,8 @@ namespace AzToolsFramework , m_entityExpansionState() , m_entityFilteredState() { + m_focusModeInterface = AZ::Interface::Get(); + AZ_Assert(m_focusModeInterface != nullptr, "EntityOutlinerListModel requires a FocusModeInterface instance on construction."); } EntityOutlinerListModel::~EntityOutlinerListModel() @@ -106,11 +108,6 @@ namespace AzToolsFramework m_editorEntityUiInterface = AZ::Interface::Get(); AZ_Assert(m_editorEntityUiInterface != nullptr, "EntityOutlinerListModel requires a EditorEntityUiInterface instance on Initialize."); - - m_focusModeInterface = AZ::Interface::Get(); - AZ_Assert( - m_focusModeInterface != nullptr, - "EntityOutlinerListModel requires a FocusModeInterface instance on Initialize."); } int EntityOutlinerListModel::rowCount(const QModelIndex& parent) const diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerWidget.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerWidget.cpp index 4db235d073..689e5b5dc4 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerWidget.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerWidget.cpp @@ -232,6 +232,9 @@ namespace AzToolsFramework m_gui->m_objectTree->header()->setSortIndicatorShown(false); m_gui->m_objectTree->header()->setStretchLastSection(false); + // Always expand root entity (level entity) - needed if the widget is re-created while a level is already open. + m_gui->m_objectTree->expand(m_proxyModel->index(0, 0)); + // resize the icon columns so that the Visibility and Lock toggle icon columns stay right-justified m_gui->m_objectTree->header()->setStretchLastSection(false); m_gui->m_objectTree->header()->setMinimumSectionSize(0); From 29d9abba5767b09647d080f504e04ef965b7d4c9 Mon Sep 17 00:00:00 2001 From: John Jones-Steele <82226755+jjjoness@users.noreply.github.com> Date: Tue, 28 Sep 2021 18:08:58 +0100 Subject: [PATCH 16/20] Removed redundant code (#4363) Signed-off-by: John Jones-Steele --- Gems/PhysX/Code/Source/EditorShapeColliderComponent.cpp | 1 - Gems/PhysX/Code/Source/EditorShapeColliderComponent.h | 2 -- 2 files changed, 3 deletions(-) diff --git a/Gems/PhysX/Code/Source/EditorShapeColliderComponent.cpp b/Gems/PhysX/Code/Source/EditorShapeColliderComponent.cpp index 57bfe17a30..2bbc04faae 100644 --- a/Gems/PhysX/Code/Source/EditorShapeColliderComponent.cpp +++ b/Gems/PhysX/Code/Source/EditorShapeColliderComponent.cpp @@ -676,7 +676,6 @@ namespace PhysX LmbrCentral::ShapeComponentNotificationsBus::Handler::BusDisconnect(); AZ::TransformNotificationBus::Handler::BusDisconnect(); AzToolsFramework::EntitySelectionEvents::Bus::Handler::BusDisconnect(); - AzFramework::EntityDebugDisplayEventBus::Handler::BusDisconnect(); AzToolsFramework::Components::EditorComponentBase::Deactivate(); if (m_sceneInterface && m_editorBodyHandle != AzPhysics::InvalidSimulatedBodyHandle) diff --git a/Gems/PhysX/Code/Source/EditorShapeColliderComponent.h b/Gems/PhysX/Code/Source/EditorShapeColliderComponent.h index 792d3c4327..431b34b1ae 100644 --- a/Gems/PhysX/Code/Source/EditorShapeColliderComponent.h +++ b/Gems/PhysX/Code/Source/EditorShapeColliderComponent.h @@ -10,7 +10,6 @@ #include #include -#include #include #include #include @@ -58,7 +57,6 @@ namespace PhysX //! component to create geometry in the PhysX simulation. class EditorShapeColliderComponent : public AzToolsFramework::Components::EditorComponentBase - , protected AzFramework::EntityDebugDisplayEventBus::Handler , protected AzToolsFramework::EntitySelectionEvents::Bus::Handler , private AZ::TransformNotificationBus::Handler , protected DebugDraw::DisplayCallback From e958b0e09cc2aca260605919917254838923c1cb Mon Sep 17 00:00:00 2001 From: Mikhail Naumov <82239319+AMZN-mnaumov@users.noreply.github.com> Date: Tue, 28 Sep 2021 14:07:56 -0500 Subject: [PATCH 17/20] No longer can instantiate prefabs in an empty level (#4277) * No longer can instantiate prefabs in an empty level Signed-off-by: Mikhail Naumov * PR feedback from Ram Signed-off-by: Mikhail Naumov * removing tab Signed-off-by: Mikhail Naumov * PR feedback Signed-off-by: Mikhail Naumov * Implement Project Manager 'build' button for Mac and Linux(#4248) Signed-off-by: Steve Pham Signed-off-by: Mikhail Naumov * Remove the PrefabEditManager. Introduce the FocusMode system on the Editor side, and a PrefabFocusHandler on the Prefab side to handle focus. Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Signed-off-by: Mikhail Naumov * Changed FocusOnOwningPrefab to return Outcome. Added comments and error checking. Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Signed-off-by: Mikhail Naumov * Remove assert for edge case that can actually happen in normal circumstances, simply return false in those cases. Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Signed-off-by: Mikhail Naumov * Remove nested FocusModeFramework namespace Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Signed-off-by: Mikhail Naumov * Minor fixes to variable names and comments. Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Signed-off-by: Mikhail Naumov * ClearFocusRoot implementation Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Signed-off-by: Mikhail Naumov * Remove pragma once from cpp file Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Signed-off-by: Mikhail Naumov * Fix header formatting to pass validation Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Signed-off-by: Mikhail Naumov * Do not assert if m_focusModeInterface can't be initialized Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Signed-off-by: Mikhail Naumov * Initialization changes to pass unit tests. Some interfaces are now retrieved on demand and there's clearer failure paths that don't involve asserts to better handle test/headless initializations. Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Signed-off-by: Mikhail Naumov * LYN-6793 [iOS] [asset_profile] 4 assets fail to process for iOS (#4268) * LYN-6793 [iOS] [asset_profile] 4 assets fail to process for iOS The issue was because the compression of ETC formats took too long. Replaced all ETC and PVRTC formats with ASTC formats. Update all pixel operation for formats with single R channel to align the change with R8. Signed-off-by: Qing Tao Signed-off-by: Mikhail Naumov * Use resize_no_construct when creating a buffer asset, since the initial data is going to be memcopy'd anyways. (#4249) Signed-off-by: amzn-tommy Signed-off-by: Mikhail Naumov * Fix a minor spelling mistake (#4247) Signed-off-by: amzn-tommy Signed-off-by: Mikhail Naumov * 1. Update iOS deployment target to 14 (#4266) 2. Set MacOS default deployment target to 11. Signed-off-by: amzn-sj Signed-off-by: Mikhail Naumov * Debug build clang fixes Signed-off-by: AMZN-Olex <5432499+AMZN-Olex@users.noreply.github.com> Signed-off-by: Mikhail Naumov * Fix failed 'server' platform assets on Linux related to Shaders (#4275) * Add missing 'server' platform identifier for the ShaderBuilder * Use the current host platform as the fallback platform identifier (and not 'pc') Signed-off-by: Steve Pham Signed-off-by: Mikhail Naumov * [development] Atom CPU profiler include cleanup (#4272) - Removed all unnecessary includes to Atom CpuProfiler.h - Added includes to AzCore Profiler.h where necessary Signed-off-by: AMZN-ScottR 24445312+AMZN-ScottR@users.noreply.github.com Signed-off-by: Mikhail Naumov * Add guard against edge case. Signed-off-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Signed-off-by: Mikhail Naumov * Expose Transform::CreateLookAt to behavior context to use with hydra tests Signed-off-by: amzn-tommy Signed-off-by: Mikhail Naumov * Cached BLAS objects at the sub-mesh level Signed-off-by: dmcdiar Signed-off-by: Mikhail Naumov * EMotion FX: Asset Processor generates an Actor for every FBX (#4284) Fixing the actor behavior to ignore actor asset generation for files that only contain nodes/bones. We're only checking for blend shapes and skins now. In case neither is present in an FBX, we don't export the actor asset. This makes a plain skeleton a special case where users manually need to add the actor group for the fbx. The most common use case for these are animation files anyway, where we don't want to export actor assets. Also increased the version number of the actor group so that they are all getting reprocessed and the unneeded actor assets get removed. Signed-off-by: Benjamin Jillich Signed-off-by: Mikhail Naumov * EMotion FX: Crash in the actor builder when fbx meta data contains node groups (#4283) Fixing a crash related to the small array conversion that made multiple assets crash the actor builder. Signed-off-by: Benjamin Jillich Signed-off-by: Mikhail Naumov * Project Manager Gem Dependencies (#4132) * Fix engine API change and add gem dependencies Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Add GemCatalog dependency test Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Clarify display name and fix missing const Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Moving a couple helper functions into private scope Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Update gem count when unselecting a gem #4074 This addresses the following issue https://github.com/o3de/o3de/issues/4074 Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Active/Inactive filter and dependency tooltips Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Accessors for previously added and dependencies Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Cart displays gem dependency changes Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Shorten titles to fit in summary popup Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Remove QString::number Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Remove extra space Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Consolidate source model accesor helpers Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Addressing minor feedback Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> * Remove unused local variable Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> Signed-off-by: Mikhail Naumov * Fix AutoGen of RPCs with no params Signed-off-by: puvvadar Signed-off-by: Mikhail Naumov * Adds Inspector to Gem Repo Screen (#4242) * Adds the gem repo screen with the UI built but with mocked data and not connected to the o3de scripts Signed-off-by: nggieber * Changed name of added to enabled, disabled define, removed unused functions Signed-off-by: nggieber * Added Repo Screen Inspector UI Signed-off-by: nggieber * Addressed minor PR feedback Signed-off-by: nggieber * Add some more minor PR changes Signed-off-by: nggieber Signed-off-by: Mikhail Naumov * Updated NetworkSpawnableHolderComponent to use TransformBus instead of FindComponent. Added dependency on TransformService Signed-off-by: pereslav Signed-off-by: Mikhail Naumov * Fixed Entity::GetTransform to work for the components of an activating entity. Made the cached transform lazy evaluated. Signed-off-by: pereslav Signed-off-by: Mikhail Naumov * Changed NetworkSpawnableHolderComponent to use Entity::GetTransform instead of an ebus call Signed-off-by: pereslav Signed-off-by: Mikhail Naumov * Moved local variable to a smaller scope Signed-off-by: pereslav Signed-off-by: Mikhail Naumov * Signed-off-by: LesaelR (#4278) * Signed-off-by: LesaelR * Removing the un-needed sandbox marks. Signed-off-by: LesaelR Signed-off-by: Mikhail Naumov * Small fix for color node tooltip to ask for values 0-1 instead of 0-255 Signed-off-by: Gene Walters Signed-off-by: Mikhail Naumov * Exposing NetworkCharacterComponent::TryMoveWithVelocity to script. Updating Multiplayer AutoComponent baseclass behavior context to Reflect itself instead of its derived (human made) component. This is so the derived class can also create behaviorcontext classes of its own if needed. Misc copyright header edit. Signed-off-by: Gene Walters Signed-off-by: Mikhail Naumov * Make sure Multiplayer AutoComponents dont generate property OnChange script events if GenerateEventBindings is disabled Signed-off-by: Gene Walters Signed-off-by: Mikhail Naumov * Adding parameter names to NetworkCharacterComponent script events so people know what the parameters are used for in scriptcanvas Signed-off-by: Gene Walters Signed-off-by: Mikhail Naumov * Small fix to use AZ_CRC_CE Signed-off-by: Gene Walters Signed-off-by: Mikhail Naumov * Disabling some tests when prefab mode is enabled Signed-off-by: Mikhail Naumov * Revert "Disabling some tests when prefab mode is enabled" This reverts commit 3fe9358d42ec9d7fa5ffaee458e4c6ca3ecbed6e. Signed-off-by: Mikhail Naumov * Revert "PR feedback" This reverts commit a5b86d1954974b950b7719d1bc7dd56bc2a2a21b. Signed-off-by: Mikhail Naumov * Revert "removing tab" This reverts commit ff6ef4bfb514d82a1b668baa95901ad363e648d0. Signed-off-by: Mikhail Naumov * Revert "PR feedback from Ram" This reverts commit cba7f9c2a114d3bb51c9b64f38a7e1fef7bb8fe8. Signed-off-by: Mikhail Naumov * Renaming level->rootInstance Signed-off-by: Mikhail Naumov Co-authored-by: Steve Pham <82231385+spham-amzn@users.noreply.github.com> Co-authored-by: Danilo Aimini <82231674+AMZN-daimini@users.noreply.github.com> Co-authored-by: Qing Tao <55564570+VickyAtAZ@users.noreply.github.com> Co-authored-by: Tommy Walton <82672795+amzn-tommy@users.noreply.github.com> Co-authored-by: SJ Co-authored-by: AMZN-Olex <5432499+AMZN-Olex@users.noreply.github.com> Co-authored-by: Scott Romero <24445312+AMZN-ScottR@users.noreply.github.com> Co-authored-by: amzn-tommy Co-authored-by: dmcdiar Co-authored-by: Benjamin Jillich <43751992+amzn-jillich@users.noreply.github.com> Co-authored-by: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com> Co-authored-by: puvvadar Co-authored-by: AMZN-nggieber <52797929+AMZN-nggieber@users.noreply.github.com> Co-authored-by: pereslav Co-authored-by: LesaelR <89800757+LesaelR@users.noreply.github.com> Co-authored-by: Gene Walters --- .../Entity/PrefabEditorEntityOwnershipInterface.h | 2 ++ .../Entity/PrefabEditorEntityOwnershipService.cpp | 9 +++++++++ .../Entity/PrefabEditorEntityOwnershipService.h | 2 ++ .../AzToolsFramework/Prefab/PrefabPublicHandler.cpp | 5 +++++ 4 files changed, 18 insertions(+) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipInterface.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipInterface.h index feb2fc12bf..3bd6e656ed 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipInterface.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipInterface.h @@ -56,5 +56,7 @@ namespace AzToolsFramework virtual void StopPlayInEditor() = 0; virtual void CreateNewLevelPrefab(AZStd::string_view filename, const AZStd::string& templateFilename) = 0; + + virtual bool IsRootPrefabAssigned() const = 0; }; } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp index 0f2b896b40..e5daf2674d 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.cpp @@ -79,6 +79,8 @@ namespace AzToolsFramework void PrefabEditorEntityOwnershipService::Reset() { + m_isRootPrefabAssigned = false; + if (m_rootInstance) { AzToolsFramework::ToolsApplicationRequestBus::Broadcast( @@ -203,6 +205,7 @@ namespace AzToolsFramework m_rootInstance->SetTemplateSourcePath(m_loaderInterface->GenerateRelativePath(filename)); m_rootInstance->SetContainerEntityName("Level"); m_prefabSystemComponent->PropagateTemplateChanges(templateId); + m_isRootPrefabAssigned = true; return true; } @@ -302,6 +305,12 @@ namespace AzToolsFramework } m_prefabSystemComponent->PropagateTemplateChanges(templateId); + m_isRootPrefabAssigned = true; + } + + bool PrefabEditorEntityOwnershipService::IsRootPrefabAssigned() const + { + return m_isRootPrefabAssigned; } Prefab::InstanceOptionalReference PrefabEditorEntityOwnershipService::CreatePrefab( diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.h b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.h index a98fce8059..a172e9550c 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.h +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/PrefabEditorEntityOwnershipService.h @@ -167,6 +167,7 @@ namespace AzToolsFramework void StopPlayInEditor() override; void CreateNewLevelPrefab(AZStd::string_view filename, const AZStd::string& templateFilename) override; + bool IsRootPrefabAssigned() const override; protected: @@ -215,5 +216,6 @@ namespace AzToolsFramework Prefab::PrefabLoaderInterface* m_loaderInterface; AzFramework::EntityContextId m_entityContextId; AZ::SerializeContext m_serializeContext; + bool m_isRootPrefabAssigned = false; }; } diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp index 63f042be85..41538d54e8 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Prefab/PrefabPublicHandler.cpp @@ -329,6 +329,11 @@ namespace AzToolsFramework return AZ::Failure(AZStd::string("Could not instantiate prefab - internal error " "(PrefabEditorEntityOwnershipInterface unavailable).")); } + if (!prefabEditorEntityOwnershipInterface->IsRootPrefabAssigned()) + { + return AZ::Failure(AZStd::string("Could not instantiate prefab - no root prefab assigned. " + "Currently, prefabs can only be instantiated inside a level")); + } InstanceOptionalReference instanceToParentUnder; From 9f1a30817783671ecf544959190097c2adc3d5e4 Mon Sep 17 00:00:00 2001 From: Alex Peterson <26804013+AMZN-alexpete@users.noreply.github.com> Date: Tue, 28 Sep 2021 13:15:24 -0700 Subject: [PATCH 18/20] Fix transparency issue on MacOS (#4358) Signed-off-by: AMZN-alexpete <26804013+AMZN-alexpete@users.noreply.github.com> --- Code/Tools/ProjectManager/Source/ProjectsScreen.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Code/Tools/ProjectManager/Source/ProjectsScreen.cpp b/Code/Tools/ProjectManager/Source/ProjectsScreen.cpp index 81d4047db5..b313c05230 100644 --- a/Code/Tools/ProjectManager/Source/ProjectsScreen.cpp +++ b/Code/Tools/ProjectManager/Source/ProjectsScreen.cpp @@ -358,8 +358,9 @@ namespace O3DE::ProjectManager painter.drawPixmap(backgroundRect, m_background); // Draw a semi-transparent overlay to darken down the colors. - painter.setCompositionMode (QPainter::CompositionMode_DestinationIn); - const float overlayTransparency = 0.7f; + // Use SourceOver, DestinationIn will make background transparent on Mac + painter.setCompositionMode (QPainter::CompositionMode_SourceOver); + const float overlayTransparency = 0.3f; painter.fillRect(backgroundRect, QColor(0, 0, 0, static_cast(255.0f * overlayTransparency))); } From 8791ae7ed7562a83b388ec659cb4e5aaa63f9a8b Mon Sep 17 00:00:00 2001 From: Ken Pruiksma Date: Tue, 28 Sep 2021 15:31:59 -0500 Subject: [PATCH 19/20] Terrain feature processor improvements regarding material, mesh, and lod (#4303) * Terrain feature processor improvements regarding material, mesh, and lod - Now using a material with pbr lighting for terrain. Removed some of the old shader code that wasn't needed anymore - Move heightmap image declaration to the material so it's not needed in the object SRG anymore - Added prototype code (commented out) for smoothing the terrain with a b-spline weighting function - Mesh data is all made with a proper mesh asset now. - Added basic LOD support (no continuous LOD yet, but it does pop between lod levels) - Moved RenderCommon to be accessible publicly. It contains stencil refs that should be public. Signed-off-by: Ken Pruiksma * Fixing terrain's per object srg because of changes in the default per object srg. Signed-off-by: Ken Pruiksma * PR Fixes Signed-off-by: Ken Pruiksma * Removing unused static. Signed-off-by: Ken Pruiksma --- .../Atom/Feature}/RenderCommon.h | 0 .../CoreLights/CoreLightsSystemComponent.cpp | 3 +- .../Code/Source/Mesh/MeshFeatureProcessor.cpp | 3 +- .../Source/ReflectionProbe/ReflectionProbe.h | 2 +- .../atom_feature_common_editor_files.cmake | 2 +- .../Code/atom_feature_common_files.cmake | 2 +- .../Terrain/DefaultPbrTerrain.material | 11 + .../Materials/Terrain/PbrTerrain.materialtype | 140 +++++ .../Assets/Shaders/Terrain/Terrain.azsl | 95 ---- .../Assets/Shaders/Terrain/Terrain.shader | 33 -- .../Shaders/Terrain/TerrainCommon.azsli | 154 +++++- .../Terrain/TerrainPBR_ForwardPass.azsl | 134 +++++ .../Terrain/TerrainPBR_ForwardPass.shader | 42 ++ .../Shaders/Terrain/Terrain_DepthPass.azsl | 1 + .../Shaders/Terrain/Terrain_Shadowmap.shader | 26 + Gems/Terrain/Code/CMakeLists.txt | 1 + .../TerrainFeatureProcessor.cpp | 506 ++++++++---------- .../TerrainRenderer/TerrainFeatureProcessor.h | 128 ++--- 18 files changed, 770 insertions(+), 513 deletions(-) rename Gems/Atom/Feature/Common/Code/{Source => Include/Atom/Feature}/RenderCommon.h (100%) create mode 100644 Gems/Terrain/Assets/Materials/Terrain/DefaultPbrTerrain.material create mode 100644 Gems/Terrain/Assets/Materials/Terrain/PbrTerrain.materialtype delete mode 100644 Gems/Terrain/Assets/Shaders/Terrain/Terrain.azsl delete mode 100644 Gems/Terrain/Assets/Shaders/Terrain/Terrain.shader create mode 100644 Gems/Terrain/Assets/Shaders/Terrain/TerrainPBR_ForwardPass.azsl create mode 100644 Gems/Terrain/Assets/Shaders/Terrain/TerrainPBR_ForwardPass.shader create mode 100644 Gems/Terrain/Assets/Shaders/Terrain/Terrain_Shadowmap.shader diff --git a/Gems/Atom/Feature/Common/Code/Source/RenderCommon.h b/Gems/Atom/Feature/Common/Code/Include/Atom/Feature/RenderCommon.h similarity index 100% rename from Gems/Atom/Feature/Common/Code/Source/RenderCommon.h rename to Gems/Atom/Feature/Common/Code/Include/Atom/Feature/RenderCommon.h diff --git a/Gems/Atom/Feature/Common/Code/Source/CoreLights/CoreLightsSystemComponent.cpp b/Gems/Atom/Feature/Common/Code/Source/CoreLights/CoreLightsSystemComponent.cpp index a2c6b8a67e..947028610c 100644 --- a/Gems/Atom/Feature/Common/Code/Source/CoreLights/CoreLightsSystemComponent.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/CoreLights/CoreLightsSystemComponent.cpp @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -33,8 +34,6 @@ #include #include -#include - namespace AZ { namespace Render diff --git a/Gems/Atom/Feature/Common/Code/Source/Mesh/MeshFeatureProcessor.cpp b/Gems/Atom/Feature/Common/Code/Source/Mesh/MeshFeatureProcessor.cpp index 3fcda71998..cced38c3a7 100644 --- a/Gems/Atom/Feature/Common/Code/Source/Mesh/MeshFeatureProcessor.cpp +++ b/Gems/Atom/Feature/Common/Code/Source/Mesh/MeshFeatureProcessor.cpp @@ -6,10 +6,9 @@ * */ -#include - #include #include +#include #include #include #include diff --git a/Gems/Atom/Feature/Common/Code/Source/ReflectionProbe/ReflectionProbe.h b/Gems/Atom/Feature/Common/Code/Source/ReflectionProbe/ReflectionProbe.h index 5b56b70ec8..bee304c5b9 100644 --- a/Gems/Atom/Feature/Common/Code/Source/ReflectionProbe/ReflectionProbe.h +++ b/Gems/Atom/Feature/Common/Code/Source/ReflectionProbe/ReflectionProbe.h @@ -9,7 +9,7 @@ #pragma once #include -#include +#include #include #include #include diff --git a/Gems/Atom/Feature/Common/Code/atom_feature_common_editor_files.cmake b/Gems/Atom/Feature/Common/Code/atom_feature_common_editor_files.cmake index 59a29f04b6..8262500ca1 100644 --- a/Gems/Atom/Feature/Common/Code/atom_feature_common_editor_files.cmake +++ b/Gems/Atom/Feature/Common/Code/atom_feature_common_editor_files.cmake @@ -7,6 +7,7 @@ # set(FILES + Include/Atom/Feature/RenderCommon.h Include/Atom/Feature/Utils/EditorRenderComponentAdapter.h Include/Atom/Feature/Utils/EditorRenderComponentAdapter.inl Include/Atom/Feature/Utils/EditorLightingPreset.h @@ -16,7 +17,6 @@ set(FILES Source/EditorCommonSystemComponent.cpp Source/EditorCommonSystemComponent.h Source/CommonModule.cpp - Source/RenderCommon.h Source/Material/ConvertEmissiveUnitFunctorSourceData.cpp Source/Material/ConvertEmissiveUnitFunctorSourceData.h Source/Material/MaterialConverterSystemComponent.cpp diff --git a/Gems/Atom/Feature/Common/Code/atom_feature_common_files.cmake b/Gems/Atom/Feature/Common/Code/atom_feature_common_files.cmake index 92f96a591b..9372d6594a 100644 --- a/Gems/Atom/Feature/Common/Code/atom_feature_common_files.cmake +++ b/Gems/Atom/Feature/Common/Code/atom_feature_common_files.cmake @@ -8,6 +8,7 @@ set(FILES 3rdParty/ACES/ACES/Aces.h + Include/Atom/Feature/RenderCommon.h Include/Atom/Feature/ACES/AcesDisplayMapperFeatureProcessor.h Include/Atom/Feature/Automation/AtomAutomationBus.h Include/Atom/Feature/AuxGeom/AuxGeomFeatureProcessor.h @@ -52,7 +53,6 @@ set(FILES Source/FrameCaptureSystemComponent.h Source/ProfilingCaptureSystemComponent.cpp Source/ProfilingCaptureSystemComponent.h - Source/RenderCommon.h 3rdParty/ACES/ACES/Aces.cpp Source/ACES/AcesDisplayMapperFeatureProcessor.cpp Source/AuxGeom/AuxGeomBase.h diff --git a/Gems/Terrain/Assets/Materials/Terrain/DefaultPbrTerrain.material b/Gems/Terrain/Assets/Materials/Terrain/DefaultPbrTerrain.material new file mode 100644 index 0000000000..095fbba21a --- /dev/null +++ b/Gems/Terrain/Assets/Materials/Terrain/DefaultPbrTerrain.material @@ -0,0 +1,11 @@ +{ + "description": "", + "materialType": "PbrTerrain.materialtype", + "parentMaterial": "", + "propertyLayoutVersion": 1, + "properties": { + "settings": { + "baseColor": [ 0.78, 0.59, 0.48 ] + } + } +} diff --git a/Gems/Terrain/Assets/Materials/Terrain/PbrTerrain.materialtype b/Gems/Terrain/Assets/Materials/Terrain/PbrTerrain.materialtype new file mode 100644 index 0000000000..bf6cbbb850 --- /dev/null +++ b/Gems/Terrain/Assets/Materials/Terrain/PbrTerrain.materialtype @@ -0,0 +1,140 @@ +{ + "description": "A material for rendering terrain with a physically-based rendering (PBR) material shading model.", + "propertyLayout": { + "version": 1, + "groups": [ + { + "id": "settings", + "displayName": "Settings" + } + ], + "properties": { + "general": [ + { + "id": "applySpecularAA", + "displayName": "Apply Specular AA", + "description": "Whether to apply specular anti-aliasing in the shader.", + "type": "Bool", + "defaultValue": false, + "connection": { + "type": "ShaderOption", + "id": "o_applySpecularAA" + } + }, + { + "id": "enableShadows", + "displayName": "Enable Shadows", + "description": "Whether to use the shadow maps.", + "type": "Bool", + "defaultValue": true, + "connection": { + "type": "ShaderOption", + "id": "o_enableShadows" + } + }, + { + "id": "enableDirectionalLights", + "displayName": "Enable Directional Lights", + "description": "Whether to use directional lights.", + "type": "Bool", + "defaultValue": true, + "connection": { + "type": "ShaderOption", + "id": "o_enableDirectionalLights" + } + }, + { + "id": "enablePunctualLights", + "displayName": "Enable Punctual Lights", + "description": "Whether to use punctual lights.", + "type": "Bool", + "defaultValue": true, + "connection": { + "type": "ShaderOption", + "id": "o_enablePunctualLights" + } + }, + { + "id": "enableAreaLights", + "displayName": "Enable Area Lights", + "description": "Whether to use area lights.", + "type": "Bool", + "defaultValue": true, + "connection": { + "type": "ShaderOption", + "id": "o_enableAreaLights" + } + }, + { + "id": "enableIBL", + "displayName": "Enable IBL", + "description": "Whether to use Image Based Lighting (IBL).", + "type": "Bool", + "defaultValue": true, + "connection": { + "type": "ShaderOption", + "id": "o_enableIBL" + } + }, + { + "id": "forwardPassIBLSpecular", + "displayName": "Forward Pass IBL Specular", + "description": "Whether to apply IBL specular in the forward pass.", + "type": "Bool", + "defaultValue": false, + "connection": { + "type": "ShaderOption", + "id": "o_materialUseForwardPassIBLSpecular" + } + } + ], + "settings": [ + { + "id": "heightmapImage", + "displayName": "Heightmap Image", + "description": "Heightmap of the terrain, controlled by the runtime.", + "type": "Image", + "connection": { + "type": "ShaderInput", + "id": "m_heightmapImage" + } + }, + { + "id": "baseColor", + "displayName": "Base Color", + "type": "Color", + "defaultValue": [ 0.18, 0.18, 0.18 ], + "connection": { + "type": "ShaderInput", + "id": "m_baseColor" + } + }, + { + "id": "roughness", + "displayName": "Roughness", + "type": "Float", + "defaultValue": 1.0, + "min": 0.0, + "max": 1.0, + "connection": { + "type": "ShaderInput", + "id": "m_roughness" + } + } + ] + } + }, + "shaders": [ + { + "file": "../../Shaders/Terrain/TerrainPBR_ForwardPass.shader" + }, + { + "file": "../../Shaders/Terrain/Terrain_Shadowmap.shader" + }, + { + "file": "../../Shaders/Terrain/Terrain_DepthPass.shader" + } + ], + "functors": [ + ] +} diff --git a/Gems/Terrain/Assets/Shaders/Terrain/Terrain.azsl b/Gems/Terrain/Assets/Shaders/Terrain/Terrain.azsl deleted file mode 100644 index 50d11ca610..0000000000 --- a/Gems/Terrain/Assets/Shaders/Terrain/Terrain.azsl +++ /dev/null @@ -1,95 +0,0 @@ -/* - * 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 -#include -#include "TerrainCommon.azsli" - -struct VertexOutput -{ - linear centroid float4 m_position : SV_Position; - float3 m_normal : NORMAL; - float3 m_tangent : TANGENT; - float3 m_bitangent : BITANGENT; - float m_height : UV; -}; - -struct ForwardPassOutput -{ - float4 m_diffuseColor : SV_Target0; //!< RGB = Diffuse Lighting, A = Blend Alpha (for blended surfaces) OR A = special encoding of surfaceScatteringFactor, m_subsurfaceScatteringQuality, o_enableSubsurfaceScattering - float4 m_specularColor : SV_Target1; //!< RGB = Specular Lighting, A = Unused - float4 m_albedo : SV_Target2; //!< RGB = Surface albedo pre-multiplied by other factors that will be multiplied later by diffuse GI, A = specularOcclusion - float4 m_specularF0 : SV_Target3; //!< RGB = Specular F0, A = roughness - float4 m_normal : SV_Target4; //!< RGB10 = EncodeNormalSignedOctahedron(worldNormal), A2 = multiScatterCompensationEnabled -}; - -struct PixelOutput -{ - float4 m_color : SV_Target0; -}; - -VertexOutput MainVS(in VertexInput input) -{ - VertexOutput output; - ObjectSrg::TerrainData terrainData = ObjectSrg::m_terrainData; - - float2 uv = input.m_uv; - float2 origUv = lerp(terrainData.m_uvMin, terrainData.m_uvMax, uv); - output.m_position = GetTerrainProjectedPosition(terrainData, input.m_position, origUv); - - // Calculate normal - float up = GetHeight(origUv + terrainData.m_uvStep * float2( 0.0f, -1.0f)); - float right = GetHeight(origUv + terrainData.m_uvStep * float2( 1.0f, 0.0f)); - float down = GetHeight(origUv + terrainData.m_uvStep * float2( 0.0f, 1.0f)); - float left = GetHeight(origUv + terrainData.m_uvStep * float2(-1.0f, 0.0f)); - - output.m_bitangent = normalize(float3(0.0, terrainData.m_sampleSpacing * 2.0f, down - up)); - output.m_tangent = normalize(float3(terrainData.m_sampleSpacing * 2.0f, 0.0, right - left)); - output.m_normal = cross(output.m_tangent, output.m_bitangent); - - output.m_height = GetHeight(origUv); - return output; -} - -ForwardPassOutput MainPS(in VertexOutput input) -{ - ForwardPassOutput output; - - float3 lightDirection = normalize(float3(-1.0, 1.0, -1.0)); - float3 lightIntensity = float3(1.0, 1.0, 1.0); - if (SceneSrg::m_directionalLightCount > 0) - { - lightDirection = SceneSrg::m_directionalLights[0].m_direction; - lightIntensity = SceneSrg::m_directionalLights[0].m_rgbIntensityLux; - } - - // Fake light intensity ranges from 1.0 for normals directly facing the light to zero for those - // directly facing away. - const float minLight = 0.01; - const float midLight = 0.1; - float lightDot = dot(normalize(input.m_normal), -lightDirection); - lightIntensity *= lightDot > 0.0 ? - lightDot * (1.0 - midLight) + midLight : // surface facing light - (lightDot + 1.0) * (midLight - minLight) + minLight; // surface facing away - - output.m_diffuseColor.rgb = 0.5 * lightIntensity; - output.m_diffuseColor.a = 0.0f; - - output.m_specularColor.rgb = 0.0; - - output.m_albedo.rgb = 0.25 + input.m_height * 0.5; - output.m_albedo.a = 0.0; - - output.m_specularF0.rgb = 0.04; - output.m_specularF0.a = 1.0; - - output.m_normal.rgb = input.m_normal; - output.m_normal.a = 0.0; - - return output; -} diff --git a/Gems/Terrain/Assets/Shaders/Terrain/Terrain.shader b/Gems/Terrain/Assets/Shaders/Terrain/Terrain.shader deleted file mode 100644 index e844a54a21..0000000000 --- a/Gems/Terrain/Assets/Shaders/Terrain/Terrain.shader +++ /dev/null @@ -1,33 +0,0 @@ -{ - "Source" : "./Terrain.azsl", - - "DepthStencilState" : - { - "Depth" : - { - "Enable" : true, - "CompareFunc" : "GreaterEqual" - } - }, - - "DrawList" : "forward", - - "ProgramSettings": - { - "EntryPoints": - [ - { - "name": "MainVS", - "type": "Vertex" - }, - { - "name": "MainPS", - "type": "Fragment" - } - ] - }, - - "BlendState" : { - "Enable" : false - } -} diff --git a/Gems/Terrain/Assets/Shaders/Terrain/TerrainCommon.azsli b/Gems/Terrain/Assets/Shaders/Terrain/TerrainCommon.azsli index 18b85bbd43..9df7b35ce8 100644 --- a/Gems/Terrain/Assets/Shaders/Terrain/TerrainCommon.azsli +++ b/Gems/Terrain/Assets/Shaders/Terrain/TerrainCommon.azsli @@ -9,18 +9,6 @@ ShaderResourceGroup ObjectSrg : SRG_PerObject { - Texture2D m_heightmapImage; - - Sampler PointSampler - { - MinFilter = Point; - MagFilter = Point; - MipFilter = Point; - AddressU = Clamp; - AddressV = Clamp; - AddressW = Clamp; - }; - row_major float3x4 m_modelToWorld; struct TerrainData @@ -33,14 +21,131 @@ ShaderResourceGroup ObjectSrg : SRG_PerObject }; TerrainData m_terrainData; + + // The below shouldn't be in this SRG but needs to be for now because the lighting functions depend on them. + + //! Reflection Probe (smallest probe volume that overlaps the object position) + struct ReflectionProbeData + { + row_major float3x4 m_modelToWorld; + row_major float3x4 m_modelToWorldInverse; // does not include extents + float3 m_outerObbHalfLengths; + float3 m_innerObbHalfLengths; + float m_padding; + bool m_useReflectionProbe; + bool m_useParallaxCorrection; + }; + + ReflectionProbeData m_reflectionProbeData; + TextureCube m_reflectionProbeCubeMap; + + float4x4 GetReflectionProbeWorldMatrix() + { + float4x4 modelToWorld = float4x4( + float4(1, 0, 0, 0), + float4(0, 1, 0, 0), + float4(0, 0, 1, 0), + float4(0, 0, 0, 1)); + + modelToWorld[0] = m_reflectionProbeData.m_modelToWorld[0]; + modelToWorld[1] = m_reflectionProbeData.m_modelToWorld[1]; + modelToWorld[2] = m_reflectionProbeData.m_modelToWorld[2]; + return modelToWorld; + } + + float4x4 GetReflectionProbeWorldMatrixInverse() + { + float4x4 modelToWorldInverse = float4x4( + float4(1, 0, 0, 0), + float4(0, 1, 0, 0), + float4(0, 0, 1, 0), + float4(0, 0, 0, 1)); + + modelToWorldInverse[0] = m_reflectionProbeData.m_modelToWorldInverse[0]; + modelToWorldInverse[1] = m_reflectionProbeData.m_modelToWorldInverse[1]; + modelToWorldInverse[2] = m_reflectionProbeData.m_modelToWorldInverse[2]; + return modelToWorldInverse; + } } +ShaderResourceGroup TerrainMaterialSrg : SRG_PerMaterial +{ + Texture2D m_heightmapImage; + + Sampler HeightmapSampler + { + MinFilter = Linear; + MagFilter = Linear; + MipFilter = Point; + AddressU = Clamp; + AddressV = Clamp; + AddressW = Clamp; + }; + + float3 m_baseColor; + float m_roughness; +} + +option bool o_useTerrainSmoothing = false; + struct VertexInput { float2 m_position : POSITION; float2 m_uv : UV; }; +// Sample a texture with a 5 tap B-Spline. Consider ripping this out and putting in a more general location. +// This function samples a 4x4 neighborhood around the uv. Normally this would take 16 samples, but by taking +// advantage of bilinear filtering this can be done with 9 taps on the edges between pixels. The cost is further +// reduced by dropping the diagonals. +float SampleBSpline5Tap(Texture2D texture, SamplerState textureSampler, float2 uv, float2 textureSize, float2 rcpTextureSize) +{ + // Think of sample locations in the 4x4 neighborhood as having a top left coordinate of 0,0 and + // a bottom right coordinate of 3,3. + + // Find the position in texture space then round it to get the center of the 1,1 pixel (tc1) + float2 texelPos = uv * textureSize; + float2 tc1= floor(texelPos - 0.5) + 0.5; + + // Offset from center position to texel + float2 f = texelPos - tc1; + + // Compute B-Spline weights based on the offset + float2 OneMinusF = (1.0 - f); + float2 OneMinusF2 = OneMinusF * OneMinusF; + float2 OneMinusF3 = OneMinusF2 * OneMinusF; + float2 w0 = OneMinusF3; + float2 w1 = 4.0 + 3.0 * f * f * f - 6.0 * f * f; + float2 w2 = 4.0 + 3.0 * OneMinusF3 - 6.0 * OneMinusF2; + float2 w3 = f * f * f; + + float2 w12 = w1 + w2; + + // Compute uv coordinates for sampling the texture + float2 tc0 = (tc1 - 1.0f) * rcpTextureSize; + float2 tc3 = (tc1 + 2.0f) * rcpTextureSize; + float2 tc12 = (tc1 + w2 / w12) * rcpTextureSize; + + // Compute sample weights + float sw0 = w12.x * w12.y; // middle + float sw1 = w12.x * w0.y; // top + float sw2 = w0.x * w12.y; // left + float sw3 = w12.x * w3.y; // bottom + float sw4 = w3.x * w12.y; // right + + // total weight of samples to normalize result. + float totalWeight = sw0 + sw1 + sw2 + sw3 + sw4; + + float result = 0.0f; + result += texture.SampleLevel(textureSampler, float2(tc12.x, tc12.y), 0.0).r * sw0; + result += texture.SampleLevel(textureSampler, float2(tc12.x, tc0.y), 0.0).r * sw1; + result += texture.SampleLevel(textureSampler, float2( tc0.x, tc12.y), 0.0).r * sw2; + result += texture.SampleLevel(textureSampler, float2(tc12.x, tc3.y), 0.0).r * sw3; + result += texture.SampleLevel(textureSampler, float2( tc3.x, tc12.y), 0.0).r * sw4; + + return result / totalWeight; +} + float4x4 GetObject_WorldMatrix() { float4x4 modelToWorld = float4x4( @@ -58,10 +163,23 @@ float4x4 GetObject_WorldMatrix() float GetHeight(float2 origUv) { float2 uv = clamp(origUv + (ObjectSrg::m_terrainData.m_uvStep * 0.5f), 0.0f, 1.0f); - return ObjectSrg::m_terrainData.m_heightScale * (ObjectSrg::m_heightmapImage.SampleLevel(ObjectSrg::PointSampler, uv, 0).r - 0.5f); + float height = 0.0f; + + if (o_useTerrainSmoothing) + { + float2 textureSize; + TerrainMaterialSrg::m_heightmapImage.GetDimensions(textureSize.x, textureSize.y); + height = SampleBSpline5Tap(TerrainMaterialSrg::m_heightmapImage, TerrainMaterialSrg::HeightmapSampler, uv, textureSize, rcp(textureSize)); + } + else + { + height = TerrainMaterialSrg::m_heightmapImage.SampleLevel(TerrainMaterialSrg::HeightmapSampler, uv, 0).r; + } + + return ObjectSrg::m_terrainData.m_heightScale * (height - 0.5f); } -float4 GetTerrainProjectedPosition(ObjectSrg::TerrainData terrainData, float2 vertexPosition, float2 uv) +float3 GetTerrainWorldPosition(ObjectSrg::TerrainData terrainData, float2 vertexPosition, float2 uv) { // Remove all vertices outside our bounds by turning them into NaN positions. if (any(uv > 1.0) || any (uv < 0.0)) @@ -71,6 +189,10 @@ float4 GetTerrainProjectedPosition(ObjectSrg::TerrainData terrainData, float2 ve // Loop up the height and calculate our final position. float height = GetHeight(uv); - float4 worldPosition = mul(GetObject_WorldMatrix(), float4(vertexPosition, height, 1.0f)); - return mul(ViewSrg::m_viewProjectionMatrix, worldPosition); + return mul(GetObject_WorldMatrix(), float4(vertexPosition, height, 1.0f)).xyz; +} + +float4 GetTerrainProjectedPosition(ObjectSrg::TerrainData terrainData, float2 vertexPosition, float2 uv) +{ + return mul(ViewSrg::m_viewProjectionMatrix, float4(GetTerrainWorldPosition(terrainData, vertexPosition, uv), 1.0)); } diff --git a/Gems/Terrain/Assets/Shaders/Terrain/TerrainPBR_ForwardPass.azsl b/Gems/Terrain/Assets/Shaders/Terrain/TerrainPBR_ForwardPass.azsl new file mode 100644 index 0000000000..d77b944498 --- /dev/null +++ b/Gems/Terrain/Assets/Shaders/Terrain/TerrainPBR_ForwardPass.azsl @@ -0,0 +1,134 @@ +/* + * 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 +#include +#include +#include +#include +#include +#include +#include +#include +#include + +struct VSOutput +{ + float4 m_position : SV_Position; + float3 m_normal: NORMAL; + float3 m_tangent : TANGENT; + float3 m_bitangent : BITANGENT; + float3 m_worldPosition : UV0; + float3 m_shadowCoords[ViewSrg::MaxCascadeCount] : UV2; + float2 m_uv : UV1; +}; + +VSOutput TerrainPBR_MainPassVS(VertexInput IN) +{ + VSOutput OUT; + + ObjectSrg::TerrainData terrainData = ObjectSrg::m_terrainData; + + float2 uv = IN.m_uv; + float2 origUv = lerp(terrainData.m_uvMin, terrainData.m_uvMax, uv); + float3 worldPosition = GetTerrainWorldPosition(terrainData, IN.m_position, origUv); + OUT.m_position = mul(ViewSrg::m_viewProjectionMatrix, float4(worldPosition, 1.0)); + OUT.m_worldPosition = worldPosition; + + // Calculate normal + float up = GetHeight(origUv + terrainData.m_uvStep * float2( 0.0f, -1.0f)); + float right = GetHeight(origUv + terrainData.m_uvStep * float2( 1.0f, 0.0f)); + float down = GetHeight(origUv + terrainData.m_uvStep * float2( 0.0f, 1.0f)); + float left = GetHeight(origUv + terrainData.m_uvStep * float2(-1.0f, 0.0f)); + + OUT.m_bitangent = normalize(float3(0.0, terrainData.m_sampleSpacing * 2.0f, down - up)); + OUT.m_tangent = normalize(float3(terrainData.m_sampleSpacing * 2.0f, 0.0, right - left)); + OUT.m_normal = cross(OUT.m_tangent, OUT.m_bitangent); + OUT.m_uv = uv; + + // directional light shadow + const uint shadowIndex = ViewSrg::m_shadowIndexDirectionalLight; + if (o_enableShadows && shadowIndex < SceneSrg::m_directionalLightCount) + { + DirectionalLightShadow::GetShadowCoords( + shadowIndex, + worldPosition, + OUT.m_shadowCoords); + } + + return OUT; +} + +ForwardPassOutput TerrainPBR_MainPassPS(VSOutput IN) +{ + // ------- Surface ------- + + Surface surface; + + // Position, Normal, Roughness + surface.position = IN.m_worldPosition.xyz; + surface.normal = normalize(IN.m_normal); + surface.roughnessLinear = TerrainMaterialSrg::m_roughness; + surface.CalculateRoughnessA(); + + // Albedo, SpecularF0 + const float specularF0Factor = 0.5f; + float3 color = TerrainMaterialSrg::m_baseColor; + surface.SetAlbedoAndSpecularF0(color, specularF0Factor, 0.0); + + // Clear Coat, Transmission + surface.clearCoat.InitializeToZero(); + surface.transmission.InitializeToZero(); + + // ------- LightingData ------- + + LightingData lightingData; + + // Light iterator + lightingData.tileIterator.Init(IN.m_position, PassSrg::m_lightListRemapped, PassSrg::m_tileLightData); + lightingData.Init(surface.position, surface.normal, surface.roughnessLinear); + + // Shadow, Occlusion + lightingData.shadowCoords = IN.m_shadowCoords; + + // Diffuse and Specular response + lightingData.specularResponse = FresnelSchlickWithRoughness(lightingData.NdotV, surface.specularF0, surface.roughnessLinear); + lightingData.diffuseResponse = 1.0f - lightingData.specularResponse; + + const float alpha = 1.0f; + + // ------- Lighting Calculation ------- + + // Apply Decals + ApplyDecals(lightingData.tileIterator, surface); + + // Apply Direct Lighting + ApplyDirectLighting(surface, lightingData); + + // Apply Image Based Lighting (IBL) + ApplyIBL(surface, lightingData); + + // Finalize Lighting + lightingData.FinalizeLighting(surface.transmission.tint); + + PbrLightingOutput lightingOutput = GetPbrLightingOutput(surface, lightingData, alpha); + + // ------- Output ------- + + ForwardPassOutput OUT; + + OUT.m_diffuseColor = lightingOutput.m_diffuseColor; + OUT.m_diffuseColor.w = -1; // Subsurface scattering is disabled + OUT.m_specularColor = lightingOutput.m_specularColor; + OUT.m_specularF0 = lightingOutput.m_specularF0; + OUT.m_albedo = lightingOutput.m_albedo; + OUT.m_normal = lightingOutput.m_normal; + + return OUT; +} diff --git a/Gems/Terrain/Assets/Shaders/Terrain/TerrainPBR_ForwardPass.shader b/Gems/Terrain/Assets/Shaders/Terrain/TerrainPBR_ForwardPass.shader new file mode 100644 index 0000000000..0e6f0beb1d --- /dev/null +++ b/Gems/Terrain/Assets/Shaders/Terrain/TerrainPBR_ForwardPass.shader @@ -0,0 +1,42 @@ +{ + "Source" : "./TerrainPBR_ForwardPass.azsl", + + "DepthStencilState" : + { + "Depth" : + { + "Enable" : true, + "CompareFunc" : "GreaterEqual" + }, + "Stencil" : + { + "Enable" : true, + "ReadMask" : "0x00", + "WriteMask" : "0xFF", + "FrontFace" : + { + "Func" : "Always", + "DepthFailOp" : "Keep", + "FailOp" : "Keep", + "PassOp" : "Replace" + } + } + }, + + "ProgramSettings": + { + "EntryPoints": + [ + { + "name": "TerrainPBR_MainPassVS", + "type": "Vertex" + }, + { + "name": "TerrainPBR_MainPassPS", + "type": "Fragment" + } + ] + }, + + "DrawList" : "forward" +} diff --git a/Gems/Terrain/Assets/Shaders/Terrain/Terrain_DepthPass.azsl b/Gems/Terrain/Assets/Shaders/Terrain/Terrain_DepthPass.azsl index 20c56323ac..fd855a1bcd 100644 --- a/Gems/Terrain/Assets/Shaders/Terrain/Terrain_DepthPass.azsl +++ b/Gems/Terrain/Assets/Shaders/Terrain/Terrain_DepthPass.azsl @@ -8,6 +8,7 @@ #include #include #include "TerrainCommon.azsli" +#include struct VSDepthOutput { diff --git a/Gems/Terrain/Assets/Shaders/Terrain/Terrain_Shadowmap.shader b/Gems/Terrain/Assets/Shaders/Terrain/Terrain_Shadowmap.shader new file mode 100644 index 0000000000..290c83bddf --- /dev/null +++ b/Gems/Terrain/Assets/Shaders/Terrain/Terrain_Shadowmap.shader @@ -0,0 +1,26 @@ +{ + "Source" : "Terrain_DepthPass", + + "DepthStencilState" : { + "Depth" : { "Enable" : true, "CompareFunc" : "LessEqual" } + }, + + "DrawList" : "shadow", + + "RasterState" : + { + "depthBias" : "10", + "depthBiasSlopeScale" : "4" + }, + + "ProgramSettings": + { + "EntryPoints": + [ + { + "name": "MainVS", + "type": "Vertex" + } + ] + } +} diff --git a/Gems/Terrain/Code/CMakeLists.txt b/Gems/Terrain/Code/CMakeLists.txt index fc07294dab..17ac3d8474 100644 --- a/Gems/Terrain/Code/CMakeLists.txt +++ b/Gems/Terrain/Code/CMakeLists.txt @@ -21,6 +21,7 @@ ly_add_target( AZ::AzFramework Gem::Atom_RPI.Public Gem::Atom_Utils.Static + Gem::Atom_Feature_Common Gem::GradientSignal Gem::SurfaceData Gem::LmbrCentral diff --git a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp index 511b206b84..3e5f9a0608 100644 --- a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp +++ b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp @@ -20,28 +20,39 @@ #include #include #include +#include #include #include +#include #include #include +#include +#include +#include +#include +#include +#include #include #include #include #include #include - +#include namespace Terrain { namespace { - const uint32_t DEFAULT_UploadBufferSize = 512 * 1024; // 512k [[maybe_unused]] const char* TerrainFPName = "TerrainFeatureProcessor"; } + namespace MaterialInputs + { + static const char* const HeightmapImage("settings.heightmapImage"); + } + namespace ShaderInputs { - static const char* const HeightmapImage("m_heightmapImage"); static const char* const ModelToWorld("m_modelToWorld"); static const char* const TerrainData("m_terrainData"); } @@ -60,142 +71,44 @@ namespace Terrain void TerrainFeatureProcessor::Activate() { m_areaData = {}; - - InitializeAtomStuff(); - EnableSceneNotification(); + Initialize(); } - void TerrainFeatureProcessor::ConfigurePipelineState(ShaderState& shaderState, bool assertOnFail) + void TerrainFeatureProcessor::Initialize() { - if (shaderState.m_shader == nullptr) { - AZ_Assert(shaderState.m_shader || !assertOnFail, "Terrain shader failed to load correctly."); - return; - } - - bool success = GetParentScene()->ConfigurePipelineState(shaderState.m_shader->GetDrawListTag(), shaderState.m_pipelineStateDescriptor); - AZ_Assert(success || !assertOnFail, "Couldn't configure the pipeline state."); - if (success) - { - shaderState.m_pipelineState = shaderState.m_shader->AcquirePipelineState(shaderState.m_pipelineStateDescriptor); - AZ_Assert(shaderState.m_pipelineState, "Failed to acquire default pipeline state."); - } - } - - void TerrainFeatureProcessor::InitializeAtomStuff() - { - m_rhiSystem = AZ::RHI::RHISystemInterface::Get(); - - { - auto LoadShader = [this](const char* filePath, ShaderState& shaderState) - { - shaderState.m_shader = AZ::RPI::LoadShader(filePath); - if (!shaderState.m_shader) + // Load the terrain material asynchronously + const AZStd::string materialFilePath = "Materials/Terrain/DefaultPbrTerrain.azmaterial"; + m_materialAssetLoader = AZStd::make_unique(); + *m_materialAssetLoader = AZ::RPI::AssetUtils::AsyncAssetLoader::Create(materialFilePath, 0u, + [&](AZ::Data::Asset assetData, bool success) -> void { - AZ_Error(TerrainFPName, false, "Failed to find or create a shader instance from shader asset '%s'", filePath); - return; + const AZ::Data::Asset& materialAsset = static_cast>(assetData); + if (success) + { + m_materialInstance = AZ::RPI::Material::FindOrCreate(assetData); + if (!materialAsset->GetObjectSrgLayout()) + { + AZ_Error("TerrainFeatureProcessor", false, "No per-object ShaderResourceGroup found on terrain material."); + } + } } - - // Create the data layout - shaderState.m_pipelineStateDescriptor = AZ::RHI::PipelineStateDescriptorForDraw{}; - { - AZ::RHI::InputStreamLayoutBuilder layoutBuilder; - - layoutBuilder.AddBuffer() - ->Channel("POSITION", AZ::RHI::Format::R32G32_FLOAT) - ->Channel("UV", AZ::RHI::Format::R32G32_FLOAT) - ; - shaderState.m_pipelineStateDescriptor.m_inputStreamLayout = layoutBuilder.End(); - } - - auto shaderVariant = shaderState.m_shader->GetVariant(AZ::RPI::ShaderAsset::RootShaderVariantStableId); - shaderVariant.ConfigurePipelineState(shaderState.m_pipelineStateDescriptor); - - // If this fails to run now, it's ok, we'll initialize it in OnRenderPipelineAdded later. - ConfigurePipelineState(shaderState, false); - }; - - LoadShader("Shaders/Terrain/Terrain.azshader", m_shaderStates[ShaderType::Forward]); - LoadShader("Shaders/Terrain/Terrain_DepthPass.azshader", m_shaderStates[ShaderType::Depth]); - - // Forward and depth shader use same srg layout. - AZ::RHI::Ptr perObjectSrgLayout = - m_shaderStates[ShaderType::Forward].m_shader->FindShaderResourceGroupLayout(AZ::Name{"ObjectSrg"}); - - if (!perObjectSrgLayout) - { - AZ_Error(TerrainFPName, false, "Failed to get shader resource group layout"); - return; - } - else if (!perObjectSrgLayout->IsFinalized()) - { - AZ_Error(TerrainFPName, false, "Shader resource group layout is not loaded"); - return; - } - - m_heightmapImageIndex = perObjectSrgLayout->FindShaderInputImageIndex(AZ::Name(ShaderInputs::HeightmapImage)); - AZ_Error(TerrainFPName, m_heightmapImageIndex.IsValid(), "Failed to find shader input image %s.", ShaderInputs::HeightmapImage); - - m_modelToWorldIndex = perObjectSrgLayout->FindShaderInputConstantIndex(AZ::Name(ShaderInputs::ModelToWorld)); - AZ_Error(TerrainFPName, m_modelToWorldIndex.IsValid(), "Failed to find shader input constant %s.", ShaderInputs::ModelToWorld); - - m_terrainDataIndex = perObjectSrgLayout->FindShaderInputConstantIndex(AZ::Name(ShaderInputs::TerrainData)); - AZ_Error(TerrainFPName, m_terrainDataIndex.IsValid(), "Failed to find shader input constant %s.", ShaderInputs::TerrainData); + ); } - AZ::RHI::BufferPoolDescriptor dmaPoolDescriptor; - dmaPoolDescriptor.m_heapMemoryLevel = AZ::RHI::HeapMemoryLevel::Host; - dmaPoolDescriptor.m_bindFlags = AZ::RHI::BufferBindFlags::InputAssembly; - - m_hostPool = AZ::RHI::Factory::Get().CreateBufferPool(); - m_hostPool->SetName(AZ::Name("TerrainVertexPool")); - AZ::RHI::ResultCode resultCode = m_hostPool->Init(*m_rhiSystem->GetDevice(), dmaPoolDescriptor); - - if (resultCode != AZ::RHI::ResultCode::Success) - { - AZ_Error(TerrainFPName, false, "Failed to create host buffer pool from RPI"); - return; - } - - InitializeTerrainPatch(); - - if (!InitializeRenderBuffers()) + if (!InitializePatchModel()) { AZ_Error(TerrainFPName, false, "Failed to create Terrain render buffers!"); return; } } - void TerrainFeatureProcessor::OnRenderPipelineAdded([[maybe_unused]] AZ::RPI::RenderPipelinePtr pipeline) - { - for (ShaderState& shaderState: m_shaderStates) - { - ConfigurePipelineState(shaderState, true); - } - } - - void TerrainFeatureProcessor::OnRenderPipelineRemoved([[maybe_unused]] AZ::RPI::RenderPipeline* pipeline) - { - } - - void TerrainFeatureProcessor::OnRenderPipelinePassesChanged([[maybe_unused]] AZ::RPI::RenderPipeline* renderPipeline) - { - } - - void TerrainFeatureProcessor::Deactivate() { DisableSceneNotification(); - DestroyRenderBuffers(); + m_patchModel = {}; m_areaData = {}; - - if (m_hostPool) - { - m_hostPool.reset(); - } - - m_rhiSystem = nullptr; } void TerrainFeatureProcessor::Render(const AZ::RPI::FeatureProcessor::RenderPacket& packet) @@ -254,240 +167,269 @@ namespace Terrain { AZ_PROFILE_FUNCTION(AzRender); - if ((m_shaderStates[ShaderType::Forward].m_shader == nullptr) || - (m_shaderStates[ShaderType::Depth].m_shader == nullptr) || - m_shaderStates[ShaderType::Forward].m_shader->GetDrawListTag().IsNull() || - m_shaderStates[ShaderType::Depth].m_shader->GetDrawListTag().IsNull()) - { - return; - } - if (!m_areaData.m_terrainBounds.IsValid()) { return; } - if (m_areaData.m_propertiesDirty) + if (m_areaData.m_propertiesDirty && m_materialInstance) { m_areaData.m_propertiesDirty = false; m_sectorData.clear(); - AZ::RHI::DrawPacketBuilder drawPacketBuilder; + AZ::RPI::MaterialPropertyIndex heightmapPropertyIndex = + m_materialInstance->GetMaterialPropertiesLayout()->FindPropertyIndex(AZ::Name(MaterialInputs::HeightmapImage)); + AZ_Error(TerrainFPName, heightmapPropertyIndex.IsValid(), "Failed to find material input constant %s.", MaterialInputs::HeightmapImage); + AZ::Data::Instance heightmapImage = m_areaData.m_heightmapImage; + m_materialInstance->SetPropertyValue(heightmapPropertyIndex, heightmapImage); + m_materialInstance->Compile(); - uint32_t numIndices = static_cast(m_gridIndices.size()); + const auto layout = m_materialInstance->GetAsset()->GetObjectSrgLayout(); - AZ::RHI::DrawIndexed drawIndexed; - drawIndexed.m_indexCount = numIndices; - drawIndexed.m_indexOffset = 0; - drawIndexed.m_vertexOffset = 0; + m_modelToWorldIndex = layout->FindShaderInputConstantIndex(AZ::Name(ShaderInputs::ModelToWorld)); + AZ_Error(TerrainFPName, m_modelToWorldIndex.IsValid(), "Failed to find shader input constant %s.", ShaderInputs::ModelToWorld); + + m_terrainDataIndex = layout->FindShaderInputConstantIndex(AZ::Name(ShaderInputs::TerrainData)); + AZ_Error(TerrainFPName, m_terrainDataIndex.IsValid(), "Failed to find shader input constant %s.", ShaderInputs::TerrainData); float xFirstPatchStart = - m_areaData.m_terrainBounds.GetMin().GetX() - fmod(m_areaData.m_terrainBounds.GetMin().GetX(), m_gridMeters); - float xLastPatchStart = m_areaData.m_terrainBounds.GetMax().GetX() - fmod(m_areaData.m_terrainBounds.GetMax().GetX(), m_gridMeters); + m_areaData.m_terrainBounds.GetMin().GetX() - fmod(m_areaData.m_terrainBounds.GetMin().GetX(), GridMeters); + float xLastPatchStart = m_areaData.m_terrainBounds.GetMax().GetX() - fmod(m_areaData.m_terrainBounds.GetMax().GetX(), GridMeters); float yFirstPatchStart = - m_areaData.m_terrainBounds.GetMin().GetY() - fmod(m_areaData.m_terrainBounds.GetMin().GetY(), m_gridMeters); - float yLastPatchStart = m_areaData.m_terrainBounds.GetMax().GetY() - fmod(m_areaData.m_terrainBounds.GetMax().GetY(), m_gridMeters); + m_areaData.m_terrainBounds.GetMin().GetY() - fmod(m_areaData.m_terrainBounds.GetMin().GetY(), GridMeters); + float yLastPatchStart = m_areaData.m_terrainBounds.GetMax().GetY() - fmod(m_areaData.m_terrainBounds.GetMax().GetY(), GridMeters); - for (float yPatch = yFirstPatchStart; yPatch <= yLastPatchStart; yPatch += m_gridMeters) + for (float yPatch = yFirstPatchStart; yPatch <= yLastPatchStart; yPatch += GridMeters) { - for (float xPatch = xFirstPatchStart; xPatch <= xLastPatchStart; xPatch += m_gridMeters) + for (float xPatch = xFirstPatchStart; xPatch <= xLastPatchStart; xPatch += GridMeters) { - drawPacketBuilder.Begin(nullptr); - drawPacketBuilder.SetDrawArguments(drawIndexed); - drawPacketBuilder.SetIndexBufferView(m_indexBufferView); - auto& forwardShader = m_shaderStates[ShaderType::Forward].m_shader; - - auto resourceGroup = AZ::RPI::ShaderResourceGroup::Create(forwardShader->GetAsset(), forwardShader->GetSupervariantIndex(), AZ::Name("ObjectSrg")); - if (!resourceGroup) + const auto& materialAsset = m_materialInstance->GetAsset(); + auto& shaderAsset = materialAsset->GetMaterialTypeAsset()->GetShaderAssetForObjectSrg(); + auto objectSrg = AZ::RPI::ShaderResourceGroup::Create(shaderAsset, materialAsset->GetObjectSrgLayout()->GetName()); + if (!objectSrg) { - AZ_Error(TerrainFPName, false, "Failed to create shader resource group"); - return; + AZ_Warning("TerrainFeatureProcessor", false, "Failed to create a new shader resource group, skipping."); + continue; } - AZStd::array uvMin = { 0.0f, 0.0f }; - AZStd::array uvMax = { 1.0f, 1.0f }; + { // Update SRG + + AZStd::array uvMin = { 0.0f, 0.0f }; + AZStd::array uvMax = { 1.0f, 1.0f }; - uvMin[0] = (float)((xPatch - m_areaData.m_terrainBounds.GetMin().GetX()) / m_areaData.m_terrainBounds.GetXExtent()); - uvMin[1] = (float)((yPatch - m_areaData.m_terrainBounds.GetMin().GetY()) / m_areaData.m_terrainBounds.GetYExtent()); + uvMin[0] = (float)((xPatch - m_areaData.m_terrainBounds.GetMin().GetX()) / m_areaData.m_terrainBounds.GetXExtent()); + uvMin[1] = (float)((yPatch - m_areaData.m_terrainBounds.GetMin().GetY()) / m_areaData.m_terrainBounds.GetYExtent()); - uvMax[0] = - (float)(((xPatch + m_gridMeters) - m_areaData.m_terrainBounds.GetMin().GetX()) / m_areaData.m_terrainBounds.GetXExtent()); - uvMax[1] = - (float)(((yPatch + m_gridMeters) - m_areaData.m_terrainBounds.GetMin().GetY()) / m_areaData.m_terrainBounds.GetYExtent()); + uvMax[0] = + (float)(((xPatch + GridMeters) - m_areaData.m_terrainBounds.GetMin().GetX()) / m_areaData.m_terrainBounds.GetXExtent()); + uvMax[1] = + (float)(((yPatch + GridMeters) - m_areaData.m_terrainBounds.GetMin().GetY()) / m_areaData.m_terrainBounds.GetYExtent()); - AZStd::array uvStep = - { - 1.0f / m_areaData.m_heightmapImageWidth, 1.0f / m_areaData.m_heightmapImageHeight, - }; + AZStd::array uvStep = + { + 1.0f / m_areaData.m_heightmapImageWidth, 1.0f / m_areaData.m_heightmapImageHeight, + }; - AZ::Transform transform = m_areaData.m_transform; - transform.SetTranslation(xPatch, yPatch, m_areaData.m_transform.GetTranslation().GetZ()); + AZ::Transform transform = m_areaData.m_transform; + transform.SetTranslation(xPatch, yPatch, m_areaData.m_transform.GetTranslation().GetZ()); - AZ::Matrix3x4 matrix3x4 = AZ::Matrix3x4::CreateFromTransform(transform); + AZ::Matrix3x4 matrix3x4 = AZ::Matrix3x4::CreateFromTransform(transform); - resourceGroup->SetImage(m_heightmapImageIndex, m_areaData.m_heightmapImage); - resourceGroup->SetConstant(m_modelToWorldIndex, matrix3x4); + objectSrg->SetConstant(m_modelToWorldIndex, matrix3x4); - ShaderTerrainData terrainDataForSrg; - terrainDataForSrg.m_sampleSpacing = m_areaData.m_sampleSpacing; - terrainDataForSrg.m_heightScale = m_areaData.m_heightScale; - terrainDataForSrg.m_uvMin = uvMin; - terrainDataForSrg.m_uvMax = uvMax; - terrainDataForSrg.m_uvStep = uvStep; - resourceGroup->SetConstant(m_terrainDataIndex, terrainDataForSrg); + ShaderTerrainData terrainDataForSrg; + terrainDataForSrg.m_sampleSpacing = m_areaData.m_sampleSpacing; + terrainDataForSrg.m_heightScale = m_areaData.m_heightScale; + terrainDataForSrg.m_uvMin = uvMin; + terrainDataForSrg.m_uvMax = uvMax; + terrainDataForSrg.m_uvStep = uvStep; + objectSrg->SetConstant(m_terrainDataIndex, terrainDataForSrg); - resourceGroup->Compile(); - drawPacketBuilder.AddShaderResourceGroup(resourceGroup->GetRHIShaderResourceGroup()); - - auto addDrawItem = [&](ShaderState& shaderState) - { - AZ::RHI::DrawPacketBuilder::DrawRequest drawRequest; - drawRequest.m_listTag = shaderState.m_shader->GetDrawListTag(); - drawRequest.m_pipelineState = shaderState.m_pipelineState.get(); - drawRequest.m_streamBufferViews = AZStd::array_view(&m_vertexBufferView, 1); - drawPacketBuilder.AddDrawItem(drawRequest); - }; - - for (ShaderState& shaderState : m_shaderStates) - { - addDrawItem(shaderState); + objectSrg->Compile(); } - //addDrawItem(m_shaderStates[ShaderType::Forward]); - - m_sectorData.emplace_back( - drawPacketBuilder.End(), + + m_sectorData.push_back(); + SectorData& sectorData = m_sectorData.back(); + + for (auto& lod : m_patchModel->GetLods()) + { + AZ::RPI::ModelLod& modelLod = *lod.get(); + sectorData.m_drawPackets.emplace_back(modelLod, 0, m_materialInstance, objectSrg); + AZ::RPI::MeshDrawPacket& drawPacket = sectorData.m_drawPackets.back(); + + // set the shader option to select forward pass IBL specular if necessary + if (!drawPacket.SetShaderOption(AZ::Name("o_meshUseForwardPassIBLSpecular"), AZ::RPI::ShaderOptionValue{ false })) + { + AZ_Warning("MeshDrawPacket", false, "Failed to set o_meshUseForwardPassIBLSpecular on mesh draw packet"); + } + uint8_t stencilRef = AZ::Render::StencilRefs::UseDiffuseGIPass | AZ::Render::StencilRefs::UseIBLSpecularPass; + drawPacket.SetStencilRef(stencilRef); + drawPacket.Update(*GetParentScene(), true); + } + + sectorData.m_aabb = AZ::Aabb::CreateFromMinMax( AZ::Vector3(xPatch, yPatch, m_areaData.m_terrainBounds.GetMin().GetZ()), - AZ::Vector3(xPatch + m_gridMeters, yPatch + m_gridMeters, m_areaData.m_terrainBounds.GetMax().GetZ()) - ), - resourceGroup - ); + AZ::Vector3(xPatch + GridMeters, yPatch + GridMeters, m_areaData.m_terrainBounds.GetMax().GetZ()) + ); + sectorData.m_srg = objectSrg; } } } - - for (auto& view : process.m_views) + + for (auto& sectorData : m_sectorData) { - AZ::Frustum viewFrustum = AZ::Frustum::CreateFromMatrixColumnMajor(view->GetWorldToClipMatrix()); - for (auto& sectorData : m_sectorData) + uint8_t lodChoice = AZ::RPI::ModelLodAsset::LodCountMax; + + // Go through all cameras and choose an LOD based on the closest camera. + for (auto& view : process.m_views) { + if ((view->GetUsageFlags() & AZ::RPI::View::UsageFlags::UsageCamera) > 0) + { + AZ::Vector3 cameraPosition = view->GetCameraTransform().GetTranslation(); + AZ::Vector2 cameraPositionXY = AZ::Vector2(cameraPosition.GetX(), cameraPosition.GetY()); + AZ::Vector2 sectorCenterXY = AZ::Vector2(sectorData.m_aabb.GetCenter().GetX(), sectorData.m_aabb.GetCenter().GetY()); + + float sectorDistance = sectorCenterXY.GetDistance(cameraPositionXY); + float lodForCamera = ceilf(AZ::GetMax(0.0f, log2f(sectorDistance / (GridMeters * 4.0f)))); + lodChoice = AZ::GetMin(lodChoice, aznumeric_cast(lodForCamera)); + } + } + + // Add the correct LOD draw packet for visible sectors. + for (auto& view : process.m_views) + { + AZ::Frustum viewFrustum = AZ::Frustum::CreateFromMatrixColumnMajor(view->GetWorldToClipMatrix()); if (viewFrustum.IntersectAabb(sectorData.m_aabb) != AZ::IntersectResult::Exterior) { - view->AddDrawPacket(sectorData.m_drawPacket.get()); + uint8_t lodToRender = AZ::GetMin(lodChoice, aznumeric_cast(sectorData.m_drawPackets.size() - 1)); + view->AddDrawPacket(sectorData.m_drawPackets.at(lodToRender).GetRHIDrawPacket()); } } } } - void TerrainFeatureProcessor::InitializeTerrainPatch() + void TerrainFeatureProcessor::InitializeTerrainPatch(uint16_t gridSize, float gridSpacing, PatchData& patchdata) { - m_gridVertices.clear(); - m_gridIndices.clear(); + patchdata.m_positions.clear(); + patchdata.m_uvs.clear(); + patchdata.m_indices.clear(); - for (float y = 0.0f; y < m_gridMeters; y += m_gridSpacing) + uint16_t gridVertices = gridSize + 1; // For m_gridSize quads, (m_gridSize + 1) vertices are needed. + size_t size = gridVertices * gridVertices; + + patchdata.m_positions.reserve(size); + patchdata.m_uvs.reserve(size); + + for (uint16_t y = 0; y < gridVertices; ++y) { - for (float x = 0.0f; x < m_gridMeters; x += m_gridSpacing) + for (uint16_t x = 0; x < gridVertices; ++x) { - float x0 = x; - float x1 = x + m_gridSpacing; - float y0 = y; - float y1 = y + m_gridSpacing; + patchdata.m_positions.push_back({ aznumeric_cast(x) * gridSpacing, aznumeric_cast(y) * gridSpacing }); + patchdata.m_uvs.push_back({ aznumeric_cast(x) / gridSize, aznumeric_cast(y) / gridSize }); + } + } - uint16_t startIndex = (uint16_t)(m_gridVertices.size()); + patchdata.m_indices.reserve(gridSize * gridSize * 6); // total number of quads, 2 triangles with 6 indices per quad. + + for (uint16_t y = 0; y < gridSize; ++y) + { + for (uint16_t x = 0; x < gridSize; ++x) + { + uint16_t topLeft = y * gridVertices + x; + uint16_t topRight = topLeft + 1; + uint16_t bottomLeft = (y + 1) * gridVertices + x; + uint16_t bottomRight = bottomLeft + 1; - m_gridVertices.emplace_back(x0, y0, x0 / m_gridMeters, y0 / m_gridMeters); - m_gridVertices.emplace_back(x1, y0, x1 / m_gridMeters, y0 / m_gridMeters); - m_gridVertices.emplace_back(x0, y1, x0 / m_gridMeters, y1 / m_gridMeters); - m_gridVertices.emplace_back(x1, y1, x1 / m_gridMeters, y1 / m_gridMeters); - - m_gridIndices.emplace_back(startIndex); - m_gridIndices.emplace_back(aznumeric_cast(startIndex + 1)); - m_gridIndices.emplace_back(aznumeric_cast(startIndex + 2)); - m_gridIndices.emplace_back(aznumeric_cast(startIndex + 1)); - m_gridIndices.emplace_back(aznumeric_cast(startIndex + 3)); - m_gridIndices.emplace_back(aznumeric_cast(startIndex + 2)); + patchdata.m_indices.emplace_back(topLeft); + patchdata.m_indices.emplace_back(topRight); + patchdata.m_indices.emplace_back(bottomLeft); + patchdata.m_indices.emplace_back(bottomLeft); + patchdata.m_indices.emplace_back(topRight); + patchdata.m_indices.emplace_back(bottomRight); } } } - - bool TerrainFeatureProcessor::InitializeRenderBuffers() + + AZ::Outcome> TerrainFeatureProcessor::CreateBufferAsset( + const void* data, const AZ::RHI::BufferViewDescriptor& bufferViewDescriptor, const AZStd::string& bufferName) { - AZ::RHI::ResultCode result = AZ::RHI::ResultCode::Fail; + AZ::RPI::BufferAssetCreator creator; + creator.Begin(AZ::Uuid::CreateRandom()); - // Create geometry buffers - m_indexBuffer = AZ::RHI::Factory::Get().CreateBuffer(); - m_vertexBuffer = AZ::RHI::Factory::Get().CreateBuffer(); + AZ::RHI::BufferDescriptor bufferDescriptor; + bufferDescriptor.m_bindFlags = AZ::RHI::BufferBindFlags::InputAssembly | AZ::RHI::BufferBindFlags::ShaderRead; + bufferDescriptor.m_byteCount = static_cast(bufferViewDescriptor.m_elementSize) * static_cast(bufferViewDescriptor.m_elementCount); - m_indexBuffer->SetName(AZ::Name("TerrainIndexBuffer")); - m_vertexBuffer->SetName(AZ::Name("TerrainVertexBuffer")); + creator.SetBuffer(data, bufferDescriptor.m_byteCount, bufferDescriptor); + creator.SetBufferViewDescriptor(bufferViewDescriptor); + creator.SetUseCommonPool(AZ::RPI::CommonBufferPoolType::StaticInputAssembly); - AZStd::vector> buffers = { m_indexBuffer , m_vertexBuffer }; - - // Fill our buffers with the vertex/index data - for (size_t bufferIndex = 0; bufferIndex < buffers.size(); ++bufferIndex) + AZ::Data::Asset bufferAsset; + if (creator.End(bufferAsset)) { - AZ::RHI::Ptr buffer = buffers[bufferIndex]; + bufferAsset.SetHint(bufferName); + return AZ::Success(bufferAsset); + } - // Initialize the buffer + return AZ::Failure(); + } - AZ::RHI::BufferInitRequest bufferRequest; - bufferRequest.m_descriptor = AZ::RHI::BufferDescriptor{ AZ::RHI::BufferBindFlags::InputAssembly, DEFAULT_UploadBufferSize }; - bufferRequest.m_buffer = buffer.get(); + bool TerrainFeatureProcessor::InitializePatchModel() + { + AZ::RPI::ModelAssetCreator modelAssetCreator; + modelAssetCreator.Begin(AZ::Uuid::CreateRandom()); - result = m_hostPool->InitBuffer(bufferRequest); + uint16_t gridSize = GridSize; + float gridSpacing = GridSpacing; - if (result != AZ::RHI::ResultCode::Success) + for (uint32_t i = 0; i < AZ::RPI::ModelLodAsset::LodCountMax && gridSize > 0; ++i) + { + PatchData patchData; + InitializeTerrainPatch(gridSize, gridSpacing, patchData); + + auto positionBufferViewDesc = AZ::RHI::BufferViewDescriptor::CreateTyped(0, aznumeric_cast(patchData.m_positions.size()), AZ::RHI::Format::R32G32_FLOAT); + auto positionsOutcome = CreateBufferAsset(patchData.m_positions.data(), positionBufferViewDesc, "TerrainPatchPositions"); + + auto uvBufferViewDesc = AZ::RHI::BufferViewDescriptor::CreateTyped(0, aznumeric_cast(patchData.m_uvs.size()), AZ::RHI::Format::R32G32_FLOAT); + auto uvsOutcome = CreateBufferAsset(patchData.m_uvs.data(), uvBufferViewDesc, "TerrainPatchUvs"); + + auto indexBufferViewDesc = AZ::RHI::BufferViewDescriptor::CreateTyped(0, aznumeric_cast(patchData.m_indices.size()), AZ::RHI::Format::R16_UINT); + auto indicesOutcome = CreateBufferAsset(patchData.m_indices.data(), indexBufferViewDesc, "TerrainPatchIndices"); + + if (!positionsOutcome.IsSuccess() || !uvsOutcome.IsSuccess() || !indicesOutcome.IsSuccess()) { AZ_Error(TerrainFPName, false, "Failed to create GPU buffers for Terrain"); return false; } + + AZ::RPI::ModelLodAssetCreator modelLodAssetCreator; + modelLodAssetCreator.Begin(AZ::Uuid::CreateRandom()); - // Grab a pointer to the buffer's data + modelLodAssetCreator.BeginMesh(); + modelLodAssetCreator.AddMeshStreamBuffer(AZ::RHI::ShaderSemantic{ "POSITION" }, AZ::Name(), {positionsOutcome.GetValue(), positionBufferViewDesc}); + modelLodAssetCreator.AddMeshStreamBuffer(AZ::RHI::ShaderSemantic{ "UV" }, AZ::Name(), {uvsOutcome.GetValue(), uvBufferViewDesc}); + modelLodAssetCreator.SetMeshIndexBuffer({indicesOutcome.GetValue(), indexBufferViewDesc}); - m_hostPool->OrphanBuffer(*buffer); + AZ::Aabb aabb = AZ::Aabb::CreateFromMinMax(AZ::Vector3(0.0, 0.0, 0.0), AZ::Vector3(GridMeters, GridMeters, 0.0)); + modelLodAssetCreator.SetMeshAabb(AZStd::move(aabb)); + modelLodAssetCreator.SetMeshName(AZ::Name("Terrain Patch")); + modelLodAssetCreator.EndMesh(); - AZ::RHI::BufferMapResponse mapResponse; - m_hostPool->MapBuffer(AZ::RHI::BufferMapRequest(*buffer, 0, DEFAULT_UploadBufferSize), mapResponse); + AZ::Data::Asset modelLodAsset; + modelLodAssetCreator.End(modelLodAsset); + + modelAssetCreator.AddLodAsset(AZStd::move(modelLodAsset)); - auto* mappedData = reinterpret_cast(mapResponse.m_data); - - //0th index should always be the index buffer - if (bufferIndex == 0) - { - // Fill the index buffer with our terrain patch indices - const uint64_t idxSize = m_gridIndices.size() * sizeof(uint16_t); - memcpy(mappedData, m_gridIndices.data(), idxSize); - - m_indexBufferView = AZ::RHI::IndexBufferView( - *buffer, 0, static_cast(idxSize), AZ::RHI::IndexFormat::Uint16); - } - else - { - // Fill the vertex buffer with our terrain patch vertices - const uint64_t elementSize = m_gridVertices.size() * sizeof(Vertex); - memcpy(mappedData, m_gridVertices.data(), elementSize); - - m_vertexBufferView = AZ::RHI::StreamBufferView( - *buffer, 0, static_cast(elementSize), static_cast(sizeof(Vertex))); - } - - m_hostPool->UnmapBuffer(*buffer); + gridSize = gridSize / 2; + gridSpacing *= 2.0f; } - return true; - } + AZ::Data::Asset modelAsset; + bool success = modelAssetCreator.End(modelAsset); - void TerrainFeatureProcessor::DestroyRenderBuffers() - { - m_indexBuffer.reset(); - m_vertexBuffer.reset(); + m_patchModel = AZ::RPI::Model::FindOrCreate(modelAsset); - m_indexBufferView = {}; - m_vertexBufferView = {}; - - for (ShaderState& shaderState : m_shaderStates) - { - shaderState.Reset(); - } + return success; } } diff --git a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.h b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.h index 382f473b25..160d3113c2 100644 --- a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.h +++ b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.h @@ -14,9 +14,11 @@ #include #include +#include #include #include +#include #include #include #include @@ -26,6 +28,15 @@ #include #include +namespace AZ::RPI +{ + namespace AssetUtils + { + class AsyncAssetLoader; + } + class Model; +} + namespace Terrain { class TerrainFeatureProcessor final @@ -57,28 +68,6 @@ namespace Terrain private: - // System-level references to the shader, pipeline, and shader-related information - enum ShaderType - { - Depth, - Forward, - Count, - }; - - struct ShaderState - { - AZ::Data::Instance m_shader; - AZ::RHI::ConstPtr m_pipelineState; - AZ::RHI::PipelineStateDescriptorForDraw m_pipelineStateDescriptor; - - void Reset() - { - m_shader.reset(); - m_pipelineState.reset(); - m_pipelineStateDescriptor = {}; - } - }; - struct ShaderTerrainData // Must align with struct in Object Srg { AZStd::array m_uvMin; @@ -87,62 +76,47 @@ namespace Terrain float m_sampleSpacing; float m_heightScale; }; - - // RPI::SceneNotificationBus overrides ... - void OnRenderPipelineAdded(AZ::RPI::RenderPipelinePtr pipeline) override; - void OnRenderPipelineRemoved(AZ::RPI::RenderPipeline* pipeline) override; - void OnRenderPipelinePassesChanged(AZ::RPI::RenderPipeline* renderPipeline) override; - - void InitializeAtomStuff(); - void ConfigurePipelineState(ShaderState& shaderState, bool assertOnFail); - - void InitializeTerrainPatch(); - - bool InitializeRenderBuffers(); - void DestroyRenderBuffers(); - - void ProcessSurfaces(const FeatureProcessor::RenderPacket& process); - - // System-level parameters - const float m_gridSpacing{ 1.0f }; - const float m_gridMeters{ 32.0f }; - - // System-level cached reference to the Atom RHI - AZ::RHI::RHISystemInterface* m_rhiSystem = nullptr; - - AZStd::array m_shaderStates; - - AZ::RHI::ShaderInputImageIndex m_heightmapImageIndex; - AZ::RHI::ShaderInputConstantIndex m_modelToWorldIndex; - AZ::RHI::ShaderInputConstantIndex m_terrainDataIndex; - - // Pos_float_2 + UV_float_2 - struct Vertex + + struct VertexPosition { float m_posx; float m_posy; - float m_u; - float m_v; - - Vertex(float posx, float posy, float u, float v) - : m_posx(posx) - , m_posy(posy) - , m_u(u) - , m_v(v) - { - } }; - // System-level definition of a grid patch. (ex: 32m x 32m) - AZStd::vector m_gridVertices; - AZStd::vector m_gridIndices; + struct VertexUv + { + float m_u; + float m_v; + }; - // System-level data related to the grid patch - AZ::RHI::Ptr m_hostPool = nullptr; - AZ::RHI::Ptr m_indexBuffer; - AZ::RHI::Ptr m_vertexBuffer; - AZ::RHI::IndexBufferView m_indexBufferView; - AZ::RHI::StreamBufferView m_vertexBufferView; + struct PatchData + { + AZStd::vector m_positions; + AZStd::vector m_uvs; + AZStd::vector m_indices; + }; + + void Initialize(); + void InitializeTerrainPatch(uint16_t gridSize, float gridSpacing, PatchData& patchdata); + bool InitializePatchModel(); + + void ProcessSurfaces(const FeatureProcessor::RenderPacket& process); + + AZ::Outcome> CreateBufferAsset( + const void* data, const AZ::RHI::BufferViewDescriptor& bufferViewDescriptor, const AZStd::string& bufferName); + + // System-level parameters + static constexpr float GridSpacing{ 1.0f }; + static constexpr uint32_t GridSize{ 64 }; // number of terrain quads (vertices are m_gridSize + 1) + static constexpr float GridMeters{ GridSpacing * GridSize }; + + AZStd::unique_ptr m_materialAssetLoader; + AZ::Data::Instance m_materialInstance; + + AZ::RHI::ShaderInputConstantIndex m_modelToWorldIndex; + AZ::RHI::ShaderInputConstantIndex m_terrainDataIndex; + + AZ::Data::Instance m_patchModel; // Per-area data struct TerrainAreaData @@ -161,15 +135,9 @@ namespace Terrain struct SectorData { - AZ::Data::Instance m_srg; + AZ::Data::Instance m_srg; // Hold on to ref so it's not dropped AZ::Aabb m_aabb; - AZStd::unique_ptr m_drawPacket; - - SectorData(const AZ::RHI::DrawPacket* drawPacket, AZ::Aabb aabb, AZ::Data::Instance srg) - : m_srg(srg) - , m_aabb(aabb) - , m_drawPacket(drawPacket) - {} + AZStd::fixed_vector m_drawPackets; }; AZStd::vector m_sectorData; From a692ae1567f7c8435832c2ad4e8884e75a6d984d Mon Sep 17 00:00:00 2001 From: Nicholas Van Sickle Date: Tue, 28 Sep 2021 14:57:09 -0700 Subject: [PATCH 20/20] Restore event listening behavior in EntityOutlinerListModel (#4254) Disables a problematic child reparenting code path in EditorEntityModel that led to the series of remove -> add -> remove notifications for reparented entities that led to unnecessary updates and a sporadic QSortFilterProxyModel crash Signed-off-by: nvsickle --- .../Entity/EditorEntityModel.cpp | 20 ++++++++++++++----- .../UI/Outliner/EntityOutlinerListModel.cpp | 7 +++++-- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntityModel.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntityModel.cpp index 8d96cc42fe..ce1bf84db0 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntityModel.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/Entity/EditorEntityModel.cpp @@ -381,12 +381,22 @@ namespace AzToolsFramework return; } - //orphan any children that remain attached to the entity - auto children = entityInfo.GetChildren(); - for (auto childId : children) + bool isPrefabSystemEnabled = false; + AzFramework::ApplicationRequests::Bus::BroadcastResult( + isPrefabSystemEnabled, &AzFramework::ApplicationRequests::IsPrefabSystemEnabled); + + // For slices, orphan any children that remain attached to the entity + // For prefabs, this is an unneeded operation because the prefab system handles the orphans + // and the extra reparenting operation can be problematic for consumers subscribed to entity + // events, such as the entity outliner. + if (!isPrefabSystemEnabled) { - ReparentChild(childId, AZ::EntityId(), entityId); - m_entityOrphanTable[entityId].insert(childId); + auto children = entityInfo.GetChildren(); + for (auto childId : children) + { + ReparentChild(childId, AZ::EntityId(), entityId); + m_entityOrphanTable[entityId].insert(childId); + } } m_savedOrderInfo[entityId] = AZStd::make_pair(entityInfo.GetParent(), entityInfo.GetIndexForSorting()); diff --git a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerListModel.cpp b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerListModel.cpp index b3ac8b4c73..2872c1bce3 100644 --- a/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerListModel.cpp +++ b/Code/Framework/AzToolsFramework/AzToolsFramework/UI/Outliner/EntityOutlinerListModel.cpp @@ -1344,7 +1344,10 @@ namespace AzToolsFramework //add/remove operations trigger selection change signals which assert and break undo/redo operations in progress in inspector etc. //so disallow selection updates until change is complete emit EnableSelectionUpdates(false); - beginResetModel(); + + auto parentIndex = GetIndexFromEntity(parentId); + auto childIndex = GetIndexFromEntity(childId); + beginRemoveRows(parentIndex, childIndex.row(), childIndex.row()); } void EntityOutlinerListModel::OnEntityInfoUpdatedRemoveChildEnd(AZ::EntityId parentId, AZ::EntityId childId) @@ -1352,7 +1355,7 @@ namespace AzToolsFramework (void)childId; AZ_PROFILE_FUNCTION(AzToolsFramework); - endResetModel(); + endRemoveRows(); //must refresh partial lock/visibility of parents m_isFilterDirty = true;