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

Offset Node3D position when drag and dropping #84438

Closed
wants to merge 1 commit into from

Conversation

warent
Copy link

@warent warent commented Nov 4, 2023

Fix #84424

@warent warent requested a review from a team as a code owner November 4, 2023 05:23
@Calinou Calinou added bug topic:editor topic:3d cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Nov 4, 2023
@Calinou Calinou added this to the 4.2 milestone Nov 4, 2023
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected.

Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

Should the rotation be applied too? I think its currently identity. I’d assume axis aligned drops?

@akien-mga
Copy link
Member

For the record, your commit seems not to be linked to your GitHub account. See: Why are my commits linked to the wrong user? for more info.

@akien-mga
Copy link
Member

akien-mga commented Nov 8, 2023

Tested locally, this seems to work when dropping 3D scenes on a scene whose root transform is the identity.

But once you start transforming the root scene, there are more (new?) issues.

For example:

Node3D scene with (2, 0, 0) position:
image

I drag and drop a mesh scene with (-10, 0, 0) position:

  • If I drop that scene on the position of the parent Node3D (2, 0, 0), then the child instance ends up at (-10, 0, 0) (so -12 offset to the parent node position)
  • If I drop that scene on the (0, 0, 0) origin of the scene, the child instance ends up at (-12, 0, 0).

If I then add rotation and scaling of the parent node to the mix, things get even weirder.

@akien-mga akien-mga modified the milestones: 4.2, 4.3 Nov 8, 2023
@akien-mga akien-mga added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Nov 8, 2023
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

See above review, this seems to introduce other issues.

@warent
Copy link
Author

warent commented Nov 11, 2023

Hmm good catch, apologies I should have tested this more thoroughly. I may have time to follow up on this in a week or so

@YuriSizov
Copy link
Contributor

@warent Will you be able to work further on this? 🙃

@ryevdokimov
Copy link
Contributor

ryevdokimov commented Jan 10, 2024

Tested locally, this seems to work when dropping 3D scenes on a scene whose root transform is the identity.

But once you start transforming the root scene, there are more (new?) issues.

For example:

Node3D scene with (2, 0, 0) position: image

I drag and drop a mesh scene with (-10, 0, 0) position:

  • If I drop that scene on the position of the parent Node3D (2, 0, 0), then the child instance ends up at (-10, 0, 0) (so -12 offset to the parent node position)
  • If I drop that scene on the (0, 0, 0) origin of the scene, the child instance ends up at (-12, 0, 0).

If I then add rotation and scaling of the parent node to the mix, things get even weirder.

Maybe I'm misunderstanding something, but not including the issue with rotation and scaling. Isn't this the expected behavior?

Those vectors represent the position of the instantiated scene relative to its parent node. If it's at (-10,0,0) world space, it should show as (-12,0,0) local, right? It's -12 units away from its parent (2,0,0).

Also, If I have a fix for the rest of this, do I open a new PR, or a review? I want to make sure @warent is credited for partially fixing this.

@akien-mga
Copy link
Member

akien-mga commented Feb 13, 2024

Maybe I'm misunderstanding something, but not including the issue with rotation and scaling. Isn't this the expected behavior?

Those vectors represent the position of the instantiated scene relative to its parent node. If it's at (-10,0,0) world space, it should show as (-12,0,0) local, right? It's -12 units away from its parent (2,0,0).

My expectation is that the position of the node I'm adding as a child is a local position relative to its new parent, not a global position.

Imagine my child scene is a sidecar positioned 1m to the right of its origin. I parent it to the origin (center of gravity) of my motorbike scene. I expect it to be 1m to the right of the bike, like a sidecar would be, regardless of the world position of the bike scene.

Edit: On the other hand when testing back then it's not impossible that I might have confused local and global positions in the inspector... :D

@akien-mga
Copy link
Member

Superseded by #87126. Thanks for the contribution!

@akien-mga akien-mga closed this Feb 14, 2024
@akien-mga akien-mga added archived and removed cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Feb 14, 2024
@AThousandShips AThousandShips removed this from the 4.3 milestone Feb 14, 2024
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.

3D objects dropped into the viewport do not respect original positioning and reset it to cursor position.
7 participants