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 767713 - Fullscreen in wayland is buggy
Fullscreen in wayland is buggy
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
3.22.x
Other Linux
: Normal major
: ---
Assigned To: gtk-bugs
gtk-bugs
: 766241 771900 773006 (view as bug list)
Depends on:
Blocks: WaylandRelated
 
 
Reported: 2016-06-16 04:43 UTC by Akilan Elango
Modified: 2016-11-14 11:34 UTC
See Also:
GNOME target: ---
GNOME version: 3.19/3.20


Attachments
Notice the top empty bar. (964.99 KB, image/png)
2016-06-16 04:43 UTC, Akilan Elango
  Details
Abnormal controls - popover gets formed at a position above the actual button widget (978.47 KB, image/png)
2016-06-16 04:45 UTC, Akilan Elango
  Details
Exiting fullscreen from the abnormal mode : notice the empty bar (893.49 KB, image/png)
2016-06-16 04:49 UTC, Akilan Elango
  Details
My mouse pointer was in the bottom empty bar just below the Pause button : notice the location of the popup (Pause) (895.80 KB, image/png)
2016-06-16 04:56 UTC, Akilan Elango
  Details
[PATCH] gdkwindow: configure native windows in move_native_children() (2.15 KB, patch)
2016-10-10 13:50 UTC, Olivier Fourdan
none Details | Review
[PATCH 1/3] Revert "gdk: Get rid of unused variables" (1.18 KB, patch)
2016-11-02 09:31 UTC, Olivier Fourdan
committed Details | Review
[PATCH 2/3] Revert "gdkwindow: configure native windows in move_native_children()" (1.73 KB, patch)
2016-11-02 09:32 UTC, Olivier Fourdan
committed Details | Review
[PATCH 3/3] gtkstack: reorder size_allocate and move_window (2.52 KB, patch)
2016-11-02 09:32 UTC, Olivier Fourdan
none Details | Review
[PATCH 3/3 v2] gtkstack: reorder size_allocate and move_window (2.11 KB, patch)
2016-11-02 09:53 UTC, Olivier Fourdan
committed Details | Review

Description Akilan Elango 2016-06-16 04:43:58 UTC
Created attachment 329885 [details]
Notice the top empty bar.

When I double click on the video canvas(press F11 or F) to enter the full screen, while the window is not maximized, works good. But when the window is maximized I get really undesired results :
The auto hide controls header bar on the top is pushed down and a new blank bar and an partially hidden seek controls bar. When I press on the controls on the original header bar, it doesn't work. But it works if press on the exact horizontal location on the empty bar above. A similar thing happens when I exit the full screen from that abnormal mode. The Top header bar is partially hidden and a blank bar pushes up the seek controls bar and the similar things happen. I'll attach the screenshots.

Thank you.
Comment 1 Akilan Elango 2016-06-16 04:45:38 UTC
Created attachment 329886 [details]
Abnormal controls - popover gets formed at a position above the actual button widget
Comment 2 Akilan Elango 2016-06-16 04:49:23 UTC
Created attachment 329887 [details]
Exiting fullscreen from the abnormal mode : notice the empty bar
Comment 3 Akilan Elango 2016-06-16 04:56:48 UTC
Created attachment 329888 [details]
My mouse pointer was in the bottom empty bar just below the Pause button : notice the location of the popup (Pause)
Comment 4 André Klapper 2016-06-16 14:38:34 UTC
Which graphics driver is used here?
Comment 5 Akilan Elango 2016-06-16 14:39:42 UTC
intel-mesa-gl
Comment 6 gan lu 2016-09-03 02:26:30 UTC
I have the same problem, I am running on Arch current, which is gnome 3.20.4, totem 3.20.1, wayland 1.11.0 and have a intel integrated GPU of Ivybridge.
Comment 7 Bastien Nocera 2016-09-06 11:11:15 UTC
Please test with the clutter-gtk from git master.

