Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix inspector rotation use XYZ rotate #482

Merged
merged 2 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Engine/Source/Editor/UILayers/Inspector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void UpdateComponentWidget<engine::TransformComponent>(engine::SceneWorld* pScen

if (isHeaderOpen)
{
if (ImGuiUtils::ImGuiTransformProperty("Transform", pTransformComponent->GetTransform()))
if (ImGuiUtils::ImGuiTransformProperty("Transform", pTransformComponent->GetTransform(), pTransformComponent->GetInspectorEular()))
{
pTransformComponent->Dirty();
pTransformComponent->Build();
Expand Down
3 changes: 3 additions & 0 deletions Engine/Source/Runtime/ECWorld/TransformComponent.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ class TransformComponent final
#ifdef EDITOR_MODE
static bool DoUseUniformScale() { return m_doUseUniformScale; }
static void SetUseUniformScale(bool use) { m_doUseUniformScale = use; }
cd::Vec3f& GetInspectorEular() { return m_inspectorEular; }
void SetInspectorRotation(const cd::Vec3f& eular) { m_inspectorEular = eular; }
Copy link
Contributor

@T-rvw T-rvw Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cd::Vec3f& GetInspectorEular() { return m_inspectorEular; }
void SetInspectorRotation(const cd::Vec3f& eular) { m_inspectorEular = eular; }
cd::Vec3f& GetInspectorEular() { return m_inspectorEular; }
const cd::Vec3f& GetInspectorEular() const { return m_inspectorEular; }
void SetInspectorRotation(cd::Vec3f eular) { m_inspectorEular = cd::MoveTemp(eular); }

For data size larger than 8 bytes, it recommends to use move.

#endif

private:
Expand All @@ -51,6 +53,7 @@ class TransformComponent final

#ifdef EDITOR_MODE
static bool m_doUseUniformScale;
cd::Vec3f m_inspectorEular = cd::Vec3f::Zero();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cd::Vec3f m_inspectorEular = cd::Vec3f::Zero();
cd::Vec3f m_rotateEular = cd::Vec3f::Zero();

As a transform component, it is better not to know what is Inspector because it is an editor concept.
What you need to know about transform component is that we need a temporary eular data storage.

#endif
};

Expand Down
42 changes: 35 additions & 7 deletions Engine/Source/Runtime/ImGui/ImGuiUtils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,36 @@
namespace ImGuiUtils
{

static cd::Matrix3x3 RotateX(float rad)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can leverage template + if constexpr to write one function to implement same feature.

{
cd::Matrix3x3 matrixRot = cd::Matrix3x3::Identity();
matrixRot.Data(4) = std::cos(rad);
matrixRot.Data(5) = -std::sin(rad);
matrixRot.Data(7) = std::sin(rad);
matrixRot.Data(8) = std::cos(rad);
return matrixRot;
}

static cd::Matrix3x3 RotateY(float rad)
{
cd::Matrix3x3 matrixRot = cd::Matrix3x3::Identity();
matrixRot.Data(0) = std::cos(rad);
matrixRot.Data(2) = std::sin(rad);
matrixRot.Data(6) = -std::sin(rad);
matrixRot.Data(8) = std::cos(rad);
return matrixRot;
}

static cd::Matrix3x3 RotateZ(float rad)
{
cd::Matrix3x3 matrixRot = cd::Matrix3x3::Identity();
matrixRot.Data(0) = std::cos(rad);
matrixRot.Data(1) = -std::sin(rad);
matrixRot.Data(3) = std::sin(rad);
matrixRot.Data(4) = std::cos(rad);
return matrixRot;
}

static bool ImGuiBoolProperty(const char* pName, bool& value)
{
return ImGui::Checkbox(pName, &value);
Expand Down Expand Up @@ -185,7 +215,7 @@ static bool ImGuiVectorProperty(const char* pName, T& value, cd::Unit unit = cd:
return dirty;
}

static bool ImGuiTransformProperty(const char* pName, cd::Transform& value)
static bool ImGuiTransformProperty(const char* pName, cd::Transform& value, cd::Vec3f& inspectorEular)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is just OK but doesn't look good in the call side. As a api user, it is strange to apply extra vec3f.

{
ImGui::PushID(pName);

Expand All @@ -195,13 +225,11 @@ static bool ImGuiTransformProperty(const char* pName, cd::Transform& value)
dirty = true;
}

cd::Vec3f eularAngles = value.GetRotation().ToEulerAngles();
if (ImGuiVectorProperty("Rotation", eularAngles, cd::Unit::Degree, cd::Vec3f::Zero(), cd::Vec3f(360.0f)))
if (ImGuiVectorProperty("Rotation", inspectorEular, cd::Unit::Degree, cd::Vec3f(-360.0f), cd::Vec3f(360.0f), false, 0.2f))
{
float pitch = std::min(eularAngles.x(), 89.9f);
pitch = std::max(pitch, -89.9f);

value.SetRotation(cd::Quaternion::FromPitchYawRoll(pitch, eularAngles.y(), eularAngles.z()));
// XYZ Rotate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// XYZ Rotate

The comment is useless as you can get XYZ rotate information from codes in next line.

cd::Matrix3x3 matrix = RotateX(cd::Math::DegreeToRadian(inspectorEular.x())) * RotateY(cd::Math::DegreeToRadian(inspectorEular.y())) * RotateZ(cd::Math::DegreeToRadian(inspectorEular.z()));
value.SetRotation(cd::Quaternion::FromMatrix(matrix));
dirty = true;
}
constexpr float labelIndetation = 10.0f;
Expand Down
Loading