Fixing Kivy event bubbling

[Samael-TLB]

The docs proclaim that Kivy events follow the below rules :

In Kivy, events bubble up from the first child upwards through the other children. If a widget has children, the event is passed through its children before being passed on to the widget after it.
In order to stop this event bubbling, a method can return True. This tells Kivy the event has been handled and the event propagation stops. For example:

class MyWidget(Widget):
    def on_touch_down(self, touch):
        If <some_condition>:
            # Do stuff here and kill the event
            return True
        else:
            return super(MyWidget, self).on_touch_down(touch)

This approach gives you good control over exactly how events are dispatched and managed. Sometimes, however, you may wish to let the event be completely propagated before taking action. You can use the Clock to help you here:

class MyWidget(Label):
    def on_touch_down(self, touch, after=False):
        if after:
            print "Fired after the event has been dispatched!"
        else:
            Clock.schedule_once(lambda dt: self.on_touch_down(touch, True))
            return super(MyWidget, self).on_touch_down(touch)

But on exploring various widgets it seems the rules aren't followed strictly. Event capturing shall always happen from top down to the leaves of widget tree receiving the events first and if they stop the event propagation only then the event is not bubbled up.

Though in essence, Kivy tries to follow this methodology of event propagation but there's inherent inconsistency and deviation in native widgets from this behaviour. As a well tested and industry accepted event propagation behaviour, its not bad to follow this like what browser events propagation mechanism.

Well, to highlight my point, in essence, the below points are discussed:

1. Events should originate (captured) from the topmost root that is the Window:
   • Kivy follows this perfectly
2. Events then without being consumed should be passed to the target widget(Capturing phase):
   • This is achieved with sending the event to the children widgets which are not disabled and whose bounding boxes collide with the event position and this is followed through each level till we reach the leaves, that is target widget or no other widget is left. In short, this is a DFS algorithm.
   • To be warned that during this capturing phase no widget should consume the event by default ( that is from the framework side) and only if the user wants this behaviour they can capture the event in this phase or else normally its captured down to the children first.
   • To be also noted that any disabled widget should not consume the event, it should only not pass the events to its children. This is necessary for overlapping widgets where the one on top may be needed to be disabled( also opacity may be 0) and the event need to be passed to the underlying widget that is not disabled and fully visible.
   • Without having any other method for reordering the widgets dynamically like z-index without affecting visual reordering in Kivy, its a bad assumption to say if a widget is disabled and the touch collides with the widget then the widget consumes the event and doesn't pass the event to children.
   • Either we need z-indexing behaviour or else we need user-events behaviour to cover overlapping widgets edge cases.
   • But in a simple way we can have just the logic rectified in the on_touch_down function of Widget class. Current implementation consumes the event if a widget is disabled and the event collides with the widget which ignores the edge case described above. Rather the event should just be ignored and not consumed by a disabled widget. This is done correctly in on_touch_move, on_motion and on_touch_up, but on_touch_down needs rectification.
3. Events once reached the target widget shall be allowed to be handled by it and if not handled the event should then bubble up:

• Kivy tries to do this but there's some inconsistencies in some of the widgets. Some widgets handle the event in the capturing phase and don't pass the event to the children first. This creates some weird behaviours.
• This mainly occurs in ScrollView and Carousel, which lets in weird behaviours such as not being able to use properly the Carousel and ScrollView widgets as child of one another.
• One issue observed is that having ScrollView as the child of Carousel, the swipe behaviours are buggy and lets say we have a TextInput inside the ScrollView, then the TextInput only receives focus when double tapped quickly or sometimes even more. This is not the expected UX behaviour. While on making Carousel ,with TextInput as a child, to be the child of the ScrollView, the tap behaviour is rectified but the scroll behaviour hinders with Carousel and we have to hack the swipe_distance of Carousel to have a botched up acceptable swiping behaviour.
• Then when we have one ScrollView inside another, the top most ScrollView doesn't receive the scrolling events in cases when the inner ScrollView doesn't have any more scrollable area in the direction. For example, lets say we have a ScrollView for vertical direction only and another ScrollView inside the first one for horizontal direction only. So the expected behaviour is that on vertical swipes the topmost ScrollView should scroll but what happens is that the innermost ScrollView consumes the event unnecessarily which leads to non-passing of the event to the topmost ScrollView. We should rectify this and make it behave like how scrolling behaves in web scenarios.

All the above pain points mentioned can be easily solved by just following the event propagation model correctly and exactly. That is unless specified by the user the event is not consumed in the capturing (top to bottom) phase and normally the bubbling phase starts from the bottom to top and the event is consumed only if explicitly specified. Disabled widgets should not consume touch down events.

