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

Incorrect Node3D transform when moving and rotating with the gizmo in local space if parent node's coordinates are not in (0,0,0) #81604

Closed
raccoon-path opened this issue Sep 13, 2023 · 6 comments · Fixed by #81609

Comments

@raccoon-path
Copy link

Godot version

v4.2.dev4.official [549fcce]

System information

Windows 10 - Godot v4.2.dev4.official - Vulkan (Forward+) editor

Issue description

Version 4.2-dev4 introduced incorrect behavior when transforming nodes in local space (Node "jumps" to unwanted position). In the previous version everything works correctly.

godot_4.2dev4_bug.mp4

Steps to reproduce

  1. Create any Node3D
  2. Add any child Node3D
  3. Move parent node away from (0,0,0) or/and rotate it
  4. Enable "use local space"
  5. Try to move child node with gizmo and see undesired behavior when transforming

Minimal reproduction project

translate_localspace_editor_bug.zip

@AThousandShips
Copy link
Member

Looks related to:

Will take a look

@kleonc
Copy link
Member

kleonc commented Sep 16, 2023

Since this is a regression introduced in 4.2.dev4 the question should be what caused it.

Looking at the changes since 4.2.dev3 I suspect it's caused by #58389 (haven't tested though). cc @rcorre
Specifically the Node3DEditorViewport::apply_transform method factored out in there seems to be not replacing the previously existing code one-to-one, there are small discrepancies:

Transform3D new_xform = _compute_transform(_edit.mode, se->original * xform, xform, p_motion, p_snap, local_coords, _edit.plane != TRANSFORM_VIEW); // Force orthogonal with subgizmo.
if (!local_coords) {
new_xform = se->original.affine_inverse() * new_xform;
}
se->gizmo->set_subgizmo_transform(GE.key, new_xform);
}
} else {
Transform3D new_xform = _compute_transform(_edit.mode, se->original, se->original_local, p_motion, p_snap, local_coords, sp->get_rotation_edit_mode() != Node3D::ROTATION_EDIT_MODE_BASIS && _edit.plane != TRANSFORM_VIEW);

vs original:

  • When scaling (this seems one to one):
    Transform3D new_xform = _compute_transform(TRANSFORM_SCALE, se->original * xform, xform, motion, snap, local_coords, _edit.plane != TRANSFORM_VIEW); // Force orthogonal with subgizmo.
    if (!local_coords) {
    new_xform = se->original.affine_inverse() * new_xform;
    }
    se->gizmo->set_subgizmo_transform(GE.key, new_xform);
    }
    } else {
    Transform3D new_xform = _compute_transform(TRANSFORM_SCALE, se->original, se->original_local, motion, snap, local_coords, sp->get_rotation_edit_mode() != Node3D::ROTATION_EDIT_MODE_BASIS && _edit.plane != TRANSFORM_VIEW);
  • When translating (no if (!local_coords) check (likely the exact cause of this issue), last arg differencies for _compute_transform calls):
    Transform3D new_xform = _compute_transform(TRANSFORM_TRANSLATE, se->original * xform, xform, motion, snap, local_coords, true); // Force orthogonal with subgizmo.
    new_xform = se->original.affine_inverse() * new_xform;
    se->gizmo->set_subgizmo_transform(GE.key, new_xform);
    }
    } else {
    Transform3D new_xform = _compute_transform(TRANSFORM_TRANSLATE, se->original, se->original_local, motion, snap, local_coords, sp->get_rotation_edit_mode() != Node3D::ROTATION_EDIT_MODE_BASIS);
  • When rotating (last arg differencies for _compute_transform calls):
    Transform3D new_xform = _compute_transform(TRANSFORM_ROTATE, se->original * xform, xform, compute_axis, angle, local_coords, true); // Force orthogonal with subgizmo.
    if (!local_coords) {
    new_xform = se->original.affine_inverse() * new_xform;
    }
    se->gizmo->set_subgizmo_transform(GE.key, new_xform);
    }
    } else {
    Transform3D new_xform = _compute_transform(TRANSFORM_ROTATE, se->original, se->original_local, compute_axis, angle, local_coords, sp->get_rotation_edit_mode() != Node3D::ROTATION_EDIT_MODE_BASIS);

@AThousandShips
Copy link
Member

Will look at restoring parity, thank you!

@kleonc
Copy link
Member

kleonc commented Sep 16, 2023

Will look at restoring parity, thank you!

Note it might be the case that some of the dicrepancies in the pre-#58389 code might have been a result of patching things separately etc. By any chance I'm not saying these parts need to be reverted to the previous state, maybe unifying them like it's done now is the way to go. I just have no idea if/what could have been broken by the mentioned changes. 🙃

@AThousandShips
Copy link
Member

AThousandShips commented Sep 16, 2023

The current change fixes the bug but will see if there's other issues with that

Edit: Removing the local_coords does not fix it, so keeping the current fix as is, and see for future cleaning up of it

@AThousandShips
Copy link
Member

The last arg for the translation case doesn't affect anything as the orthogonal arg is unused there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants