2
\$\begingroup\$

This code represents an Entity Component that holds and manipulates an entity's local and world transforms, dealing with hierarchy and translation/rotation/scale transformations. The code is written in C++17.

This code was not built for performance or memory efficiency. It was built more as a way to learn how to support a scene hierarchy. By all means, you can point out ways where these two aspects could be improved, but the main reason for this code review is to ensure the mathematical and logical correctness of each operation.

SceneComponent.hpp

#pragma once

#include <Components/EntityComponent.hpp>

class SceneComponent : public EntityComponent
{
public:
    SceneComponent() = default;
    virtual ~SceneComponent();

    entt::entity GetParent() const { return m_parent; }
    std::vector<entt::entity> const& GetChildren() const { return m_children; }

    bool AttachTo(entt::entity parent);
    bool Detach();

    // Local transform
    inline glm::vec3 const& GetLocalTranslation() const { return m_localTransform.GetTranslation(); }
    inline glm::quat const& GetLocalRotation() const { return m_localTransform.GetRotation(); }
    inline glm::vec3 const& GetLocalScale() const { return m_localTransform.GetScale(); }
    inline glm::vec3 GetLocalEulerRotation() const { return m_localTransform.GetEulerRotation(); }
    inline glm::mat4 const& GetLocalMatrix() { return m_localTransform.GetMatrix(); }
    void SetLocalTranslation(glm::vec3 const& translation);
    void SetLocalScale(glm::vec3 const& scale);
    void SetLocalRotation(glm::quat const& rotation);
    void SetLocalEulerRotation(glm::vec3 const& eulerRotation);

    // World transform
    glm::vec3 const& GetWorldTranslation();
    glm::quat const& GetWorldRotation();
    glm::vec3 const& GetWorldScale();
    glm::vec3 GetWorldEulerRotation();
    glm::mat4 const& GetWorldMatrix();
    void SetWorldTranslation(glm::vec3 const& translation);
    void SetWorldScale(glm::vec3 const& scale);
    void SetWorldRotation(glm::quat const& rotation);
    void SetWorldEulerRotation(glm::vec3 const& eulerRotation);

private:
    struct Transform
    {
        Transform() = default;
        Transform(glm::vec3 const& translation, glm::quat const& rotation, glm::vec3 const& scale);

        // Getters
        inline glm::vec3 const& GetTranslation() const { return m_translation; }
        inline glm::quat const& GetRotation() const { return m_rotation; }
        inline glm::vec3 const& GetScale() const { return m_scale; }
        glm::vec3 GetEulerRotation() const;
        glm::mat4 const& GetMatrix();

        // Setters
        void SetTranslation(glm::vec3 const& translation);
        void SetScale(glm::vec3 const& scale);
        void SetRotation(glm::quat const& rotation);
        void SetEulerRotation(glm::vec3 const& eulerRotation);

        // Modifiers
        void Translate(glm::vec3 const& translation);
        void Scale(glm::vec3 const& scale);
        void Rotate(glm::vec3 const& eulerRotation);
        void Rotate(glm::quat const& rotation);

    private:
        void CalculateMatrix();

    private:
        glm::vec3 m_translation{ 0.0f };
        glm::quat m_rotation{ 0.0f, 0.0f, 0.0f, 1.0f };
        glm::vec3 m_scale{ 1.0f };

        glm::mat4 m_matrix;
        bool m_isMatrixDirty = true;
    };

private:
    void InvalidateWorldTransformRecursively();
    void CalculateWorldTransform();
    bool IsDescendent(entt::entity const entity) const;

private:
    Transform m_localTransform;
    Transform m_worldTransform;
    bool m_isWorldTransformDirty = true;

    entt::entity m_parent = entt::null;
    std::vector<entt::entity> m_children;
};

SceneComponent.cpp

#include <Components/SceneComponent.hpp>
#include <Systems/EntitySystem.hpp>

SceneComponent::~SceneComponent()
{
    Detach();

    for (entt::entity child : m_children)
    {
        SceneComponent& childScene = EntitySystem::GetInstance().GetComponent<SceneComponent>(child);
        childScene.Detach();
    }
}

bool SceneComponent::AttachTo(entt::entity parent)
{
    if (!EntitySystem::IsEntityValid(parent) || parent == m_parent)
    {
        return false;
    }

    if (IsDescendent(parent))
    {
        return false; // Trying to attach to one of our descendents
    }

    if (EntitySystem::IsEntityValid(m_parent))
    {
        Detach();
    }
   
    SceneComponent& parentScene = EntitySystem::GetInstance().GetComponent<SceneComponent>(parent);
    parentScene.m_children.push_back(m_entity);
    m_parent = parent;

    SetWorldTranslation(GetLocalTranslation());
    SetWorldRotation(GetLocalRotation());
    SetWorldScale(GetLocalScale());

    return true;
}

