Skip to content

Conversation

@shipp02
Copy link
Contributor

@shipp02 shipp02 commented Mar 12, 2024

Hello,

This fixes above issue, demo:

Screen.Recording.2024-03-12.at.12.00.48.AM.mov

Closes #1529

@Keavon Keavon changed the title Fix #1529 Fix doubled translation from dragging layers sharing an upstream Transform node Mar 12, 2024
@shipp02 shipp02 marked this pull request as ready for review March 12, 2024 16:13
@Keavon Keavon requested a review from 0HyperCube March 14, 2024 22:50
@Keavon
Copy link
Member

Keavon commented Mar 14, 2024

@0HyperCube you're better than me at understanding this part of the code and the implications of this change. Could you please see if this looks good to go? Thanks.

@Keavon
Copy link
Member

Keavon commented Mar 15, 2024

@shipp02 please leave a comment in the original issue so I can assign it to you.

@Keavon
Copy link
Member

Keavon commented Mar 16, 2024

I finally got a chance to do some QA testing on this. The first thing I noticed is that Dragging individual layers is completely broken. Just draw two layers and try dragging each of them on after the other. This is probably caused by your choice to store the transform in the Select tool's struct. I'd avoid attempting to persist state since this should be a stateless problem as far as I can tell.

@Keavon Keavon marked this pull request as draft March 16, 2024 09:06
@Keavon Keavon removed the request for review from 0HyperCube March 16, 2024 09:06
@0HyperCube
Copy link
Contributor

@Keavon persisting state is necessary to implement the first of my suggested solutions from the issue:

In select_tool.rs, instead of applying a delta to the current transform, store the transform at the start of the drag and compute the new transform based on that (similar to how dragging the selection bounds works)

@shipp02
Copy link
Contributor Author

shipp02 commented Mar 22, 2024 via email

@Keavon
Copy link
Member

Keavon commented Apr 4, 2024

@shipp02 just checking if you've been able to continue addressing this.

@shipp02
Copy link
Contributor Author

shipp02 commented Apr 10, 2024

I was able to reproduce the regression you mentioned but wasn't sure how to fix it.

@Keavon
Copy link
Member

Keavon commented Apr 24, 2024

I'm going to close this for now but feel free to reopen it if you're able to continue making progress.

@Keavon Keavon closed this Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dragging layers sharing a transform node applies multiple translations

3 participants