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 transform calculations for drag-moving CanvasItems in editor #82667

Conversation

kleonc
Copy link
Member

@kleonc kleonc commented Oct 2, 2023

Fixes #82643.

Lately these calculations were tweaked in #81735 but we must have tested only some simple cases. In #81735 (comment) I deduced the drag_from/drag_to are in the parent coordinate space, but I've deduced it only based on the new code in that PR (which seemed to work). This was a wrong a conclusion. Now I've properly taken a look at a bigger chunk of code and drag_from/drag_to seem to be in the Viewport's global canvas coordinate space (viewport space transformed by inverse of its global_canvas_transform). Updated the calculations accordingly, now things seem to work just fine.
(To be clear things were broken for "non-standard" transforms even before #81735.)

Before After
foegTIB6X3 o1aB4FdPB8

@kleonc kleonc added this to the 4.2 milestone Oct 2, 2023
@kleonc kleonc requested a review from KoBeWi October 2, 2023 08:45
@KoBeWi
Copy link
Member

KoBeWi commented Oct 4, 2023

Found a case that's still broken:
godot windows editor dev x86_64_th8ckHVfi8
Both ancestor nodes are rotated and scaled non-uniformly (with different ratios). Affects only the gizmos.

@kleonc
Copy link
Member Author

kleonc commented Oct 4, 2023

Found a case that's still broken:

Ah, right, I've made the movement be locked to local X/Y axes, but:

  • gizmo X axis matches local X axis (so X dragging should work fine as is),
  • gizmo Y axis is not equivalent to the local Y axis, as it's forced to be perpendicular/orthogonal to the gizmo X axis.

Just skewing a single item is enough to see this discrepancy in action.

So what's desired here?:thinking: Keeping the gizmo axes perpendicular as is and just changing the Y drag to match the gizmo direction?

@KoBeWi
Copy link
Member

KoBeWi commented Oct 4, 2023

Keeping the gizmo axes perpendicular as is and just changing the Y drag to match the gizmo direction?

Yes, gizmos should always display in the same way. If they were transformed it would appear bugged.

@kleonc kleonc force-pushed the canvas-item-editor-drag-moving-transforms-fix branch from 7c1fad1 to 6f941cd Compare October 4, 2023 13:46
Comment on lines +2074 to +2076
Transform2D parent_xform = selected->get_global_transform_with_canvas() * selected->get_transform().affine_inverse();
Transform2D unscaled_transform = (transform * parent_xform * selected->_edit_get_transform()).orthonormalized();
Transform2D simple_xform = viewport->get_transform() * unscaled_transform;
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed it so this transform is calculated just like in many different places in this file. The last part (viewport->get_transform() *) don't make much sense to me, something seems off. The viewport here is a Control and according to the docs in _gui_input the events should be relative to such control, so applying its own transform on top makes no sense. Maybe the docs lie, maybe here things are incorrect. For now leaving it calculated as in other places as it seems like a potential pandora box (changing it here, would require changes in 20 different places and so on). For potential another PR.

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Works correctly now.

@akien-mga akien-mga merged commit 6bf936c into godotengine:master Oct 22, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@kleonc kleonc deleted the canvas-item-editor-drag-moving-transforms-fix branch October 22, 2023 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sprite2D Position Drag Error When Parent Sprite2D is Rotated and Child Sprite2D scale is unlinked and Changed
3 participants