bool SceneComponent::Detach()
{
    if (!EntitySystem::IsEntityValid(m_parent))
    {
        return false;
    }

    SetLocalTranslation(GetWorldTranslation());
    SetLocalRotation(GetWorldRotation());
    SetLocalScale(GetWorldScale());

    SceneComponent& parentScene = EntitySystem::GetInstance().GetComponent<SceneComponent>(m_parent);
    auto const childIt = std::find(parentScene.m_children.begin(), parentScene.m_children.end(), m_entity);
    if (childIt != parentScene.m_children.end())
    {
        parentScene.m_children.erase(childIt);
    }
    m_parent = entt::null;

    return true;
}

void SceneComponent::InvalidateWorldTransformRecursively()
{
    if (m_isWorldTransformDirty)
    {
        return; // Already dirty from another call
    }

    // Invalidate our world transform and our children's recursively
    m_isWorldTransformDirty = true;

    for (entt::entity child : m_children)
    {
        SceneComponent& childScene = EntitySystem::GetInstance().GetComponent<SceneComponent>(child);
        childScene.InvalidateWorldTransformRecursively();
    }
}

void SceneComponent::SetLocalTranslation(glm::vec3 const& translation) 
{
    m_localTransform.SetTranslation(translation);
    InvalidateWorldTransformRecursively();
}

void SceneComponent::SetLocalScale(glm::vec3 const& scale) 
{
    m_localTransform.SetScale(scale);
    InvalidateWorldTransformRecursively();
}

void SceneComponent::SetLocalRotation(glm::quat const& rotation) 
{
    m_localTransform.SetRotation(rotation);
    InvalidateWorldTransformRecursively();
}

void SceneComponent::SetLocalEulerRotation(glm::vec3 const& eulerRotation) 
{
    m_localTransform.SetEulerRotation(eulerRotation);
    InvalidateWorldTransformRecursively();
}

glm::mat4 const& SceneComponent::GetWorldMatrix()
{
    if (m_isWorldTransformDirty)
    {
        CalculateWorldTransform();
    }

    return m_worldTransform.GetMatrix();
}

glm::vec3 const& SceneComponent::GetWorldTranslation()
{
    if (m_isWorldTransformDirty)
    {
        CalculateWorldTransform();
    }

    return m_worldTransform.GetTranslation();
}

glm::quat const& SceneComponent::GetWorldRotation()
{
    if (m_isWorldTransformDirty)
    {
        CalculateWorldTransform();
    }

    return m_worldTransform.GetRotation();
}

glm::vec3 SceneComponent::GetWorldEulerRotation()
{
    if (m_isWorldTransformDirty)
    {
        CalculateWorldTransform();
    }

    return m_worldTransform.GetEulerRotation();
}

glm::vec3 const& SceneComponent::GetWorldScale()
{
    if (m_isWorldTransformDirty)
    {
        CalculateWorldTransform();
    }

    return m_worldTransform.GetScale();
}

void SceneComponent::SetWorldTranslation(glm::vec3 const& translation)
{
    if (EntitySystem::IsEntityValid(m_parent))
    {
        SceneComponent& parentScene = EntitySystem::GetInstance().GetComponent<SceneComponent>(m_parent);
        glm::mat4 const& parentWorldMatrix = parentScene.GetWorldMatrix();
        m_localTransform.SetTranslation(glm::inverse(parentWorldMatrix) * glm::vec4(translation, 1.0f));
    }
    else
    {
        m_localTransform.SetTranslation(translation);
    }

    InvalidateWorldTransformRecursively();
}

void SceneComponent::SetWorldScale(glm::vec3 const& scale)
{
    if (EntitySystem::IsEntityValid(m_parent))
    {
        SceneComponent& parentScene = EntitySystem::GetInstance().GetComponent<SceneComponent>(m_parent);
        m_localTransform.SetScale(scale / parentScene.GetWorldScale());
    }
    else
    {
        m_localTransform.SetScale(scale);
    }

    InvalidateWorldTransformRecursively();
}

void SceneComponent::SetWorldRotation(glm::quat const& rotation)
{
    if (EntitySystem::IsEntityValid(m_parent))
    {
        SceneComponent& parentScene = EntitySystem::GetInstance().GetComponent<SceneComponent>(m_parent);
        m_localTransform.SetRotation(glm::inverse(parentScene.GetWorldRotation()) * rotation);
    }
    else
    {
        m_localTransform.SetRotation(rotation);
    }

    InvalidateWorldTransformRecursively();
}

void SceneComponent::SetWorldEulerRotation(glm::vec3 const& eulerRotation)
{
    SetWorldRotation(glm::quat(eulerRotation));
}

void SceneComponent::CalculateWorldTransform()
{
    if (EntitySystem::IsEntityValid(m_parent))
    {
        SceneComponent& parentScene = EntitySystem::GetInstance().GetComponent<SceneComponent>(m_parent);
        m_worldTransform.SetTranslation(parentScene.GetWorldMatrix() * glm::vec4(m_localTransform.GetTranslation(), 1.0f));
        m_worldTransform.SetRotation(parentScene.GetWorldRotation() * m_localTransform.GetRotation());
        m_worldTransform.SetScale(parentScene.GetWorldScale() * m_localTransform.GetScale());
    }
    else
    {
        m_worldTransform = m_localTransform;
    }

    m_isWorldTransformDirty = false;
}

