Skip to content

Conversation

@dumganhar
Copy link

@dumganhar dumganhar commented Mar 16, 2017

PR #17476
Uses range loop for clearing claimed touches in the next touch begin event.

Image We have two sprite squares, A (Left) and square B(Right), A is above of B, Both A and B have one by one touch listeners, and it will not swallow touches.

Click at the intersection of A and B, do not move
Touch press
A - onTouchBegan, return true, listener->_claimedTouches is true for A
B - onTouchBegan, return true, listener->_claimedTouches is true for B
Touch release
A - onTouchEnded, we manually call event->stopPropagation() in the onTouchEnd function !!! listener->_claimedTouches is cleared
B - Because A stopPropagation, listener->_claimedTouches is not cleared

In this case, Because A stop propagation for touch end, the listener->_claimedTouches for B will not be cleared forever ! because B has touch begin, which record the touch in listener->_claimedTouches, but it has not touch end !

dadi and others added 4 commits March 16, 2017 14:12
Once a new touch is begin, we should clear the _claimedTouches for the one by one listeners.
Since the touch begin and touch end is not always in pairs !
Image a  call event->stopPropagation() in function onTouchEnded(), it will make the other listeners have no end events !  even the other listeners has begin event and the onTouchBegin fuction  return true.
@dumganhar
Copy link
Author

DON'T MERGE IT NOW.
I have to check the logic again.

for (auto l : (*fixedPriorityListeners))
{
EventListenerTouchOneByOne* listener = static_cast<EventListenerTouchOneByOne*>(l);
listener->_claimedTouches.clear();
Copy link
Author

Choose a reason for hiding this comment

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

@dadidzf , if clear _claimedTouches, it may break the multi-touch logic of using EventListenerTouchOneByOne. I will check TouchTest to see whether it works after this fix.

Copy link
Author

Choose a reason for hiding this comment

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

I confirmed that this patch breaks cpp-tests/63:Touches

Copy link

Choose a reason for hiding this comment

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

@dumganhar Thanks, Do you know why it breaks? I test the cpp-tests/63, it is ok ! it seems multi-touch do not use _claimedTouches

Copy link
Author

Choose a reason for hiding this comment

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

@dadidzf , just test it on iOS or Android mobile phones. Using multi-touch on the white blocks. The second time you touch the blocks, it will not be responsive.

@minggo
Copy link
Contributor

minggo commented Mar 23, 2017

@dadidzf do you have any question about this PR? If not, i will merge it. Thanks.

@dumganhar
Copy link
Author

@minggo , please don't merge this PR.
There are some issues.

@dadidzf
Copy link

dadidzf commented Mar 26, 2017

@minggo This patch do cause some issues as @dumganhar described , it can not be merged! we need to find a different way to fix it ! ^_^

@minggo
Copy link
Contributor

minggo commented Mar 27, 2017

Got it, thanks.

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.

3 participants