From d2e2d660c28af55315d71bb3afc48f526aa6bea0 Mon Sep 17 00:00:00 2001 From: Panos Karabelas Date: Sun, 19 Nov 2023 22:59:01 +0000 Subject: [PATCH] [Commands] Fixed undo for Transform --- .../Implementation/ImGui_TransformGizmo.h | 42 ++++++------- runtime/Commands/CommandStack.cpp | 18 +++--- runtime/Commands/CommandStack.h | 1 - ...ansformEntity.cpp => CommandTransform.cpp} | 61 +++++++------------ .../{TransformEntity.h => CommandTransform.h} | 4 +- 5 files changed, 52 insertions(+), 74 deletions(-) rename runtime/Commands/{TransformEntity.cpp => CommandTransform.cpp} (52%) rename runtime/Commands/{TransformEntity.h => CommandTransform.h} (90%) diff --git a/editor/ImGui/Implementation/ImGui_TransformGizmo.h b/editor/ImGui/Implementation/ImGui_TransformGizmo.h index 8699bf162..a3c66b55e 100644 --- a/editor/ImGui/Implementation/ImGui_TransformGizmo.h +++ b/editor/ImGui/Implementation/ImGui_TransformGizmo.h @@ -29,19 +29,15 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #include "Rendering/Renderer.h" #include "Input/Input.h" #include "Commands/CommandStack.h" -#include "Commands/TransformEntity.h" +#include "Commands/CommandTransform.h" #include "Engine.h" //====================================== namespace ImGui::TransformGizmo { - static inline bool transform_command_done = false; + bool first_frame_in_control = true; - static inline Spartan::Math::Vector3 begin_position; - static inline Spartan::Math::Quaternion begin_rotation; - static inline Spartan::Math::Vector3 begin_scale; - - static void apply_style() + void apply_style() { ImGuizmo::Style& style = ImGuizmo::GetStyle(); style.TranslationLineThickness = 6.0f; @@ -103,46 +99,44 @@ namespace ImGui::TransformGizmo ImGuizmo::SetOrthographic(is_orthographic); ImGuizmo::BeginFrame(); - // Map transform to ImGuizmo + // map transform to ImGuizmo Spartan::Math::Vector3 position = transform->GetPosition(); Spartan::Math::Vector3 scale = transform->GetScale(); Spartan::Math::Quaternion rotation = transform->GetRotation(); Spartan::Math::Matrix transform_matrix = Spartan::Math::Matrix::GenerateRowFirst(position, rotation, scale); - // Set viewport rectangle + // set viewport rectangle ImGuizmo::SetDrawlist(); ImGuizmo::SetRect(ImGui::GetWindowPos().x, ImGui::GetWindowPos().y, ImGui::GetWindowWidth(), ImGui::GetWindowHeight()); ImGuizmo::Manipulate(&matrix_view.m00, &matrix_projection.m00, transform_operation, transform_space, &transform_matrix.m00, nullptr, nullptr); - // Map ImGuizmo to transform + // map imguizmo to transform if (ImGuizmo::IsUsing()) { + if (first_frame_in_control) + { + // add the old transform to the command stack + Spartan::CommandStack::Apply( + entity.get(), + transform->GetPosition(), transform->GetRotation(), transform->GetScale() + ); + + first_frame_in_control = false; + } + transform_matrix.Transposed().Decompose(scale, rotation, position); transform->SetPosition(position); transform->SetRotation(rotation); transform->SetScale(scale); - - begin_position = position; - begin_rotation = rotation; - begin_scale = scale; - } - else - { - if (!transform_command_done) - { - SP_LOG_INFO("Applying command"); - Spartan::CommandStack::Apply(entity.get(), begin_position, begin_rotation, begin_scale); - transform_command_done = true; - } } } static bool allow_picking() { - transform_command_done = false; + first_frame_in_control = true; return !ImGuizmo::IsOver() && !ImGuizmo::IsUsing(); } } diff --git a/runtime/Commands/CommandStack.cpp b/runtime/Commands/CommandStack.cpp index b62528d05..19dbe0398 100644 --- a/runtime/Commands/CommandStack.cpp +++ b/runtime/Commands/CommandStack.cpp @@ -19,22 +19,24 @@ IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -//= INCLUDES ========================== +//= INCLUDES ============ #include "pch.h" #include "CommandStack.h" -//===================================== +//======================= +//= NAMESPACES ===== +using namespace std; +//================== namespace Spartan { - void CommandStack::Undo() { if (m_undo_buffer.size() == 0) return; // Fetch the latest applied command, so we know what we are undoing - std::shared_ptr undo_command = m_undo_buffer.back(); + shared_ptr undo_command = m_undo_buffer.back(); m_undo_buffer.pop_back(); // Actually undo @@ -50,7 +52,7 @@ namespace Spartan return; // Fetch the latest undoed command, so we know what we are redoing - std::shared_ptr redo_command = m_redo_buffer.back(); + shared_ptr redo_command = m_redo_buffer.back(); m_redo_buffer.pop_back(); // Actually redo @@ -60,8 +62,6 @@ namespace Spartan m_undo_buffer.push_back(redo_command); } - std::vector> CommandStack::m_undo_buffer; - - std::vector> CommandStack::m_redo_buffer; - + vector> CommandStack::m_undo_buffer; + vector> CommandStack::m_redo_buffer; } diff --git a/runtime/Commands/CommandStack.h b/runtime/Commands/CommandStack.h index 2b9512e9d..2ed6bf785 100644 --- a/runtime/Commands/CommandStack.h +++ b/runtime/Commands/CommandStack.h @@ -67,7 +67,6 @@ namespace Spartan static void Redo(); protected: - static std::vector> m_undo_buffer; static std::vector> m_redo_buffer; }; diff --git a/runtime/Commands/TransformEntity.cpp b/runtime/Commands/CommandTransform.cpp similarity index 52% rename from runtime/Commands/TransformEntity.cpp rename to runtime/Commands/CommandTransform.cpp index 3819f58c4..5ba6f638b 100644 --- a/runtime/Commands/TransformEntity.cpp +++ b/runtime/Commands/CommandTransform.cpp @@ -19,71 +19,56 @@ IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -//= INCLUDES ========================== +//= INCLUDES ================ #include "pch.h" -#include "TransformEntity.h" +#include "CommandTransform.h" #include "../World/World.h" -//===================================== +//=========================== +//= NAMESPACES ===== +using namespace std; +//================== namespace Spartan { - - TransformEntity::TransformEntity(Spartan::Entity* entity, Math::Vector3 old_position, Math::Quaternion old_rotation, Math::Vector3 old_scale) + CommandTransform::CommandTransform(Spartan::Entity* entity, Math::Vector3 old_position, Math::Quaternion old_rotation, Math::Vector3 old_scale) { SP_ASSERT(entity); - // In the current implementation of GetObjectId, it may seem unneccessary to not just store a (shared) pointer to the entity + // In the current implementation of GetObjectId, it may seem unnecessary to not just store a (shared) pointer to the entity // however if we ever move to a UUID based system, or hashed name system, and entities can be destroyed/created or unloaded/loaded with consistent ids // we want to actually store the ID and then resolve from that. // Right now this wont work as expected, since the object ids are just incremented on creation m_entity_id = entity->GetObjectId(); - std::shared_ptr transform = entity->GetTransform(); - SP_ASSERT(transform.get()); - - m_new_position = transform->GetPosition(); - m_new_rotation = transform->GetRotation(); - m_new_scale = transform->GetScale(); - - + m_new_position = entity->GetTransform()->GetPosition(); + m_new_rotation = entity->GetTransform()->GetRotation(); + m_new_scale = entity->GetTransform()->GetScale(); + m_old_position = old_position; m_old_rotation = old_rotation; - m_old_scale = old_scale; + m_old_scale = old_scale; } - void TransformEntity::OnApply() + void CommandTransform::OnApply() { - std::shared_ptr entity = Spartan::World::GetEntityById(m_entity_id); - - // this may likely happen in legitimate edge cases according to my experience + shared_ptr entity = Spartan::World::GetEntityById(m_entity_id); if (!entity) return; - std::shared_ptr transform = entity->GetTransform(); - - SP_ASSERT_MSG(transform, "We had entity, but it didn't have a valid transform."); - - transform->SetPosition(m_new_position); - transform->SetRotation(m_new_rotation); - transform->SetScale(m_new_scale); + entity->GetTransform()->SetPosition(m_new_position); + entity->GetTransform()->SetRotation(m_new_rotation); + entity->GetTransform()->SetScale(m_new_scale); } - void TransformEntity::OnRevert() + void CommandTransform::OnRevert() { - std::shared_ptr entity = Spartan::World::GetEntityById(m_entity_id); - - // this may likely happen in legitimate edge cases according to my experience + shared_ptr entity = Spartan::World::GetEntityById(m_entity_id); if (!entity) return; - std::shared_ptr transform = entity->GetTransform(); - - SP_ASSERT_MSG(transform, "We had entity, but it didn't have a valid transform."); - - transform->SetPosition(m_old_position); - transform->SetRotation(m_old_rotation); - transform->SetScale(m_old_scale); + entity->GetTransform()->SetPosition(m_old_position); + entity->GetTransform()->SetRotation(m_old_rotation); + entity->GetTransform()->SetScale(m_old_scale); } - } diff --git a/runtime/Commands/TransformEntity.h b/runtime/Commands/CommandTransform.h similarity index 90% rename from runtime/Commands/TransformEntity.h rename to runtime/Commands/CommandTransform.h index d45f60fc6..9c6e0cc4a 100644 --- a/runtime/Commands/TransformEntity.h +++ b/runtime/Commands/CommandTransform.h @@ -33,10 +33,10 @@ namespace Spartan /** * Baseclass for commands to be used in a CommandStack. */ - class SP_CLASS TransformEntity : public Spartan::Command + class SP_CLASS CommandTransform : public Spartan::Command { public: - TransformEntity(Spartan::Entity* entity, Math::Vector3 old_position, Math::Quaternion old_rotation, Math::Vector3 old_scale); + CommandTransform(Spartan::Entity* entity, Math::Vector3 old_position, Math::Quaternion old_rotation, Math::Vector3 old_scale); virtual void OnApply() override; virtual void OnRevert() override;