[Wayland-bugs] [Bug 771561] Epiphany application mode on Wayland broken with WebKit 2.13.92

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=771561

--- Comment #11 from Olivier Fourdan  ---
Created attachment 335854
  --> https://bugzilla.gnome.org/attachment.cgi?id=335854=edit
Simple reproducer

Ok, trying to root cause this issue, I think I know what's going on...

Long story short, the toplevel epiphany window is switching to maximized state
while realizing the window meaning that the shadows are 0, but the backend is
not yet aware. That is definitely not a bug in epiphany, nor in gdk Wayland
backend, but rather in gtk itself (ie higher in the stack)

Basically, what debugging in gdb shows is, initially, the min size set is 99
(in my case, the actual value doesn't matter, this may vary depending on the
theme/and or DPI and/or font size, etc.)

Then, a few iterations later, it is set to 47, which leads to the min size
being negative once you remove the shadow margins in the backend.

The initial value of 99 = 47 + 23 + 29, i.e. in
gtk_window_get_preferred_height() minimum_size = title_min + window_border.top
+ window_border.bottom;

The last value of 47 is, well, 47 + 0 + 0 because
gtk_container_get_border_width() returned (0,0,0,0), because the window has
transitioned to the maximized state in between, but the Wayland backend still
applies the "old" shadow margins...

Ths attached simple reproducer (similar program as before, just added a
"gtk_window_maximize (GTK_WINDOW(window))" prior to showing the window shows
the behavior.

So actually, I think we should not even clamp (although it would be safe), we
should tahter fix the upper layers to keep the shadows margins in sync, if
doable...

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 771561] Epiphany application mode on Wayland broken with WebKit 2.13.92

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=771561

--- Comment #12 from Olivier Fourdan  ---
(In reply to Olivier Fourdan from comment #11)
> gtk_container_get_border_width() returned (0,0,0,0), because the window has
> [...]

Sorry, I meant get_shadow_width() here, not gtk_container_get_border_width()
obviously.

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 771561] Epiphany application mode on Wayland broken with WebKit 2.13.92

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=771561

--- Comment #13 from Olivier Fourdan  ---
Created attachment 335859
  --> https://bugzilla.gnome.org/attachment.cgi?id=335859=edit
[PATCH] gtkwindow: Update shadow size on state change

Otherwise, with CSD, we could have a discrepancy where gtk uses the right
values for the shadows whereas the gdk backend still uses the old values,
leading in some cases to invalid or negative min size being computed (which, in
Wayland, leads a protocol error).

This fixes the issue at the upper, gtk level and avoids the invalid min size
being set in Wayland (and consequently the protocol error).

You may still want to clamp() the values in
gdk_wayland_window_set_geometry_hints() but that should not be needed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 771561] Epiphany application mode on Wayland broken with WebKit 2.13.92

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=771561

Matthias Clasen  changed:

   What|Removed |Added

 Attachment #335859|none|accepted-commit_now
 status||

--- Comment #14 from Matthias Clasen  ---
Review of attachment 335859:

ok

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 771561] Epiphany application mode on Wayland broken with WebKit 2.13.92

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=771561

Olivier Fourdan  changed:

   What|Removed |Added

 Attachment #335859|accepted-commit_now |committed
 status||

--- Comment #15 from Olivier Fourdan  ---
Comment on attachment 335859
  --> https://bugzilla.gnome.org/attachment.cgi?id=335859
[PATCH] gtkwindow: Update shadow size on state change

attachment 335859 pished to git mater as commit 4cb1b96 - gtkwindow: Update
shadow size on state change

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 771561] Epiphany application mode on Wayland broken with WebKit 2.13.92

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=771561

Olivier Fourdan  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 771561] Epiphany application mode on Wayland broken with WebKit 2.13.92

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=771561

Olivier Fourdan  changed:

   What|Removed |Added

 Attachment #335777|none|needs-work
 status||

--- Comment #8 from Olivier Fourdan  ---
Review of attachment 335777:

(In reply to Sjoerd Simons from comment #7)
> The geometry pased to _set_geometry_hints doesn't take shadow margins
> into account so don't decrease the width/height with the margins.

Actually, it does when using CSD (as in Wayland), so this patch is not correct,
the correct approach imho is to clamp as suggested by Carlos.

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 771561] Epiphany application mode on Wayland broken with WebKit 2.13.92

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=771561

--- Comment #9 from Sjoerd Simons  ---
CSD != shadows.

If your claim is correct (which i don't think it is, but i could be wrong)
other code in gdkwindow-wayland is obviously wrong as
gdk_wayland_window_get_window_geometry already removed the margins for the
geometry, so calling _set_geometry_hints with that geometry will decrease with
the margins twice.. 

clamping make sense to prevent protocol errors, but from what i can tell the
problem here isn't the application given dumb values...

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs

[Wayland-bugs] [Bug 771561] Epiphany application mode on Wayland broken with WebKit 2.13.92

[gtk+]

https://bugzilla.gnome.org/show_bug.cgi?id=771561

--- Comment #10 from Olivier Fourdan  ---
Created attachment 335846
  --> https://bugzilla.gnome.org/attachment.cgi?id=335846=edit
Sample program

(In reply to Sjoerd Simons from comment #9)
> CSD != shadows.

Not sure what you mean by that sentence...

With CSD, the shadows are drawn by the client, i.e. they are within the
toplevel GdkWindow size whereas with SSD the shadows are drawn by the window
manager, ie outside of the GdkWindow.

That's precisely why you need _GTK_FRAME_EXTENTS for CSD on X11.

> If your claim is correct (which i don't think it is, but i could be wrong)
> other code in gdkwindow-wayland is obviously wrong as
> gdk_wayland_window_get_window_geometry already removed the margins for the
> geometry, so calling _set_geometry_hints with that geometry will decrease
> with the margins twice.. 
> 
> clamping make sense to prevent protocol errors, but from what i can tell the
> problem here isn't the application given dumb values...

To demonstrate why we need to remove the shadow sizes, please consider the
attached sample program.

What this code does is trivially setting the min height of its window to 1
pixel usign the GtkWindow API (gtk_window_set_geometry_hints()).

Now, this needs to work in all 3 following cases:

1. X11 without CSD (i.e. with SSD)
2. X11 with CSD
3. Wayland

Which means that the geometry value passed to X11 must match what the program
sets as-is, whereas the values given to CSD must add up the header bar and the
shadow values.

This program works as expected in all 3 cases, ie the window *content* (ie not
counting the shadows and hederbar) minimum is 1 in X11 with SSD, X11 with CSD
and Wayland with CSD.

Applying your patch results in the min height being too large in Wayland as
seen in WAYLAND_DEBUG=1 logs:

Without your patch:

[3747632.054]  -> zxdg_toplevel_v6@28.set_min_size(248, 37)
[3747632.062]  -> zxdg_toplevel_v6@28.set_max_size(248, 248)
[3747632.070]  -> zxdg_surface_v6@27.set_window_geometry(26, 23, 248, 239)

With your patch:

[3782500.453]  -> zxdg_toplevel_v6@28.set_min_size(300, 89)
[3782500.462]  -> zxdg_toplevel_v6@28.set_max_size(300, 300)
[3782500.472]  -> zxdg_surface_v6@27.set_window_geometry(26, 23, 248, 248)

And resizing to the min gives:

[4270822.924] zxdg_toplevel_v6@28.configure(248, 37, array)

And 37 is the height of the gtkheaderbar, matching the min height (without your
patch) whereas your patch gives a min height of 89.

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
wayland-bugs mailing list
wayland-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-bugs