bool SceneComponent::IsDescendent(entt::entity const entity) const
{
    for (entt::entity child : m_children)
    {
        if (child == entity) 
        {
            return true;
        }
        
        SceneComponent const& childScene = EntitySystem::GetInstance().GetComponent<const SceneComponent>(child);
        if (childScene.IsDescendent(entity))
        {
            return true;
        }
    }

    return false;
}

SceneComponent::Transform::Transform(glm::vec3 const& translation, glm::quat const& rotation, glm::vec3 const& scale)
    : m_translation(translation)
    , m_rotation(rotation)
    , m_scale(scale)
{
}

glm::vec3 SceneComponent::Transform::GetEulerRotation() const
{
    // Returns pitch, yaw and roll
    return glm::eulerAngles(m_rotation);
}

glm::mat4 const& SceneComponent::Transform::GetMatrix()
{
    if (m_isMatrixDirty)
    {
        CalculateMatrix();
    }
    
    return m_matrix;
}

void SceneComponent::Transform::CalculateMatrix()
{
    glm::mat4 const translationMatrix = glm::translate(glm::mat4(1.0f), m_translation);
    glm::mat4 const rotationMatrix = glm::toMat4(m_rotation);
    glm::mat4 const scaleMatrix = glm::scale(glm::mat4(1.0f), m_scale);

    m_matrix = translationMatrix * rotationMatrix * scaleMatrix;

    m_isMatrixDirty = false;
}

void SceneComponent::Transform::SetTranslation(glm::vec3 const& translation)
{
    m_translation = translation;
    m_isMatrixDirty = true;
}

void SceneComponent::Transform::SetRotation(glm::quat const& rotation)
{
    m_rotation = rotation;
    m_isMatrixDirty = true;
}

void SceneComponent::Transform::SetScale(glm::vec3 const& scale)
{
    m_scale = scale;
    m_isMatrixDirty = true;
}

void SceneComponent::Transform::SetEulerRotation(glm::vec3 const& eulerRotation)
{
    m_rotation = glm::quat(eulerRotation);
    m_isMatrixDirty = true;
}

void SceneComponent::Transform::Translate(glm::vec3 const& translation)
{
    m_translation += translation;
    m_isMatrixDirty = true;
}

void SceneComponent::Transform::Rotate(glm::vec3 const& eulerRotation)
{
    // Applies euler angles in XYZ order
    Rotate(glm::quat(eulerRotation));
}

void SceneComponent::Transform::Rotate(glm::quat const& rotation)
{
    m_rotation = m_rotation * rotation;
    m_isMatrixDirty = true;
}

void SceneComponent::Transform::Scale(glm::vec3 const& scale)
{
    m_scale *= scale;
    m_isMatrixDirty = true;
}
\$\endgroup\$
2
  • 3
    \$\begingroup\$ It would help if you indicated which version of C++ you are using (c++11, c++14, c++17, c++20). Are you providing includes for the all the header files you are including? \$\endgroup\$
    – pacmaninbw
    Mar 23 at 16:31
  • 1
    \$\begingroup\$ @pacmaninbw Yes but I didn't provide the code for those headers because it didn't seem necessary for this code review, in my opinion. If anyone disagrees, I can add them. As for the C++ standard version, I'm using C++17. \$\endgroup\$ Mar 23 at 17:07

1 Answer 1

3
\$\begingroup\$

One observation is the use of "world" vs "local". This appears to be leading to an inappropriate hierarchy, where the children insert themselves into their parent's business. The AttachTo() method allows a child to attach to a parent, which then then stuffs its translations upward. I find it difficult to understand, and tough to assert that it's done correctly in every case.

Consider having the parent own the relationships with the children following a strict composite pattern. Allow a parent to add a child, but never allow a child to add a parent. (It's unlikely the child should ever need a pointer to its parent.) When it comes time to transform, have the top level (world) get the children (local) to do their transforms; which fan downwards to their children, and so on. This should eliminate the need for all the "World" vs "Local" logic. If a specific order is needed (parent-then-child or child-then-parent) just change the logic to apply it before or after calling the children's transform() methods.

As far as proving the mathematical and correctness of the logic, that's beyond the scope of a code review, as that's what unit tests are for. However, if the hierarchy is simplified I think you'll find it much easier to test.

EDIT: I said above that it's unlikely the child needs a parent pointer. This is true for the logic in a composite pattern to be correct. However, there are cases where optimization for performance is needed; for example if the scene is indoors in a building with no windows or open doors to the outside, the entire world wouldn't need to re-transform just to rearrange the furniture in the room, or the items on a desk.

To render a particular view, it would be adequate to stop at the local context unless there are unresolved areas, at which case it would be necessary to render the parent to fill those in. That's a case where it needs to walk backwards up the hierarchy, only rendering parents when necessary. This can permit a "distance" optimization - such as limiting rendering to 2 levels of hierarchy and fill in the rest with fog.

Limiting the scope of the transform to a parent or two is usually adequate for local rendering without racing through the entire world hierarchy of scenes.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.