Skip to content

only 1 pressup triggered after 2 consecutive mousedown #551

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

Open
danielocdh opened this issue Jan 8, 2015 · 13 comments
Open

only 1 pressup triggered after 2 consecutive mousedown #551

danielocdh opened this issue Jan 8, 2015 · 13 comments
Labels

Comments

@danielocdh
Copy link

I found this thinking it was an error in my code, it happens with 0.7.1 and 0.8.0

I don't know if this is the correct behaviour, but if you press 2 mouse buttons without releasing them, you get 2 "mousedown" events, after releasing the buttons you only get 1 "pressup" event.

If you believe this is the correct behaviour, is there any easy way to generate both pressup events? Thank you.

0.7.1 and 0.8.0 examples:
http://jsfiddle.net/ncknqtaw/
http://jsfiddle.net/ncknqtaw/1/

@noobiept
Copy link
Contributor

I looked at the source, couldn't really find a fix, but the problem seems to be in the Stage's _handlePointerUp() function.

That function is called for both the pressup events, but in the second time the 'oTarget' variable doesn't get the proper target. Couldn't really figured out the reason why that happens.

If you do something like this here: https://github.com/CreateJS/EaselJS/blob/master/src/easeljs/display/Stage.js#L723

oTarget = target = this._getObjectsUnderPoint(o.x, o.y, null, true);

then you actually get the 2 pressup events. Of course this is not the fix, but maybe it can help someone more into the codebase.

@MannyC
Copy link
Contributor

MannyC commented Jan 13, 2015

Looks like it doesn't differentiate between left click and right click when it comes to the down flag in the pointer data, so that will get cleared on the first mouse up so stagemouseup won't get sent for the second up. ( https://github.com/CreateJS/EaselJS/blob/0.8.0/src/easeljs/display/Stage.js#L720 )

Then, later, it's setting the pointerData's target to null when clear is false (which appears to be always when using a mouse), which is why there's no target when the second mouse up is registered ( https://github.com/CreateJS/EaselJS/blob/0.8.0/src/easeljs/display/Stage.js#L731 ).

@noobiept
Copy link
Contributor

You are right, the problem is that it isn't differentiating between the different mouse buttons, both get the same id, so it ends up reusing the same object (from _pointerData).
I've made a pull request with a change.
Thanks for the help :)

@gskinner
Copy link
Member

Good catch. I'll check out the pull request, and see what the best approach to fix this is. Note that right now some mouse interactions use the "-1" mouse id as an indicator that the interaction is not a touch. Perhaps we could switch those to check for <0 instead and simply multiply the which value by -1. This should also prevent collisions with potential touch IDs on OSes like Windows that support both touch and mouse input.

@gskinner gskinner added the bug label Jan 14, 2015
@noobiept
Copy link
Contributor

Yes, I can make that change, just see what is the best approach.

@gskinner
Copy link
Member

Cool. If you want to take a stab at it, I'd be happy to review and sanity check / test your updated Pull Request. Thanks!

@noobiept
Copy link
Contributor

I've updated the pull request. I don't really know where the touch part is in the code though.

@MannyC
Copy link
Contributor

MannyC commented Jan 15, 2015

Have you considered using a bit field (or even boolean array) to mark which buttons are currently pressed (and setting down/releasing the target only when none are marked as pressed) and storing it in the one pointerData? It doesn't seem like having each button have its own pointerData fits with the word "pointer". I'm not sure we can count on always getting mouseups (so we can clear the target), but if that's a problem then the original had it too. I saw mention of a buttons property, which would have been handy, but it didn't seem to exist for me on Chrome.

@noobiept
Copy link
Contributor

The pointerData has other information than just if its being pressed though, don't know how you would fit that in a bit field.

@MannyC
Copy link
Contributor

MannyC commented Jan 16, 2015

Actually, I meant as a property of the pointerData object.

@noobiept
Copy link
Contributor

So would we check the state of the buttons on the (next) mouse down event (to see if there was a missing mouseup event)? And release there the target. Not sure I follow your idea completely.

Right now the target is removed when 'clear' is not true, and it seems that its only true for touch.

@bobbyhiom
Copy link

Hey guys is there any news on a fix for this issue? It's affecting a game I am writing. Which is here http://www.galaxyexplorer.net/ (When you click and release the left and right mouse button only one button is released). Thanks! Bobby

@bobbyhiom
Copy link

I implemented noobiepts change and it solved my issues 👍

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

No branches or pull requests

5 participants