This needs some discussion before going for an extensive pr and hence I opened this discussion. Hoping for some good insights from the members.

[Julian-O]

Below are a list of some issues that I believe are related, going back to 2015. I am not saying they will be fixed or must be fixed by your plans, but maybe worth seeing if they shed any light, or suggest related changes that can be fixed at the same time.

kivy/kivy#8075
kivy/kivy#6889
kivy/kivy#6460
kivy/kivy#6405
kivy/kivy#5617
kivy/kivy#4461
kivy/kivy#4052
kivy/kivy#2986

[Samael-TLB]

Thanks @Julian-O for the extra contents. For the past 2days, i have investigated them and concluded that they all stem from the same issues as highlighted by my first post in this thread, i.e., inconsistencies in event propagation. I'm now trying to see if the ScrollView can be fixed for this and thereafter shall continue on Carousel.
Also as a side question can we say, that on_touch_move if the widget is disabled just ignore the touch or its better to say if the widget is disabled and the touch collides with the widget then you ignore the touch . Extension to this shall be to also fine grain whether we consume the event if the widget is disabled and the touch collides with the widget and the widget has non zero opacity by returning True and dont consume otherwise if opacity is zero , or shall we totally ignore the touch.

What I'm trying to look for are some edge cases that might occur due to consuming or non consuming of the touch on a disabled widget based on its opacity.

[Julian-O]

I've been thinking about the disabled widget case, and it is definitely outside my paygrade.

My first intuition was that the touch not be passed to a disabled widget, and should just go to its parent (or other overlapping widgets).

I tried to think of a scenario where a disabled widget would need to receive touches (all I could think of was that it could pop up a warning to say "Don't press this button again"). If needed, that could be implemented by putting the widget inside a non-disabled layout of the same size that could monitor for touch events, and inform the button.

The only counter-argument I had was "Disabled has a meaning related to the display of an object. Adding additional side-effects may astonish the developer." But, I think not sending the events is the least astonishing.

This is intuition, and I will bow down to others with more experience.

[Samael-TLB]

Well, going with established patterns like the web event propagation, events are not propagated to a disabled widget and its children. And that should be the norm, a widget be it opaque or fully transparent always consumes the event if its on top of some other widgets and only if explicitly stated that pointer-events are none or not received then only it doesn't consume the event and passes it to the below widgets, other way is to have the z-index modified.

I think if we were to follow this established pattern then we need to provide with pointer-events and z-index functionality in kivy. Pointer events functionality is easy to make if we either have it as a kivy property and check it in touch events or else we can rely on the motion filters established to not receive any events. A kivy property seems to be easier to implement but advises are welcomed.

For z-index, we need the ability to insert the widget as per the z-index. This is also possible as we have the add widget and remove widget capabilities and we just need a helper function in the base widget class . But this entails additional z-index mapping to be stored and may not be a feasible solution. But we can have it shelved for further discussion.

Mainly it shall be good to have the pointer-events behvaiour implemented so that the users can decide on their own if the widget consumes the event or not. But this can have some additional footguns wherein some invisible widgets consume the touch and then some debugging is needed on the developers side to find out the widget responsible for this and to make its pointer-events to none.

While relying on opacity seems like a nice idea to me to not have the footguns and also as it doesn't introduce any new properties.

Be it any of the above methods, the end developer can always change the behaviour as per his/her wish as we follow OOP patterns so they can suit it to their needs.

[Samael-TLB]

Opened the first pr in this direction #8488 starting from the Widget class.

From the initial run, as expected the primary offenders are ScrollView and Carousel.

Pulled another pr #8498 after receiving advice from @akshayaurora to make the prs atomic.

[Samael-TLB]

I have success in making the ScrollView work as expected with multiple nested ScrollViews.

While, trying to make it that I had stumbled upon a design choice which is better done after discussion with the community I believe.
So currently, in the event propagation methods we have three main behaviours common to all widgets. One is to check if the touch is within bounds, next the disabled checks, and then the third is passing the event to the children before trying to handle it. Now, shall we just have all three in code within the touch handling functions or shall we create separate three functions for these which handle these three stages and these three functions are called in the touch handling functions. To highlight this, I have provided the below example:

def on_touch_down(self,touch):
        # within bounds check
        if not self.collide_point(*touch.pos): 
            return
        # disabled check
        if self.disabled:
            if self.opacity == 0:
                return
            else:
                return True
        # pass to children first
        for child in self.children[:]: 
            if child.dispatch('on_touch_down', touch):
                return True