(Or you can try to backport this commit:
https://git.gnome.org/browse/clutter-gtk/commit/?id=88d74c56612773dc350efde57eff97f461f438e6)
Comment 8 Atri 2016-09-23 23:50:43 UTC
Still a problem with totem 3.22. The ugly header bar still shows.
Comment 9 Olivier Fourdan 2016-10-10 13:50:27 UTC
Created attachment 337316 [details] [review]
[PATCH] gdkwindow: configure native windows in move_native_children()

ClutterEmbed on Wayland uses a subsurface and relocate it on a configure
handler, but when placed within a scrolled window, no configure event is
emitted and the CLutterEmbed subsurface remains static.

Emit a configure event for native windows in GdkWindow's internal
move_native_children() so that custom widgets relying on configure
events such as ClutterEmbed can relocate their stuff.
--
Right, this patch here solves the problem and is actually the same as the patch for bug 771320.
Comment 10 Olivier Fourdan 2016-10-10 13:53:21 UTC
(In reply to Bastien Nocera from comment #7)
> Please test with the clutter-gtk from git master.
> 
> (Or you can try to backport this commit:
> https://git.gnome.org/browse/clutter-gtk/commit/
> ?id=88d74c56612773dc350efde57eff97f461f438e6)

I don't think this particular issue is related to duble buffering, I can reproduce at will using clutter and clutter-gtk from git master.

I think this is because of gtk's own internal layout management. FWIW attachment 337316 [details] [review] solves the issue for me, but I'm quite unsure of the possible side efefcts, this code seems to be very sensitive...
Comment 11 Olivier Fourdan 2016-10-10 13:56:33 UTC
Possible duplicate of bug 771900
Comment 12 Olivier Fourdan 2016-10-10 15:13:18 UTC
For completion, it will need the patch for cogl from bug 772707 as well.
Comment 13 Olivier Fourdan 2016-10-13 07:35:15 UTC
*** Bug 771900 has been marked as a duplicate of this bug. ***
Comment 14 Olivier Fourdan 2016-10-13 07:36:38 UTC
Should be fixed with commit 9e2b1ad in master and commit 12579fe in branch gtk-3.22 - gdkwindow: configure native windows in move_native_children()
Comment 15 Olivier Fourdan 2016-10-13 13:25:37 UTC
*** Bug 766241 has been marked as a duplicate of this bug. ***
Comment 16 Bastien Nocera 2016-10-16 00:41:05 UTC
*** Bug 773006 has been marked as a duplicate of this bug. ***
Comment 17 Olivier Fourdan 2016-10-28 14:30:06 UTC
I would reopen this bug, commit 9e2b1ad in master and commit 12579fe in branch gtk-3.22 are causing side effects (like in gvim, see bug 773387) so we would have to revert these changes eventually, I'm afraid.

So we need to fix this issue differently.

On Wayland, clutter uses a subsurface that it needs to resize/reposition along with the corresponding GtkClutterEmbed widget.

To do so, it implements its own size_allocate [1] to move the corresponding gdk window and emit a configure event that the clutter gdk backend will receive and use to resize/reposition the actual Wayland subsurface [2].

To place the subsurface right where it should be, it's using gdk_window_get_origin() will calls in gdk_window_get_root_coords() and eventually gdk_window_wayland_get_root_coords().

The root coordinate is computed by adding the window (x,y) to the the window (abs_x,abs_y), and this is where (abs_x,abs_y) is wrong and still accounts for the title bar height whereas it's just been hidden.

The window tree goes like this:
  [GtkApplicationWindow] 0,0 1920x1200 impl(0x0) toplevel abs[0,0] clipbox[{1920x1200 @0,0}]
     [GtkMenuButton] 1753,6 34x34 input-only hidden abs[1753,6] clipbox[{empty}]
     [GtkButton] 1713,6 34x34 input-only hidden abs[1713,6] clipbox[{empty}]
     [GtkButton] 39,6 34x34 input-only hidden abs[39,6] clipbox[{empty}]
     [GtkMenuButton] -1,-1 1x1 input-only hidden abs[-1,-1] clipbox[{empty}]
     [GtkToggleButton] 1536,23 34x46 input-only hidden abs[1536,23] clipbox[{empty}]
     [GtkToggleButton] 1576,23 34x46 input-only hidden abs[1576,23] clipbox[{empty}]
     [GtkStack] 703,11 514x24 hidden abs[703,11] clipbox[{empty}]
         [GtkStack] 0,0 514x24 abs[703,11] clipbox[{empty}]
             [GtkMenuButton] -1,-1 1x1 input-only hidden abs[702,10] clipbox[{empty}]
     [GtkStack] 0,47 1920x1153 abs[0,47] clipbox[{1920x1153 @0,0}]
         [GtkStack] 0,0 1920x1153 abs[0,47] clipbox[{1920x1153 @0,0}]
             [BaconVideoWidget] 0,0 1920x1153 abs[0,47] clipbox[{1920x1153 @0,0}]
             [GtkRevealer] -1,-1 1x1 hidden abs[-1,46] clipbox[{empty}]
                 [GtkRevealer] 0,-46 1x47 abs[-1,0] clipbox[{empty}]
             [GdMainView] -1,-1 1x1 hidden abs[-1,46] clipbox[{empty}]
                 [GdMainIconView] -1,-1 1x1 hidden abs[-2,45] clipbox[{empty}]
                     [GdMainIconView] 0,0 1x1 abs[-2,45] clipbox[{empty}]
                 [GdMainView] -12,1 13x1 hidden abs[-13,47] clipbox[{empty}]
                     [GtkScrollbar] -1,-1 1x1 input-only hidden abs[-14,46] clipbox[{empty}]
                 [GdMainView] 1,-12 1x13 hidden abs[0,34] clipbox[{empty}]
                     [GtkScrollbar] -1,-1 1x1 input-only hidden abs[-1,33] clipbox[{empty}]
             [GtkOverlay] 0,0 1717x1 hidden abs[0,47] clipbox[{empty}]
     [GtkApplicationWindow] 1723,1076 30x30 input-only shaped hidden abs[1723,1076] clipbox[{empty}]
     [GtkApplicationWindow] 46,1096 1677x10 input-only hidden abs[46,1096] clipbox[{empty}]
     [GtkApplicationWindow] 16,1076 30x30 input-only shaped hidden abs[16,1076] clipbox[{empty}]
     [GtkApplicationWindow] 1743,43 10x1033 input-only hidden abs[1743,43] clipbox[{empty}]
     [GtkApplicationWindow] 16,43 10x1033 input-only hidden abs[16,43] clipbox[{empty}]
     [GtkApplicationWindow] 1723,13 30x30 input-only shaped hidden abs[1723,13] clipbox[{empty}]
     [GtkApplicationWindow] 46,13 1677x10 input-only hidden abs[46,13] clipbox[{empty}]
     [GtkApplicationWindow] 16,13 30x30 input-only shaped hidden abs[16,13] clipbox[{empty}]

The window in question here is [BaconVideoWidget] 0,0 1920x1153 abs[0,47] clipbox[{1920x1153 @0,0}]

On further investigation, the problem here occurs because the shadows width doesn't change between maximized and fullscreen, whereas the header bar disappears.

So this makes me think it works when transitioning from normal state to maximized or fullscreen and vice versa only by chance somehow, because the css theme used has a different shadow width (or rather sets a shadow width for normal state and none for maximized/fullscreen).

This is because the shadow width is taken into account in _gtk_window_set_allocation() [3], and because it doesn't change between fullscreen and maximized, we do not get another round of gtk_widget_size_allocate().

Whereas, when transitioning from normal state to fullscreen, we get a second round of gtk_widget_size_allocate(), with the abs_x/abs_y recomputed correctly and gdk_window_get_origin() returns the right values, so the subsurface get placed correctly, eventually.

(Note that also explains the regression found in bug 771915 introduced by commit 4cb1b96 which imho was correct though, to address bug 771561)

[1] https://git.gnome.org/browse/clutter-gtk/tree/clutter-gtk/gtk-clutter-embed.c#n670
[2] https://git.gnome.org/browse/clutter/tree/clutter/gdk/clutter-stage-gdk.c#n267
[3] https://git.gnome.org/browse/gtk+/tree/gtk/gtkwindow.c?h=gtk-3-22#n7580
Comment 18 Cédric Bellegarde 2016-10-29 14:41:30 UTC
Can confirm a regression due to commit https://git.gnome.org/browse/gtk%2B/commit/?id=12579fe

Since this commit, Lollypop is freezing while scrolling in Album View...

See: https://github.com/gnumdk/lollypop/issues/788

Reverting this commit (12579fe) fixes the issue...

But what is really strange:
- Disabling Gio.FileMonitor() for lollypop collection also fixes the issue
- Removing this line also fixes the issue:
https://github.com/gnumdk/lollypop/blob/master/src/inotify.py#L49

Really can't understand how keeping a Gio.FileMonitor() in a python dict can make scrolledwindow freezing...
Comment 19 Cédric Bellegarde 2016-10-29 14:46:57 UTC
Removed dict from lollypop code and bug is gone...
Comment 20 Owen Taylor 2016-10-31 15:57:48 UTC
Isn't the problem here that gtk_stack_allocate() orders things as:

 - Allocate child
 - Move windows

Which is different then, say, GtkEventBox or GtkView - I think reordering would be a complete fix here, though it doesn't help with bug 771320.

I certainly agree that emitting Configure events for child windows has definitely some downsides. Configure events were always defined to be:
 - Toplevel only
 - Synthesized by GtkDrawingArea as a weird hack that I regret - I think the idea was that the widget woudl reliably be realized when you got ::configure-event but not ::size-allocate - something like that - it somehow made the GTK+ tutorial look simpler.

So you confuse GtkDrawingArea users - at a minimum you'd have to not deliver ConfigureEvent to GtkDrawingArea with a gtkmain.c hack for compatibility.
I think you're also going to lag a frame, since the event won't get delivered until *after* allocate, redraw.

The clean way to do subwindow notification would have been to add a ::position-in-toplevel-changed signal to GdkWindow - but I'm not at all sure what to do now since 3.22 is out and supposed to be API/ABI stable - removing the configure event sending is also an ABI break now that it was added and released.

Maybe add the gtkmain.c hack, accept the frame lag, on to GTK+ 4? Do we know of any issues related to sending the ConfigureEvents that don't have to do with GtkDrawingArea?
Comment 21 Cédric Bellegarde 2016-10-31 18:14:49 UTC
My bad, it doesn't fix anything.

Please revert commit 12579fe, it makes GTK broken for apps with many widgets...

Ex:
- Lollypop with Gtk 3.21, smooth scrolling:
https://youtu.be/qqxNEaJJHxM
- Lollypop with Gtk 3.22, freezing scrolling, CPU up to 100% usage:
https://youtu.be/84Q2JRZNrY8

Applying this patch(12579fe revert) fix the issue:
--- gtk+-3.22.2/gdk/gdkwindow.c	2016-10-22 06:14:15.000000000 +0200
+++ gtk+-3.22.1/gdk/gdkwindow.c	2016-08-24 20:43:50.000000000 +0200
@@ -5975,23 +5972,6 @@
   _gdk_synthesize_crossing_events_for_geometry_change (window);
 }
 
-static void
-configure_native_child (GdkWindow *window)
-{
-  GdkEvent *event;
-
-  event = gdk_event_new (GDK_CONFIGURE);
-
-  event->configure.window = g_object_ref (window);
-  event->configure.send_event = FALSE;
-  event->configure.x = window->x;
-  event->configure.y = window->y;
-  event->configure.width = window->width;
-  event->configure.height = window->height;
-
-  gdk_event_put (event);
-  gdk_event_free (event);
-}
 
 static void
 move_native_children (GdkWindow *private)
@@ -6012,10 +5992,7 @@
 				   child->width, child->height);
 	}
       else
