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
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
Next Next commit
fix inspector rotation use XYZ rotate
VTui22 committed Feb 28, 2024
commit 2877a9a45a6138cb7f47ef9e24c2926740989885
2 changes: 1 addition & 1 deletion Engine/Source/Editor/UILayers/Inspector.cpp
Original file line number Diff line number Diff line change
@@ -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();
3 changes: 3 additions & 0 deletions Engine/Source/Runtime/ECWorld/TransformComponent.h
Original file line number Diff line number Diff line change
@@ -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:
@@ -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
};

42 changes: 35 additions & 7 deletions Engine/Source/Runtime/ImGui/ImGuiUtils.hpp
Original file line number Diff line number Diff line change
@@ -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);
@@ -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);

@@ -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;