Created attachment 329886 [details]
Abnormal controls - popover gets formed at a position above the actual button widget

Created attachment 329887 [details]
Exiting fullscreen from the abnormal mode : notice the empty bar

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)

Which graphics driver is used here?

intel-mesa-gl

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.

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)

Still a problem with totem 3.22. The ugly header bar still shows.

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.

(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...

Possible duplicate of bug 771900

For completion, it will need the patch for cogl from bug 772707 as well.

*** Bug 771900 has been marked as a duplicate of this bug. ***

Should be fixed with commit 9e2b1ad in master and commit 12579fe in branch gtk-3.22 - gdkwindow: configure native windows in move_native_children()

*** Bug 766241 has been marked as a duplicate of this bug. ***

*** Bug 773006 has been marked as a duplicate of this bug. ***

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

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...

Removed dict from lollypop code and bug is gone...

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?

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)

>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?

(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

I'm not using GtkDrawingArea in Lollypop.

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()

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

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.

(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.

>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?

(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.

Created attachment 338935 [details] [review]
[PATCH 1/3] Revert "gdk: Get rid of unused variables"

This reverts commit 6f7a6f769f1a8a7aa0214acc3d063940a33779c5.

Created attachment 338936 [details] [review]
[PATCH 2/3] Revert "gdkwindow: configure native windows in move_native_children()"

This reverts commit 12579fe71b3b8f79eb9c1b80e429443bcc437dd0.

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.

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

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

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

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.

Review of attachment 338935 [details] [review]:

Fine if we're reverting the other patch.

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.

(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 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 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 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

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