# please ignore the naming as they are just for example 
def check_if_touch_is_within_bounds(self, touch):
    if not self.collide_point(*touch.pos): 
            return False
def disabled_check(self, touch):
       if self.disabled:
            if self.opacity == 0:
                return
            else:
                return True
def pass_to_children_first(self, touch):
    for child in self.children[:]: 
            if child.dispatch('on_touch_down', touch):
                return True
def on_touch_down(self,touch):
        # within bounds check
        if not check_if_touch_is_within_bounds( touch): return
        # disabled check
        if disabled_check(touch): return True
        # pass to children first
        if pass_to_children_first(touch): return True

Well, the question is which one is better 1 or 2.

For 1, we have the pros that this doesn't introduce any new methods and doesn't have function calling overhead , with the cons that if any changes are to be made by the end user in this behaviour then they have to have the raw functions implemented in their event handling functions and then make their custom changes, and can't rely on just a super call. This might unleash inconsistencies, but for internal team with some effort we can enforce the consistency, and for end users we have to rely on their sound judgement. This might lead them to have the need to read and learn the internal implementation, that might be little uncomfortable in terms of DX.

The method 2, has the pros that the internal implementation is always the same and the user can override a particular piece and dont need to implement them in code in the touch handling functions, leading to a simpler super call , which ensures better consistency while exposing the internal behaviours for easy overriding. This might lead to little bit easier DX. The cons are that there's a slight function calling overhead and then double return handling overhead, this means that we handle return not only in the sub-functions but also need to handle the returns of these sub-functions in the touch handling function as highlighted by the above second code example.

So, I would look forward for comments on which design pattern may be well suited.

[Julian-O]

I have a question.

I assume:

