From a9a325774526f140bd1c99d32fd518e70f63fbf9 Mon Sep 17 00:00:00 2001 From: Possseidon Date: Fri, 27 Aug 2021 17:10:17 +0200 Subject: [PATCH] Some cleanup in Transform. - Removed static create in favor of just using make_unique/make_shared. - Added implicit conversion from a dual quaternion. - Added a hint, that copying is currently disabled. - Made full_transform_ mutable. - Fixed some refs/consts. --- dang-gl/include/dang-gl/Math/Transform.h | 19 +++++++------ dang-gl/include/dang-gl/Rendering/Camera.h | 2 +- dang-gl/src/Math/Transform.cpp | 32 ++++++++++------------ 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/dang-gl/include/dang-gl/Math/Transform.h b/dang-gl/include/dang-gl/Math/Transform.h index 34ad2907..d4c17fb8 100644 --- a/dang-gl/include/dang-gl/Math/Transform.h +++ b/dang-gl/include/dang-gl/Math/Transform.h @@ -23,8 +23,11 @@ class Transform { public: using Event = dutils::Event; - /// @brief Creates a new pointer-based transform. - static UniqueTransform create(); + /// @brief Allows for implicit construction from a dual quaternion. + Transform(const dquat& own_transform = {}); + + // TODO: Copy is currently disabled because of event subscription + // Implement copy to properly update the subscription /// @brief The own transformation, without any parent transform. const dquat& ownTransform() const; @@ -32,22 +35,22 @@ class Transform { void setOwnTransform(const dquat& transform); /// @brief The full transformation, including all parent transformations. - const dquat& fullTransform(); + const dquat& fullTransform() const; /// @brief The optional parent of this transformation. - SharedTransform parent() const; + const SharedTransform& parent() const; /// @brief Checks, if the chain of parents contains the given transform. bool parentChainContains(const Transform& transform) const; /// @brief UNSAFE! Forces the parent of this transform to the given transform, without checking for potential /// cycles. /// @remark A cycle will cause an immediate stack overflow, from recursively calling parent change events. - void forceParent(const SharedTransform& parent); + void forceParent(SharedTransform parent); /// @brief Tries to set the parent of this transform to the given transform and returns false if it would introduce /// a cycle. - bool trySetParent(const SharedTransform& parent); + bool trySetParent(SharedTransform parent); /// @brief Tries to set the parent of this transform to the given transform and throws a TransformCycleError if it /// would introduce a cycle. - void setParent(const SharedTransform& parent); + void setParent(SharedTransform parent); /// @brief Removes the current parent, which is the same as setting the parent to nullptr. void resetParent(); @@ -59,7 +62,7 @@ class Transform { private: dquat own_transform_; - std::optional full_transform_; + mutable std::optional full_transform_; SharedTransform parent_; Event::Subscription parent_change_; }; diff --git a/dang-gl/include/dang-gl/Rendering/Camera.h b/dang-gl/include/dang-gl/Rendering/Camera.h index 0e619523..b6cd4949 100644 --- a/dang-gl/include/dang-gl/Rendering/Camera.h +++ b/dang-gl/include/dang-gl/Rendering/Camera.h @@ -191,7 +191,7 @@ class Camera { private: SharedProjectionProvider projection_provider_; - SharedTransform transform_ = Transform::create(); + SharedTransform transform_ = std::make_shared(); mutable std::vector uniforms_; }; diff --git a/dang-gl/src/Math/Transform.cpp b/dang-gl/src/Math/Transform.cpp index 68b8a7f4..44310c58 100644 --- a/dang-gl/src/Math/Transform.cpp +++ b/dang-gl/src/Math/Transform.cpp @@ -2,7 +2,9 @@ namespace dang::gl { -UniqueTransform Transform::create() { return std::make_unique(); } +Transform::Transform(const dquat& own_transform) + : own_transform_(own_transform) +{} const dquat& Transform::ownTransform() const { return own_transform_; } @@ -13,18 +15,14 @@ void Transform::setOwnTransform(const dquat& transform) on_change(*this); } -const dquat& Transform::fullTransform() +const dquat& Transform::fullTransform() const { - if (!full_transform_) { - if (parent_) - full_transform_ = own_transform_ * parent_->fullTransform(); - else - full_transform_ = own_transform_; - } + if (!full_transform_) + full_transform_ = parent_ ? own_transform_ * parent_->fullTransform() : own_transform_; return *full_transform_; } -SharedTransform Transform::parent() const { return parent_; } +const SharedTransform& Transform::parent() const { return parent_; } bool Transform::parentChainContains(const Transform& transform) const { @@ -39,15 +37,15 @@ bool Transform::parentChainContains(const Transform& transform) const return false; } -void Transform::forceParent(const SharedTransform& parent) +void Transform::forceParent(SharedTransform parent) { - parent_ = parent; - if (parent) { + parent_ = std::move(parent); + if (parent_) { auto parent_change = [&] { full_transform_.reset(); on_change(*this); }; - parent_change_ = parent->on_change.subscribe(parent_change); + parent_change_ = parent_->on_change.subscribe(parent_change); } else { parent_change_.remove(); @@ -57,7 +55,7 @@ void Transform::forceParent(const SharedTransform& parent) on_change(*this); } -bool Transform::trySetParent(const SharedTransform& parent) +bool Transform::trySetParent(SharedTransform parent) { if (parent == parent_) return true; @@ -65,14 +63,14 @@ bool Transform::trySetParent(const SharedTransform& parent) if (parent && parent->parentChainContains(*this)) return false; - forceParent(parent); + forceParent(std::move(parent)); return true; } -void Transform::setParent(const SharedTransform& parent) +void Transform::setParent(SharedTransform parent) { - if (!trySetParent(parent)) + if (!trySetParent(std::move(parent))) throw TransformCycleError("Cannot set transform parent, as it would introduce a cycle."); }