CedricGuillemet / ImGuizmo

Immediate mode 3D gizmo for scene editing and other controls based on Dear Imgui
MIT License
3.17k stars 895 forks source link

Clipping issue when rotation the grid #205

Closed Kryptos-FR closed 3 years ago

Kryptos-FR commented 3 years ago

Hello,

I recently started to integrate to integrate ImGuizmo into our chainblocks engine. It works very well so far, so thank you for sharing that project with the community.

I have found an issue when attempting to rotate the grid. I assume that is the purpose of the const float* matrix argument in the DrawGrid function: https://github.com/CedricGuillemet/ImGuizmo/blob/7c16cac47891ee0cb14641b1699cb5f38ce4a7d3/ImGuizmo.cpp#L2608

Testing it out with a simple rotation matrix (around X or Z axis), brings some clipping issue:

static const float gridX[16] = {1.f, 0.f,  0.f, 0.f, 0.f, 0.f, -1.f, 0.f, 0.f, 1.f, 0.f, 0.f, 0.f, 0.f, 0.f, 1.f};

gizmo_grid_axis_issue2

Looking at DrawGrid implementation, I can see that the frustum planes are calculated using the projection and view matrices only. My limited knowledge in 3D math tell me that it could be the issue and indeed if I replace it with a frustum calculated on the whole matrix multiplication (including this matrix argument) it seems to work:

diff --git a/ImGuizmo.cpp b/ImGuizmo.cpp
index ab71afe..3961e88 100644
--- a/ImGuizmo.cpp
+++ b/ImGuizmo.cpp
@@ -2608,9 +2608,9 @@ namespace IMGUIZMO_NAMESPACE
    void DrawGrid(const float* view, const float* projection, const float* matrix, const float gridSize)
    {
       matrix_t viewProjection = *(matrix_t*)view * *(matrix_t*)projection;
-      vec_t frustum[6];
-      ComputeFrustumPlanes(frustum, viewProjection.m16);
       matrix_t res = *(matrix_t*)matrix * viewProjection;
+      vec_t frustum[6];
+      ComputeFrustumPlanes(frustum, res.m16);

gizmo_grid_axis_issue_fixed2

Before opening a pull-request with that change, I would like to discuss whether my assumptions are correct or if it could break in other cases when the matrix argument is not a simple rotation around an axis.

CedricGuillemet commented 3 years ago

Thanks for the kind words. Yes, you can pre multiply the view matrix with an additional rotation like:

void DrawGrid(const float* view, const float* projection, const float* matrix, const float gridSize)
   {
      matrix_t rtz;
      rtz.RotationAxis({0.f,0.f,1.f}, ZPI * 0.5f);
      matrix_t viewProjection = rtz * *(matrix_t*)view * *(matrix_t*)projection;

This should not be linked with model matrix. It's up to the user to change the view matrix for the grid. Moreover, the grid and debug cube are there more as a sample. I don't think they should be used in production editor.

Kryptos-FR commented 3 years ago

I see, so there is no issue in the current code, and it's up to the caller to modify the view matrix when necessary.

In my case it worked because I wasn't passing anything as matrix before so it was having the same effect. Got it. Thanks for the quick reply.

As for the cube, the one in my screen capture is not from the sample but a model 😀. I guess I should use a more interesting one next time.

Kryptos-FR commented 3 years ago

Update: I could make it work by rotating the view matrix before passing it to the DrawGrid function.

Thanks again for the support.