linux4sam / egt

Ensemble Graphics Toolkit - Modern C++ GUI Toolkit for AT91/SAMA5 Microprocessors
https://ensemble.graphics
Apache License 2.0
63 stars 25 forks source link

No pointer_drag events emitted with egt 1.9-rc1 #29

Closed SaschaKoch closed 7 months ago

SaschaKoch commented 7 months ago

Hi,

I just updated to the most recent commit 826751cc and found that there is an issue with pointer_drag events. Just before the update I tested the branch 27-event-handler-sees-no-further-pointer_hold-after-pointer_drag_start where everything worked fine (as described in the last comment of issue #27 ) and pointer_drag events were emitted.

With the master branch and the most recent commit there is a pointer_drag_start emitted but no further pointer_drags anymore. the raw_pointer_move still works as well as the pointer_holds.

The test program from #27 delivers the following results if I "drag" on the widgets:

ImageButton: pointer_hold
ImageButton: pointer_hold
ImageButton: pointer_drag_start
ImageButton: pointer_hold
ImageButton: pointer_hold
ImageButton: pointer_hold
ImageButton: pointer_hold
ImageButton: pointer_hold
ImageButton: pointer_hold
ImageButton: raw_pointer_up
RectWdgt: pointer_drag_start
RectWdgt: pointer_hold
RectWdgt: pointer_hold
RectWdgt: pointer_hold
RectWdgt: pointer_hold
RectWdgt: raw_pointer_up
ImageLabel: pointer_drag_start
ImageLabel: pointer_hold
ImageLabel: pointer_hold
ImageLabel: pointer_hold
ImageLabel: raw_pointer_up

As you can see the pointer_drag_start is emitted but then nothing anymore but the pointer_holds.

Regards Sascha

CyrillePitchen commented 7 months ago

Hi Sascha,

The way 'pointer_drag' and 'pointer_drag_stop' events are handled has changed.

'pointer_drag_start' events, like previously, go through the widget hierarchy. However, now one of them must accept this 'pointer_drag_start' event in order to receive the associated 'pointer_drag' and 'pointer_drag_stop' events.

Also, only one widget can accept the 'pointer_drag_start' event. Once accepted, further widgets in the hierarchy may not receive the 'pointer_drag_start' event at all and should not accept it anyway.

Later, when 'pointer_drag' and 'pointer_drag_stop' are emitted, they are sent directly and only to the widget that has accepted the 'pointer_drag_start' event, if any. Indeed, if no widget at all has accepted the 'pointer_drag_start' event, then no widget receives the associated 'pointer_drag' and 'pointer_drag_stop' events. So, 'pointer_drag' and 'pointer_drag_stop' events, no longer walk through the widget hierarchy.

Then, in order to accept 'pointer_drag_start' events, widgets now have to either: 1 - override the new Widget::internal_drag() virtual method and make it return 'true' or 2 - call user_drag(true) on this widget to enable 'pointer_drag*' events if internal_drag() doesn't return 'true'

/**
 * Tell the start_drag() method whether the widget accepts
 * 'pointer_drag*' events.
 *
 * @return true if and only if the widget accepts 'pointer_drag*' events.
 */
bool accept_drag() const { return internal_drag() || user_drag(); }

Widgets defined in libegt that are supposed to handle drag & drop events, like egt::Slider for instance, override the internal_drag() method. Therefore, there is nothing more to do and they should work as before. Other widgets, like egt::Window, egt::ImageButton or egt::ImageLabel, don't; hence 'pointer_drag_start' events just walk through them but such widgets just ignore these events.

Indeed, widgets inheriting from egt::Frame may not handle 'pointer_drag*' events, but still, one of their descendant may be the target of the 'pointer_drag_start' event. So, the event walk through the hierarchy until one widget accepts it.

Finally, the track_drag() method has been introduced aside the accept_drag() method to tune a little bit how 'pointer_drag*' events are emitted:

/**
 * Tell the continue_drag() method whether the widget tracks
 * 'pointer_drag*' events once they have crossed out the widget boundaries.
 * If not, a 'pointer_drag_stop' event is inserted wihtin the widget
 * boundaries.
 *
 * @return true if and only if the widget tracks 'pointer_drag*' events.
 */
bool track_drag() const { return internal_track_drag() || user_track_drag(); }

If a widget accepts 'pointer_drag_start' events and track_drag() is enabled (for egt::Slider as an example), it works as before in libegt 1.8: even if the mouse pointer moves out of the widget boundaries, this widget still receives 'pointer_drag' and 'pointer_drag_stop' events

On the other hand, if a widget accepts 'pointer_drag_start' events but track_drag() is disabled (egt::ScrolledView for instance), if the mouse pointer moves out the widget boundaries, the 'pointer_drag' event is reshaped into a 'pointer_drag_stop' event and the widget won't receive the further 'pointer_drag' or 'pointer_drag_stop' events associated with the initial 'pointer_drag_start' event.

BR,

Cyrille

SaschaKoch commented 7 months ago

Hi Cyrille,

thanks for the explanation! I tried the user_drag() on libegt defined widgets as well as the internal_drag() const override{} on my own widgets and both works as expected. If I define a widget class with the internal_drag() set to true, I assume there is no way to switch the acceptance of the drag pointers for a dedicated object of that class off via something like user_drag(false)?

Regards Sascha

CyrillePitchen commented 7 months ago

If you override internal_drag(), then you can make it return whatever you want, ie some else but an hard-coded 'true' value.

Hence, for instance, you can make it return user_drag() and set the associated 'user_drag' flag in the constructor if you want the drag & drop feature to be enabled by default for this custom class.

BR,

Cyrille

SaschaKoch commented 7 months ago

Thanks! That was all very helpful and my code works again as expected.

One last question: What made you introduce the change about the need to accept the pointer_drag_starts to further receive pointer_drags? In older EGT versions a widget would just not define an action for a pointer_drag in the handler and would not be further affected either?

Issue can be closed otherwise.

Regards Sascha

CyrillePitchen commented 7 months ago

When we reworked the egt-launcher application, we made use of the egt::ScrolledView widget to create grids of icons that can be swiped, like the launcher of a smartphone.

However, during our tests, we noticed many issues with the egt::ScrolledView. For instance, if we put an egt::Slider as a child of an egt::ScrolledView, when we try to move the slider handle, both the slider and the scrolled view were handling the 'pointer_drag*' events. Hence both the slider handle and the slider itself, with all other children of the scrolled view, were translated.

To solve this issue, we had to find a way so only the expected widget handles 'pointer_drag*' events.

Therefore, when 'pointer_drag_start' events walk through the widgets hierarchy, we want to find the widget that is the uppermost (highest z coordinate) among widgets that handle 'pointer_drag*' events and having a box containing the mouse pointer. Once this widget found, we assume it consumes the 'pointer_drag_start' event so the event should not walk further the hierarchy.

This is why, we now need a mean to know whether a widget handle() method has consumed the 'pointer_drag_start' event.

One solution would have been to patch all overridden handle methods that deal with any 'pointer_drag*' event so those methods would have called Event::stop() on such events. Also, we would have to make sure that all handle() overrides test Event::quit() at the very beginning then return immediately if needed.

We thought this solution would have required too many changes in the source code, including customers applications. Instead, we preferred to introduce the new accept_drag() method so almost everything can be handled within Widget::handle(), leaving handle method overrides unchanged. All developers have to do is just to override internal_drag() or set the 'user_drag' flag if needed in their code, so Widget::handle() can find out whether the widget, actually its overridden handle(), method accepts 'pointer_drag*' events.

Obviously, for this solution to work, we also assume that every override of handle() method calls Widget::start_drag() at some point, directly or indirectly by calling the handle() method of the parent class, so the 'pointer_drag_start' event is consumed and stopped properly. Also, Widget::start_drag() calls detail::dragged(this) so later Input::dispatch() forwards the further 'pointer_drag*' events associated to the 'pointer_drag_start' event only to the widget that has accepted this event.

BR,

Cyrille

SaschaKoch commented 7 months ago

Thanks for the explanation, that is very helpful.