After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 754671 - Improve dealing with foreign stages on GDK
Improve dealing with foreign stages on GDK
Status: RESOLVED FIXED
Product: clutter
Classification: Platform
Component: gdk
unspecified
Other All
: Normal normal
: ---
Assigned To: clutter-maint
clutter-maint
Depends on:
Blocks:
 
 
Reported: 2015-09-07 11:45 UTC by Lionel Landwerlin
Modified: 2015-09-07 17:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gdk: master-clock: disable vsync throttling (1.40 KB, patch)
2015-09-07 11:45 UTC, Lionel Landwerlin
committed Details | Review
gdk: master clock: only process mapped & realized stages (2.86 KB, patch)
2015-09-07 11:45 UTC, Lionel Landwerlin
none Details | Review
gdk: stage: disable some operations for foreign windows (3.46 KB, patch)
2015-09-07 11:45 UTC, Lionel Landwerlin
committed Details | Review
gdk: master clock: only process mapped & realized stages (2.71 KB, patch)
2015-09-07 16:37 UTC, Lionel Landwerlin
committed Details | Review

Description Lionel Landwerlin 2015-09-07 11:45:36 UTC
Here are a few improvement we can make in the GDK backend, in
particular with foreign windows, we want to avoid doing any kind of
processing when the stage in not visible.
Comment 1 Lionel Landwerlin 2015-09-07 11:45:39 UTC
Created attachment 310805 [details] [review]
gdk: master-clock: disable vsync throttling

When running with a master clock based on the GdkFrameClock, we get
synchronized with the compositor, so no need for throttling rendering.

In particular when dealing with foreign windows, we run into deadlocks
in Mesa because of the way the Mesa wayland backend is implemented [1].

[1] : http://cgit.freedesktop.org/mesa/mesa/tree/src/egl/drivers/dri2/platform_wayland.c#n330
Comment 2 Lionel Landwerlin 2015-09-07 11:45:44 UTC
Created attachment 310806 [details] [review]
gdk: master clock: only process mapped & realized stages

When using Clutter embed inside a Gtk application, a stage might end
up realized but not visible. In this case we might discard doing any
kind of animation processing.
Comment 3 Lionel Landwerlin 2015-09-07 11:45:49 UTC
Created attachment 310807 [details] [review]
gdk: stage: disable some operations for foreign windows

Some operations like :
     * resize
     * show/hide
     * set_title
     * set_user_resizable

should be handled by the embedding framework, so disable them for
foreign windows.
Comment 4 Emmanuele Bassi (:ebassi) 2015-09-07 12:03:54 UTC
Review of attachment 310805 [details] [review]:

Yes, makes sense.
Comment 5 Emmanuele Bassi (:ebassi) 2015-09-07 12:05:33 UTC
Review of attachment 310806 [details] [review]:

::: clutter/gdk/clutter-master-clock-gdk.c
@@ +391,3 @@
 {
+  ClutterActor *actor = CLUTTER_ACTOR (stage);
+  if (clutter_actor_is_mapped (actor) && clutter_actor_is_realized (actor))

The "mapped" state implies realization, which means that this is always going to be true.

You can just replace the condition with is_mapped().

@@ +407,1 @@
   g_signal_connect (stage, "notify::realized",

If we're using the "mapped" state, then we don't need the "realized" one.
Comment 6 Emmanuele Bassi (:ebassi) 2015-09-07 12:06:56 UTC
Review of attachment 310807 [details] [review]:

Looks good.
Comment 7 Lionel Landwerlin 2015-09-07 16:37:59 UTC
Created attachment 310845 [details] [review]
gdk: master clock: only process mapped & realized stages

When using Clutter embed inside a Gtk application, a stage might end
up realized but not visible. In this case we might discard doing any
kind of animation processing.
Comment 8 Emmanuele Bassi (:ebassi) 2015-09-07 16:52:38 UTC
Review of attachment 310845 [details] [review]:

Okay.
Comment 9 Lionel Landwerlin 2015-09-07 17:18:36 UTC
Review of attachment 310805 [details] [review]:

Pushed to master.
Comment 10 Lionel Landwerlin 2015-09-07 17:18:46 UTC
Review of attachment 310805 [details] [review]:

Pushed to master.
Comment 11 Lionel Landwerlin 2015-09-07 17:19:04 UTC
Review of attachment 310807 [details] [review]:

Pushed to master.
Comment 12 Lionel Landwerlin 2015-09-07 17:19:14 UTC
Review of attachment 310845 [details] [review]:

Pushed to master.