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

Sprite editor select tool jumbles the selection content when moved #1071

Closed
ddelemeny opened this issue May 2, 2020 · 8 comments
Closed
Assignees
Labels
bug editor: sprites Issues related to sprite editor

Comments

@ddelemeny
Copy link

ddelemeny commented May 2, 2020

Compiled from 92ced62 on ubuntu

Selecting beyond the frame of the sprite editor causes the selected content to get corrupted when moved around with the arrow keys.

Steps to reproduce :

  • open the sprite editor and center on a drawn part of the spritesheet
  • use the select tool and drag the selection outside of the canvas (leave some space to be able to move), release the mouse button but keep the mouse out of canvas
  • move the selection around with an arrow key -> content gets corrupted

bug_select

What's happening :

Keeping the mouse out of canvas prevents the release of the drag and the call to copySelection (set a breakpoint there to see it's not reached in that case). Therefore, when the selection is moved, it considers the pending selected area, but not the corresponding data.

@ddelemeny ddelemeny changed the title Sprite editor select tool garbages the selection content when moved Sprite editor select tool jumbles the selection content when moved May 2, 2020
nesbox added a commit that referenced this issue May 3, 2020
@nesbox
Copy link
Owner

nesbox commented May 3, 2020

fixed here 38d4ed7

@nesbox nesbox closed this as completed May 3, 2020
@nesbox nesbox added this to the 0.80.0 milestone May 3, 2020
@nesbox nesbox added bug editor: sprites Issues related to sprite editor labels May 3, 2020
@ddelemeny
Copy link
Author

ddelemeny commented May 3, 2020

Hmm, your fix resets the selection when you drag outside of canvas, which can be an annoying behavior when you select close to the border and you accidentally slip out.
This is why I was checking that the left mouse button wasn't pressed as a condition for drag release (albeit not on the appropriate rect I suppose).

Also, there's one corner case isn't fixed by either solution : when the user moves the selection while dragging.

Please reopen.

@ddelemeny
Copy link
Author

ddelemeny commented May 3, 2020

@nesbox The head ddelemeny@c2ee5b9 referenced above tries to wrap it all up (based on your solution). Please have a look.

@nesbox
Copy link
Owner

nesbox commented May 3, 2020

yes, you are right, it resets the selection

@nesbox
Copy link
Owner

nesbox commented May 3, 2020

So, I reverted my previous fix and just disabled keyboard handling when you dragging a selection rectangle. It looks much simpler than other solutions 9352a37.
Thank you for your help.

@nesbox nesbox closed this as completed May 3, 2020
@ddelemeny
Copy link
Author

@nesbox I confirm the corrupting behavior is fixed in all cases I could test.
I think preventing keyboard moves when dragging is a good call (and your location to do it for it is better than the one I chose).

However, in my opinion the select tool should follow this rule :

The drag state should always be released when the mouse button is released, no matter if the pointer is inside or outside of canvas.

With your last fix, you kept the original behavior : the drag state is kept active if you release the mouse button out of canvas, and it will stay active until the user brings the pointer back in canvas.
That's way less critical than corrupting the sprite of course, but I think it's a quirk that deserves to be fixed as well.

Please (sorry) consider implementing out-of-canvas release.

@nesbox
Copy link
Owner

nesbox commented May 3, 2020

I combined all the solutions in one here 0717a19
You are hard-bitten :) Thanks

@ddelemeny
Copy link
Author

Haha, I'm a simple person : I see a nail sticking out of a board, I hit the nail with a hammer until it's not sticking out of the board.

This specific nail is now perfectly leveled (as far as I can tell). Thank you!

@nesbox nesbox removed this from the 0.80.0 milestone Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug editor: sprites Issues related to sprite editor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants