Created attachment 336191 [details] [review]
[PATCH] gtkwindow: Update on state event only for unrealized

If the window is realized, the shadow width update is handled from
update_realized_window_properties(), doing it on state event even for
all windows confused gtk/gdk internal parent/child positionning.

To avoid the issue, update the shadow width on state event for
unrealized only.

Created attachment 336192 [details] [review]
[PATCH] gtkwindow: Update on state event only for unrealized

Fix typos

Oops, this patch reintroduces bug 771561 that commit 4cb1b96 fixed, so no go...

(In reply to Olivier Fourdan from comment #3)
> Oops, this patch reintroduces bug 771561 that commit 4cb1b96 fixed, so no
> go...

See https://bugzilla.gnome.org/show_bug.cgi?id=771553#c10

(In reply to Bjørn Lie from comment #4)
> See https://bugzilla.gnome.org/show_bug.cgi?id=771553#c10

No, not here,  attachment 336250 [details] [review] from bug 771553 does not help afaics, this is not actually an issue with fullscreen, but with maximize and attachment 336250 [details] [review] wouldn't help here.

I suspect this bug here is caused by the correct shadow border being applied sooner (required to set the correct min/max size, see bug 771561), which sets the right container size sooner and somehow shortcuts the update of abs_x/abs_y in recompute_visible_regions_internal(), or something along these lines...

Comment on attachment 336192 [details] [review]
[PATCH] gtkwindow: Update on state event only for unrealized

Just to make it clear, this patch is a no-go, as it's in effect a revert of the fix for bug 771561 which is a (more) serious issue.

The goal here is to fix this issue without reintroducing bug 771561 again.

(In reply to Olivier Fourdan from comment #6)
> Comment on attachment 336192 [details] [review] [review]
> [PATCH] gtkwindow: Update on state event only for unrealized
> 
> Just to make it clear, this patch is a no-go, as it's in effect a revert of
> the fix for bug 771561 which is a (more) serious issue.
> 
> The goal here is to fix this issue without reintroducing bug 771561 again.

Sure, but with my current "patch" set (I noticed that I have a couple of mutter patches in plus), I can not reproduce bug 771561 anymore.

(In reply to Bjørn Lie from comment #7)
> Sure, but with my current "patch" set (I noticed that I have a couple of
> mutter patches in plus), I can not reproduce bug 771561 anymore.

Not even with attachment 335854 [details] ? If so, can you list your current patch set applied?

(In reply to Olivier Fourdan from comment #5)
> I suspect this bug here is caused by the correct shadow border being applied
> sooner (required to set the correct min/max size, see bug 771561), which
> sets the right container size sooner and somehow shortcuts the update of
> abs_x/abs_y in recompute_visible_regions_internal(), or something along
> these lines...

Quick folllow-up on my previous comment, the shortcut in question is in gtk_widget_size_allocate_with_baseline():

  https://git.gnome.org/browse/gtk+/tree/gtk/gtkwidget.c#n6101

  6102: if (!alloc_needed && !size_changed && !position_changed && !baseline_changed)
  6103:    goto out;

That prevents the size_allocate to be propagated to clutter-gtk's gtk_clutter_embed_size_allocate() which reposition the sub-surface where it should be. Removing this test fixes the issue, but it's still not an /acceptable/ fix.

Well damn... 

I have irccloud.com as a epbiy webapp, and that one just keeps on working fine (before it would crash and die as soon as I tried to resize/maximize the window),
so I went ahead and made bugzilla.gnome.org a web-app, and that does indeed crash and die as soon as I try to launch it.

Sorry for the noise.

I see basically 3 possibilities:

1. Revert commit 4cb1b96 and use a clamp() in gdk_wayland_window_set_geometry_hints() -> That's a quick and dirty hack but would get us out of the problem, but the size contraints might be wrong at some point in time

2. Amend gtk_widget_size_allocate_with_baseline() to chain the size_allocate signal even if the gtk_widget_size_allocate_with_baseline() /thinks/ nothing significant has changed -> Tested, it works, but I am not sure of the side effects on other code and efficiency, as we might end up sending size_allocate signals a lot more often

3. Continue searching for a better solution than 1. and 2.

Created attachment 336367 [details] [review]
[PATCH] wayland: Avoid negative size constraints

Setting the shadow width earlier as done with commit 4cb1b96 to address
bug 771561 proved to cause unexpected side effects on size_allocate
signal propagation.

As the window is sized correctly earlier, the size_allocate signal is
not emitted again in gtk_widget_size_allocate_with_baseline() which
prevents clutter-gtk from relocating its child widget correctly.

To avoid this issue, revert commit 4cb1b96 but make sure the values
passed as min and max size is never negative in Wayland as this is a
protocol error.

With this, the min/max size will be wrong for a short amount of time,
during the state transition, until the shadow width is updated from
gdk_window_set_shadow_width().

This approach is much safer and less intrusive than changing the
size_allocate logic in gtk.

This reverts commit 4cb1b9645e84054c059f174240e8e288c4befe05.

With this patch, gnome-maps comes up correctly initially, but content is mispositioned after maximizing or unmaximizing. A manual resize after the unmaximize makes it snap back into the right position.

err, ignore. I hadn't actually applied the patch

It does fix the content positioning problem in gnome-maps

Review of attachment 336367 [details] [review]:

Looks fine to me. We could, in the future, move the actual setting of the hints (zxdg_toplevel_v6_set_min/max_size()) before the commit (as done with the opaque, input and window geometry regions). It doesn't affect any functionality, its just easier when debugging protocol flows.

Comment on attachment 336367 [details] [review]
[PATCH] wayland: Avoid negative size constraints

attachment 336367 [details] [review] pushed to git master as commit dbd0923 - wayland: Avoid negative size constraints

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