-        {
-          configure_native_child (child);
-          move_native_children (child);
-        }
+	move_native_children  (child);
     }
 }
 
@@ -6030,6 +6007,7 @@
   cairo_region_t *old_region, *new_region;
   GdkWindowImplClass *impl_class;
   gboolean expose;
+  int old_abs_x, old_abs_y;
 
   g_return_if_fail (GDK_IS_WINDOW (window));
 
@@ -6087,6 +6065,9 @@
       window->height = height;
     }
 
+  old_abs_x = window->abs_x;
+  old_abs_y = window->abs_y;
+
   recompute_visible_regions (window, FALSE);
 
   if (gdk_window_has_impl (window))
@@ -6099,7 +6080,8 @@
 			       window->x, window->y,
 			       window->width, window->height);
     }
-  else
+  else if (old_abs_x != window->abs_x ||
+	   old_abs_y != window->abs_y)
     move_native_children (window);
 
   if (expose)
Comment 22 Cédric Bellegarde 2016-10-31 18:35:00 UTC
>removing the configure event sending is also an ABI break now that it was added 
>and released.

Even if configure event was added between 3.22.1 and 3.22.2 release?
Comment 23 Matthias Clasen 2016-11-01 00:47:33 UTC
(In reply to Owen Taylor from comment #20)
 
> Maybe add the gtkmain.c hack, accept the frame lag, on to GTK+ 4? Do we know
> of any issues related to sending the ConfigureEvents that don't have to do
> with GtkDrawingArea?

Sounds plausible to me
Comment 24 Cédric Bellegarde 2016-11-01 09:20:12 UTC
I'm not using GtkDrawingArea in Lollypop.
Comment 25 Cédric Bellegarde 2016-11-01 10:45:52 UTC
Here a simple code reproducing the issue, this works with GTK 3.22.1 or GTK 3.22.2 less commit 12579fe.

import gi
gi.require_version('Gtk', '3.0')
from gi.repository import Gtk, Gio
import os


flowbox = Gtk.FlowBox()
flowbox.set_selection_mode(Gtk.SelectionMode.NONE)
flowbox.set_homogeneous(True)
flowbox.set_max_children_per_line(1000)

scrolled = Gtk.ScrolledWindow()
scrolled.add(flowbox)

win = Gtk.Window()
win.connect("delete-event", Gtk.main_quit)
win.resize(800, 800)
win.add(scrolled)

for i in range(0, 2000):
   image = Gtk.Image.new_from_icon_name("gedit", Gtk.IconSize.DIALOG)
   eventbox = Gtk.EventBox()
   eventbox.set_size_request(200, 200)
   eventbox.add(image)
   flowbox.add(eventbox)
win.show_all()
Gtk.main()
Comment 26 Cédric Bellegarde 2016-11-01 10:50:28 UTC
Seems GtkEventBox widgets get flooded by events on scrolling:
- Not visible when scrolling with scrollbar
- Visible when scrolling with mouse wheel
- Really visible when scrolling with touchpad
Comment 27 Cédric Bellegarde 2016-11-01 11:10:03 UTC
That's it, with Gtk 3.22, eventbox get flooded on scroll:
<Gdk.Event object at 0x7f4f7f460b28 (GdkEvent at 0x3a79180) type=<enum GDK_CONFIGURE of type Gdk.EventType>>
<Gdk.Event object at 0x7f4f7f460b28 (GdkEvent at 0x3a790e0) type=<enum GDK_CONFIGURE of type Gdk.EventType>>
<Gdk.Event object at 0x7f4f7f460b28 (GdkEvent at 0x3a795e0) type=<enum GDK_CONFIGURE of type Gdk.EventType>>
<Gdk.Event object at 0x7f4f7f460b28 (GdkEvent at 0x3a78660) type=<enum GDK_CONFIGURE of type Gdk.EventType>>
<Gdk.Event object at 0x7f4f7f460b28 (GdkEvent at 0x3a78160) type=<enum GDK_CONFIGURE of type Gdk.EventType>>
<Gdk.Event object at 0x7f4f7f460b28 (GdkEvent at 0x3a79b20) type=<enum GDK_CONFIGURE of type Gdk.EventType>>
<Gdk.Event object at 0x7f4f7f460b28 (GdkEvent at 0x3a78480) type=<enum GDK_CONFIGURE of type Gdk.EventType>>
<Gdk.Event object at 0x7f4f7f460b28 (GdkEvent at 0x3a79040) type=<enum GDK_CONFIGURE of type Gdk.EventType>>

Without commit 12579fe, no configure event.
Comment 28 Owen Taylor 2016-11-01 15:53:27 UTC
(In reply to Cédric Bellegarde from comment #25)

> for i in range(0, 2000):
>    image = Gtk.Image.new_from_icon_name("gedit", Gtk.IconSize.DIALOG)
>    eventbox = Gtk.EventBox()
>    eventbox.set_size_request(200, 200)
>    eventbox.add(image)
>    flowbox.add(eventbox)
> win.show_all()
> Gtk.main()

Hmm, OK, so this has 2000 windows in a flow box in a scrolled window - it's probably better to avoid on any version of GTK+, but if it works in GTK+-3.22.0 it needs to keep on working in future version of GTK+ 3!

It's not surprising that there's overhead from emitting 2000 events on every scroll - I'm a little surprised that it is so bad that it causes a complete lockup. I wonder if this is a11y releated - see:

 a11y/gtkaccessibility.c:configure_event_watcher().

The event pausing in gdkwindow.c/gdkdisplay.c is supposed to prevent a complete lockup in cases where event processing takes a lot of time - we still go ahead and draw frames - but maybe the frames get very apart. Does it matter if you are dragging the scrollbar (compression) or with scroll events (no compression)?

Some profiling would be the next step.
Comment 29 Cédric Bellegarde 2016-11-01 16:13:05 UTC
>Does it matter if you are dragging the scrollbar (compression) or with scroll 
>events (no compression)?

Dragging the scrollbar works better.

>it's probably better to avoid on any version of GTK+

How should I do if I want to get enter/leave events on flowbox child(to show overlay buttons on album cover)? And lollypop has more EventBox in FlowBoxChild to draw overlay button (and get button press event)... Is there a better solution than EventBox?
Comment 30 Olivier Fourdan 2016-11-02 07:56:02 UTC
(In reply to Owen Taylor from comment #20)
> Isn't the problem here that gtk_stack_allocate() orders things as:
> 
>  - Allocate child
>  - Move windows
> 
> Which is different then, say, GtkEventBox or GtkView - I think reordering
> would be a complete fix here, though it doesn't help with bug 771320.

Woohoo, thanks a bunch Owen, yes, absolutely, this *is* the bug! I've been searching for several days and could not spot it, this is great!

As for bug 771320, well, this needs another fix, but this is expected anyway.

> [...]
> The clean way to do subwindow notification would have been to add a
> ::position-in-toplevel-changed signal to GdkWindow - but I'm not at all sure
> what to do now since 3.22 is out and supposed to be API/ABI stable -
> removing the configure event sending is also an ABI break now that it was
> added and released.

Yes, I thought of that as well, but ruled it out for the same reason you listed.

I would really advocate to revert commit 9e2b1ad  and re-order the size_allocate()/move_resize_window() in gtk_stack_allocate() which is the correct fix for this bug.
Comment 31 Olivier Fourdan 2016-11-02 09:31:23 UTC
Created attachment 338935 [details] [review]
[PATCH 1/3] Revert "gdk: Get rid of unused variables"

This reverts commit 6f7a6f769f1a8a7aa0214acc3d063940a33779c5.
Comment 32 Olivier Fourdan 2016-11-02 09:32:09 UTC
Created attachment 338936 [details] [review]
[PATCH 2/3] Revert "gdkwindow: configure native windows in move_native_children()"

This reverts commit 12579fe71b3b8f79eb9c1b80e429443bcc437dd0.
Comment 33 Olivier Fourdan 2016-11-02 09:32:47 UTC
Created attachment 338937 [details] [review]
[PATCH 3/3] gtkstack: reorder size_allocate and move_window

Unlike other container widgets, GtkStack would allocate its children
prior to moving its windows, which might prevent further valid size
allocation signals to be emitted.

Re-order the size allocation of child widgets to be performed after
moving the GtkStack windows.

Thanks to Owen for spotting the real issue here.
Comment 34 Olivier Fourdan 2016-11-02 09:43:25 UTC
Needs more testing, looking at the git history on gtkstack.c, such a change could possibly cause some other regression (thinking of commit 0d8993f for example):

https://git.gnome.org/browse/gtk+/log/gtk/gtkstack.c?h=gtk-3-22
Comment 35 Olivier Fourdan 2016-11-02 09:53:45 UTC
Created attachment 338940 [details] [review]
[PATCH 3/3 v2] gtkstack: reorder size_allocate and move_window

v2: do not undo commit 0d8993f while at it
Comment 36 Cédric Bellegarde 2016-11-02 12:23:15 UTC
Thanks Olivier, these patches fix Lollypop issue. I've build a package for ArchLinux and tell Lollypop users to test for any regression in GTK apps:
https://github.com/gnumdk/lollypop/issues/788#issuecomment-257848626
Comment 37 Owen Taylor 2016-11-09 08:57:29 UTC
Review of attachment 338940 [details] [review]:

This looks good to me, though there obviously could be some oddity triggered in some usage of GtkStack that is hard to anticipate. The fact that other widgets have this ordering makes me more confident in this.
Comment 38 Owen Taylor 2016-11-09 08:58:27 UTC
Review of attachment 338935 [details] [review]:

Fine if we're reverting the other patch.
Comment 39 Owen Taylor 2016-11-09 08:59:12 UTC
Review of attachment 338936 [details] [review]:

OK - but would be good to coordinate some fix for 771320 in the same GTK+ point release as not to regresss GNOME Maps.
Comment 40 Olivier Fourdan 2016-11-09 09:22:18 UTC
(In reply to Owen Taylor from comment #39)
> OK - but would be good to coordinate some fix for 771320 in the same GTK+
> point release as not to regresss GNOME Maps.

Yes, absolutely, this is what I've been trying to do, but it's more complicated than I anticipated (it always is).

With regard to bug 771320, the idea was to use a gdkwindow (instead of implementing the subsurface directly using wayland API in clutter and clutter-gtk) and link it to the gtk widget.

But the problem I am facing now, apart from a bug in gdk that would not allow to create a native window if the parent is not the root window (even for subwindows on Wayland, I have a patch for that, need to file the bug...), is that by default we use the _gdk_ backend of clutter and it has no visibility on the _gtk_ widget (i.e. it's not in clutter-gtk but clutter itself), so even if I could use gdk API in clutter gdk backend, that doesn't solve the issue of moving the subsurface along with the scrolled gtkwidget.

I am also facing other issues with events and input shapes, basically, the widget doesn't receive the input events anymore when using a subsurface created by gdk.
Comment 41 Olivier Fourdan 2016-11-14 11:15:15 UTC
Comment on attachment 338935 [details] [review]
[PATCH 1/3] Revert "gdk: Get rid of unused variables"

attachment 338935 [details] [review] pushed to branch gtk-3-22 as commit c1507cf - Revert "gdk: Get rid of unused variables"
Comment 42 Olivier Fourdan 2016-11-14 11:16:19 UTC
Comment on attachment 338936 [details] [review]
[PATCH 2/3] Revert "gdkwindow: configure native windows in move_native_children()"

attachment 338936 [details] [review] pushed to branch gtk-3-22 as commit f70039c - Revert "gdkwindow: configure native windows in move_native_children()"
Comment 43 Olivier Fourdan 2016-11-14 11:17:10 UTC
Comment on attachment 338940 [details] [review]
[PATCH 3/3 v2] gtkstack: reorder size_allocate and move_window

attachment 338940 [details] [review] pushed to branch gtk-3-22 as commit 57f551a - gtkstack: reorder size_allocate and move_window
Comment 44 Olivier Fourdan 2016-11-14 11:34:54 UTC
attachment 338935 [details] [review] pushed to git master as commit 5ccc570 - Revert "gdk: Get rid of unused variables"

attachment 338936 [details] [review] pushed to git master as commit 4ae1eab - Revert "gdkwindow: configure native windows in move_native_children()"

attachment 338940 [details] [review] pushed to git master as commit 03b8a8a - gtkstack: reorder size_allocate and move_window