• this code is common across all/most widgets or layouts.
• this implementation is in one place (I don't know where: widget or layout or some behaviour or somewhere), and is inherited throughout.

Is that right? We aren't seeing it duplicated are we? If, via inheritance, some widgets are changing this behaviour, what are they changing?

If we are seeing this code in many places, consistency is more important than the minor other factors being discussed. But I am hoping we aren't seeing this duplicated, so consistency isn't a factor.

Note: There's no right answer - it is a balance of different demands, and at some point it is time to just proceed and not get caught up on analysis paralysis.

[Samael-TLB]

Well, in the current kivy codebase, a lot many widgets override these pieces of code. Just a few examples are button behavior, scrollview, carousel, etc.

For example, in button behavior we have:

def on_touch_down(self, touch):
        if super(ButtonBehavior, self).on_touch_down(touch):
            return True
        if touch.is_mouse_scrolling:
            return False
        if not self.collide_point(touch.x, touch.y):
            return False
        if self in touch.ud:
            return False
        touch.grab(self)
        touch.ud[self] = True
        self.last_touch = touch
        self.__touch_time = time()
        self._do_press()
        self.dispatch('on_press')
        return True

This shows that the internal code for checking if the point is within the widget is being rechecked or extended in the code.

In scrollview, we have:

 def on_scroll_start(self, touch, check_children=True):
        if check_children:
            touch.push()
            touch.apply_transform_2d(self.to_local)
            if self.dispatch_children('on_scroll_start', touch):
                touch.pop()
                return True
            touch.pop()

        if not self.collide_point(*touch.pos):
            touch.ud[self._get_uid('svavoid')] = True
            return
        if self.disabled:
            return True
        if self._touch or (not (self.do_scroll_x or self.do_scroll_y)):
            return self.simulate_touch_down(touch)

This is also showcasing the need to override this pieces as per the context need.

this code is common across all/most widgets or layouts.

Yes, ideally this is the case.

this implementation is in one place (I don't know where: widget or layout or some behaviour or somewhere), and is inherited throughout

Its in widget class from which all others inherit this.

Is that right? We aren't seeing it duplicated are we? If, via inheritance, some widgets are changing this behaviour, what are they changing?

Currently, the widgets are changing it without relying on inheritance, leading to code duplication in most of the widgets which perform some custom touch handling.

If we are seeing this code in many places, consistency is more important than the minor other factors being discussed. But I am hoping we aren't seeing this duplicated, so consistency isn't a factor.

Upon investigating the codebase, its clear to me that its being duplicated and utilized in a lot of places without relying on inheritance.

Now to stand my concern, this may lead to the burdening of the developer to know the sequence of the operations where they are needed, the order they are needed and why they are needed. This may lead to inconsistencies which is also evident from the current codebase. We have a lot of functionalities achieving the same goal but throughout the codebase there's no uniform usage leading to ambiguities.

Now, let's imagine we have these three functions(please ignore naming for now) in the widget:

def event_in_widget(self, touch, mode):
    .....
def disabled_widget_check(self, touch, mode):
    .....
def child_handled_event(self, touch, mode):
    .....

Mode in above functions notify whether they are being called in touch down, move or up (needed if there's differences in handling in different phases).
And in widget we have:

def on_touch_down(self, touch):
     if not self.event_in_widget( touch, "touch_down"):
        return False
     if self.disabled_widget_check(touch,"touch_down"):
        return True
     if self.child_handled_event(touch, "touch_down"):
        return True
def on_touch_move(self, touch):
 ..... # similar to touch down
 ""
 

def on_touch_up(self, touch):
 ..... # similar to touch down

And in any widgets wishing to override the behaviour, lets say the event in widget check or transforming the touch to local coordinates:

def event_in_widget(self, touch, mode):
   if self.collide_point(*touch.pos) and self._touch is not touch: 
        return False
   return True
def child_handled_event(self, touch, mode):
   touch.push()
   touch.apply_transform_2d(self.to_local)
        
   if self.dispatch_children("on_touch_down", touch):
       touch.pop()
       return True
        
   touch.pop()     
   return False

def on_touch_down(self, touch):
   if super().on_touch_down(touch):
      return True
      
   # do something else if not handled by children
   ....

This seems to be little bit elegant and consistent to me with just one blemish regarding the usage of mode ( which also may need to be used very less).

[Samael-TLB]

Note: There's no right answer - it is a balance of different demands, and at some point it is time to just proceed and not get caught up on analysis paralysis.

Agreed, I would look for your(@Julian-O ) comments to my last post after which I feel we shall proceed ahead.

[Julian-O]

Interesting. In the ButtonBehaviour, we need to call the superclass anyway, and it isn't sufficient to override one of the (new) methods. One vote against the split.

But then it checks the collision point a second time. One vote for the split. DRY on the definition of collision.

But, then you point out that in the one class, this same code is used over and over again for touch_down, touch_up, etc. etc. To me that makes it worth the effort to make it look cleaner.

I am not overly bothered that some widgets will still use the old (current) way, and some will use the new way. They will still work perfectly, and over time they will be simplified.

So, I say do it! But I also say there isn't much in it, and I wouldn't object either way.

[Samael-TLB]

So, I believe we are seeing much good in doing the split so shall it be.

[Samael-TLB]

How does this look? Now each of the potential functions is one statement, and it is clearer that isn't worth saving.

def on_touch_down(self,touch):
    if self.collide_point(*touch.pos): 
        return False  # Don't consume event not on this widget
	
    if self.disabled:
        # Disabled widgets don't pass events to children.
        # Consume event only if opaque
        return self.opacity != 0:
		
    # Does any child want the event?
    return any(
        child.dispatch('on_touch_down', touch)
        for child in self.children[:]
    )

Looks good to me.
Well, in tackling the event propagation I found that there's few widgets which might need some augmenting like in case of relative widgets , e.g., ScrollView, we need to transform the touch to local positions before we pass it on to the children. This entails either we split the code or not

[Samael-TLB]

A small update is that I have been able to fix ScrollView and Carousel. They both now can work together in any hierarchy and also nested scrollveiws or carousels are also able to co-exist from event handling perspective.

Two improvements are I have been able to reduce the code in both of them for touch handling to just a few lines rather than the current convoluted programme flow. This in turn may increase the performance of these widgets.

ScrollView.always_overscroll can now also work in similar way even if ScrollView size is equal to its viewport size.

I'm doing some more testing to find if any edge cases are there in my implementation.

We have a lot of widgets/behaviors which consume events unnecessarily, currently investigating these widgets, checking the impact of rectifying their behaviour.

[Julian-O]

It sounds like what you have already is a big improvement. Consider delivering what you have, before continuing. Reviews are easier in small chunks.

[Samael-TLB]

Yes, I am gonna submit the works in chunks soon. Just having some edge case detection and testing.

[Samael-TLB]

Pasting some resources in regards tp event bubbling and gesture recognition:

1. Event dispatching in Ios/MacOS is done through responder chains and hit testing. Hit testing ignores view objects that are hidden, that have disabled user interactions, or that have an alpha level less than 0.01.

2. These shows that Ios/MacOs follows the proposed event dispatching mechanism.

3. Gesture Recognizer functioning in Ios/MacOs is such that they dont participate in event chaining but receive the touch before the widget they are attached to.