Hi,

Yeah this feels like the Clutter switch to the GDK backend, I think we saw this the last time it was tried as well.

So, the work-around for this when we experienced the same when just clicking the map was to make sure we do not do anything that generates GTK events in the Clutter event handler.

For this case the code for the right-click is here:
https://git.gnome.org/browse/gnome-maps/tree/src/contextMenu.js#n47

I wonder if the menu.popup is to blame. I will not have time to test this for a couple of days (travelling to your home town this afternoon). If you have time could you try either:

1) Doing the menu.popup in an idle.

2) Not use clutter events for button-release (maybe have the event on the mapView not the mapView.view)

Jonas

There's also a third, more controversial option, which is selecting the X11 windowing system backend for Maps, at least until you fix the usage of Clutter to remove all event handling.

You can call:

  clutter_set_windowing_backend (CLUTTER_WINDOWING_X11);

Prior to calling clutter_init() or gtk_clutter_init(). This will make Clutter and Clutter-GTK use the X11 backend, like it did until Clutter 1.20, and give you more time to fix Maps.

(In reply to Jonas Danielsson from comment #1)
> I wonder if the menu.popup is to blame. I will not have time to test this
> for a couple of days (travelling to your home town this afternoon). If you
> have time could you try either:
> 
> 1) Doing the menu.popup in an idle.
> 
> 2) Not use clutter events for button-release (maybe have the event on the
> mapView not the mapView.view)

I can confirm that wrapping ContextMenu._onButtonReleaseEvent (whose calls menu.popup) with an idle solves the problem.

So the question is, how should we proceed, patching this and the scrolling stuff (maybe in libchamplain) or forcing X11 backend?

(In reply to Damián Nohales from comment #3)
> (In reply to Jonas Danielsson from comment #1)
> > I wonder if the menu.popup is to blame. I will not have time to test this
> > for a couple of days (travelling to your home town this afternoon). If you
> > have time could you try either:
> > 
> > 1) Doing the menu.popup in an idle.
> > 
> > 2) Not use clutter events for button-release (maybe have the event on the
> > mapView not the mapView.view)
> 
> I can confirm that wrapping ContextMenu._onButtonReleaseEvent (whose calls
> menu.popup) with an idle solves the problem.
> 
> So the question is, how should we proceed, patching this and the scrolling
> stuff (maybe in libchamplain) or forcing X11 backend?

Yeah, good questions :)

Obviously forcing X11 backend is not a long-term solution. But I guess it could be done and we could switch back around 3.16.1 or so. I will take a look tonight, while doing the Maps release. If fixing up the scroll is easy I could live with a rule that we do not perform GTK-event thingies from clutter events since they are not that common.

And I think for many things this must be a rule. Because in the case of this right-click thing I believe we must listen to the input event on the Clutter actor. Because we need the button coordinates in that domain. So I don't think it is possible for us to use Clutter for pure rendering. We need to interact with the map, which is Clutter.

So either we tread gently and carry a great dead-lock stick. Or this gets fixed, if possible at all, in Clutter.

(In reply to Jonas Danielsson from comment #4)
 
> And I think for many things this must be a rule. Because in the case of this
> right-click thing I believe we must listen to the input event on the Clutter
> actor. Because we need the button coordinates in that domain. So I don't
> think it is possible for us to use Clutter for pure rendering. We need to
> interact with the map, which is Clutter.

The coordinates you're getting from Clutter are the same you'd be getting from GDK — literally, since both relay pointer coordinates from the same windowing system. If you want to know the actor underneath the pointer coordinates, you can simply ask Clutter.

> So either we tread gently and carry a great dead-lock stick. Or this gets
> fixed, if possible at all, in Clutter.

I'm doubtful this can be fixed in Clutter, or in Clutter-GTK. We cannot know beforehand if a certain event is going to cause re-entrancy issues inside Clutter or GDK, because any event emission can cause another event to be synthesized by either toolkits, or by user code.

(In reply to Emmanuele Bassi (:ebassi) from comment #5)
> (In reply to Jonas Danielsson from comment #4)
>  
> > And I think for many things this must be a rule. Because in the case of this
> > right-click thing I believe we must listen to the input event on the Clutter
> > actor. Because we need the button coordinates in that domain. So I don't
> > think it is possible for us to use Clutter for pure rendering. We need to
> > interact with the map, which is Clutter.
> 
> The coordinates you're getting from Clutter are the same you'd be getting
> from GDK — literally, since both relay pointer coordinates from the same
> windowing system. If you want to know the actor underneath the pointer
> coordinates, you can simply ask Clutter.
> 

Ah, great, then we can probably fix this up that way as well.

> > So either we tread gently and carry a great dead-lock stick. Or this gets
> > fixed, if possible at all, in Clutter.
> 
> I'm doubtful this can be fixed in Clutter, or in Clutter-GTK. We cannot know
> beforehand if a certain event is going to cause re-entrancy issues inside
> Clutter or GDK, because any event emission can cause another event to be
> synthesized by either toolkits, or by user code.

Yeah, gotcha.

Fixed this with a work-around, lets keep the Gdk backend.