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

[modules/touchring]Fix odd behaviour while kivy.modules.touchring is active #5924

Conversation

gottadiveintopython
Copy link
Member

@gottadiveintopython gottadiveintopython commented Aug 29, 2018

Hi, I think kivy.modules.touchring has some issues.

Issue 1

While TouchRing is active, on_touch_move and on_touch_up are doubled.

from kivy.config import Config
Config.set('modules', 'touchring', '')
from kivy.app import runTouchApp
from kivy.factory import Factory
from kivy.clock import Clock
from kivy.tests.common import UnitTestTouch


def simulate_touch(dt):
    touch = UnitTestTouch(0, 0)
    touch.touch_down()
    touch.touch_move(1, 1)
    touch.touch_up()


Clock.schedule_once(simulate_touch, 0)


runTouchApp(Factory.Widget(
    on_touch_down=lambda __, touch: print('on_touch_down', touch.grab_current),
    on_touch_move=lambda __, touch: print('on_touch_move', touch.grab_current),
    on_touch_up=lambda __, touch: print('on_touch_up', touch.grab_current),
))
on_touch_down None
on_touch_move None
on_touch_move <kivy.core.window.window_sdl2.WindowSDL object at 0x7f40f578ece0>
on_touch_up None
on_touch_up <kivy.core.window.window_sdl2.WindowSDL object at 0x7f40f578ece0>

This is because _touch_move() and touch_up() in the code below do not return True when touch.grab_current is win

def _touch_move(win, touch):
ud = touch.ud
if not ud.get('tr.rect', False):
_touch_down(win, touch)
ud['tr.rect'].pos = (
touch.x - (pointer_image.width / 2. * pointer_scale),
touch.y - (pointer_image.height / 2. * pointer_scale))
def _touch_up(win, touch):
if touch.grab_current is win:
ud = touch.ud
win.canvas.after.remove(ud['tr.color'])
win.canvas.after.remove(ud['tr.rect'])
if ud.get('tr.grab') is True:
touch.ungrab(win)
ud['tr.grab'] = False

Issue 2

If there is another module that does touch.grab(window) at the same time, the program behave unexpectedly.

from kivy.config import Config
Config.set('modules', 'touchring', '')
from kivy.app import runTouchApp
from kivy.lang import Builder
from kivy.core.window import Window


def _touch_down(window, touch):
    ud = touch.ud
    if not ud.get('another_grab', False):
        ud['another_grab'] = True
        touch.grab(window)


def _touch_move(window, touch):
    if touch.grab_current is window:
        return True


def _touch_up(window, touch):
    if touch.grab_current is window:
        ud = touch.ud
        if ud.get('another_grab', False):
            touch.ungrab(window)
            ud['another_grab'] = False
        return True


def on_active_changed(switch, active):
    window = switch.get_root_window()
    if active:
        window.bind(
            on_touch_down=_touch_down,
            on_touch_move=_touch_move,
            on_touch_up=_touch_up,
        )
    else:
        window.unbind(
            on_touch_down=_touch_down,
            on_touch_move=_touch_move,
            on_touch_up=_touch_up,
        )


root = Builder.load_string('''
Widget:
    Switch:
        id: switch
''')
switch = root.ids.switch
switch.bind(active=on_active_changed)
runTouchApp(root)

Rings don't disappear while switch is active.

screenshot at 2018-08-30 07-32-16

In the first place, is touch.grab(window) safe? I don't think so because there is no guarantee that touchring is the only one who does touch.grab(window). The argument of touch.grab() should be your own widget instance.

@gottadiveintopython gottadiveintopython force-pushed the fix_odd_behaviour_while_kivy.modules.touchring_is_active branch 4 times, most recently from 5c914e1 to ffb12da Compare August 31, 2018 23:02
Copy link
Member

@tshirtman tshirtman left a comment

Choose a reason for hiding this comment

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

While the refactoring makes code cleaner overall, this has issues:

  • this only allows one touch at a time to have a touchring, this is not multitouch compatible anymore!
  • It was nice to use Rectangle instead of image widget, for performances, as in some setups, a lot of touches will be on the device at the same time (from dozens to hundreds) and it can make a significant difference.

On the other hand, it's nice to have tests, but you might want to add one that does multiple concurrent tests, and catches the previous issue.

@gottadiveintopython
Copy link
Member Author

Sorry I completely forgot about multitouch.

It was nice to use Rectangle instead of image widget, for performances ...

Then how about this:

widget = Factory.Widget(...)
Window.add_widget(widget)

with widget.canvas:
    Rectangle(...)

This fix both of issue#1 and issue#2 and the performance issue you mentioned.

Btw I realized that issue#1 is not an actual issue. It's normal behavior as documented.

You might receive an event with a grabbed touch, but not from you: it can be because the parent has sent the touch to its children while it was in the grabbed state.

But even though it's normal, I think it should be fixed because it confuse the users.

@gottadiveintopython gottadiveintopython force-pushed the fix_odd_behaviour_while_kivy.modules.touchring_is_active branch from ffb12da to 1336dd1 Compare September 1, 2018 03:41
@gottadiveintopython
Copy link
Member Author

I forgot to write one test. I'm doing that now.

@gottadiveintopython gottadiveintopython force-pushed the fix_odd_behaviour_while_kivy.modules.touchring_is_active branch from 1336dd1 to d044030 Compare September 2, 2018 15:15
@gottadiveintopython
Copy link
Member Author

@tshirtman I've fixed the issues you mentioned. Do you think it's fine or not?

@matham matham changed the title Fix odd behaviour while kivy.modules.touchring is active [modules/touchring]Fix odd behaviour while kivy.modules.touchring is active May 23, 2019
gottadiveintopython added a commit to gottadiveintopython/kivy that referenced this pull request Aug 30, 2019
fix TouchRing bugs
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.

None yet

2 participants