[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

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

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

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

[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

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

Jonas Ådahl  changed:

   What|Removed |Added

 CC||ofour...@redhat.com

--- Comment #6 from Jonas Ådahl  ---
Yea, looks like an incorrect min size, and yes, makes sense to g_warn and clamp
to 0.

-- 
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 #5 from Carlos Garnacho  ---
FWIW, Some gdb debugging:

(gdb) bt
#0  gdk_wayland_window_set_geometry_hints (window=0x6fe330,
geometry=0x7fffcce0, geom_mask=(GDK_HINT_MIN_SIZE | GDK_HINT_BASE_SIZE |
GDK_HINT_WIN_GRAVITY)) at gdkwindow-wayland.c:2981
#1  0x71f0f51a in gdk_window_set_geometry_hints (window=0x6fe330,
geometry=0x7fffcce0, geom_mask=(GDK_HINT_MIN_SIZE | GDK_HINT_BASE_SIZE |
GDK_HINT_WIN_GRAVITY))
at gdkwindow.c:10392
#2  0x726e334a in gtk_window_move_resize (window=0xbd43c0) at
gtkwindow.c:9691
#3  0x726e0771 in gtk_window_check_resize (container=0xbd43c0) at
gtkwindow.c:8304
#4  0x70c40a32 in g_cclosure_marshal_VOID__VOIDv (closure=0x741910,
return_value=0x0, instance=0xbd43c0, args=0x7fffd1a8,
marshal_data=0x726e0682 , 
n_params=0, param_types=0x0) at
/home/carlos/Source/gnome/glib/gobject/gmarshal.c:905
#5  0x70c3e05e in g_type_class_meta_marshalv (closure=0x741910,
return_value=0x0, instance=0xbd43c0, args=0x7fffd1a8, marshal_data=0x348,
n_params=0, param_types=0x0)
at /home/carlos/Source/gnome/glib/gobject/gclosure.c:1024
#6  0x70c3dc20 in _g_closure_invoke_va (closure=0x741910,
return_value=0x0, instance=0xbd43c0, args=0x7fffd1a8, n_params=0,
param_types=0x0)
at /home/carlos/Source/gnome/glib/gobject/gclosure.c:867
#7  0x70c59536 in g_signal_emit_valist (instance=0xbd43c0,
signal_id=120, detail=0, var_args=0x7fffd1a8) at
/home/carlos/Source/gnome/glib/gobject/gsignal.c:3300
#8  0x70c5a6b9 in g_signal_emit (instance=0xbd43c0, signal_id=120,
detail=0) at /home/carlos/Source/gnome/glib/gobject/gsignal.c:3447
#9  0x723ad9c2 in gtk_container_check_resize (container=0xbd43c0) at
gtkcontainer.c:2171
#10 0x723ad5b6 in gtk_container_idle_sizer (clock=0x748220,
container=0xbd43c0) at gtkcontainer.c:2064
#11 0x70c4099f in g_cclosure_marshal_VOID__VOID (closure=0xd02e70,
return_value=0x0, n_param_values=1, param_values=0x7fffd510,
invocation_hint=0x7fffd450, marshal_data=0x0)
at /home/carlos/Source/gnome/glib/gobject/gmarshal.c:875
#12 0x70c3d988 in g_closure_invoke (closure=0xd02e70, return_value=0x0,
n_param_values=1, param_values=0x7fffd510, invocation_hint=0x7fffd450)
at /home/carlos/Source/gnome/glib/gobject/gclosure.c:804
#13 0x70c5ae29 in signal_emit_unlocked_R (node=0x717b30, detail=0,
instance=0x748220, emission_return=0x0, instance_and_params=0x7fffd510)
at /home/carlos/Source/gnome/glib/gobject/gsignal.c:3635
#14 0x70c5a177 in g_signal_emit_valist (instance=0x748220,
signal_id=39, detail=0, var_args=0x7fffd7c8) at
/home/carlos/Source/gnome/glib/gobject/gsignal.c:3391
#15 0x70c5a6b9 in g_signal_emit (instance=0x748220, signal_id=39,
detail=0) at /home/carlos/Source/gnome/glib/gobject/gsignal.c:3447
#16 0x71ef5588 in _gdk_frame_clock_emit_layout (frame_clock=0x748220)
at gdkframeclock.c:634
#17 0x71ef601e in gdk_frame_clock_paint_idle (data=0x748220) at
gdkframeclockidle.c:408
#18 0x71ed8dfd in gdk_threads_dispatch (data=0x86c300) at gdk.c:743
#19 0x709590b5 in g_timeout_dispatch (source=0xbfdcc0,
callback=0x71ed8db8 , user_data=0x86c300) at
/home/carlos/Source/gnome/glib/glib/gmain.c:4672
#20 0x70957225 in g_main_dispatch (context=0x6f1a90) at
/home/carlos/Source/gnome/glib/glib/gmain.c:3201
#21 0x709580a7 in g_main_context_dispatch (context=0x6f1a90) at
/home/carlos/Source/gnome/glib/glib/gmain.c:3854
#22 0x7095828b in g_main_context_iterate (context=0x6f1a90, block=1,
dispatch=1, self=0x6f2b30) at /home/carlos/Source/gnome/glib/glib/gmain.c:3927
#23 0x7095834f in g_main_context_iteration (context=0x6f1a90,
may_block=1) at /home/carlos/Source/gnome/glib/glib/gmain.c:3988
#24 0x70f4b112 in g_application_run (application=0x764140, argc=1,
argv=0x7fffdc88) at /home/carlos/Source/gnome/glib/gio/gapplication.c:2381
#25 0x004303a3 in main ()
(gdb) p *geometry
$4 = {min_width = 405, min_height = 47, max_width = 0, max_height = 0,
base_width = 0, base_height = 0, width_inc = 0, height_inc = 0, min_aspect = 0,
max_aspect = 0, 
  win_gravity = GDK_GRAVITY_NORTH_WEST}
(gdb) p (GdkWindowImplWayland)*window->impl
$5 = {parent_instance = {parent = {g_type_instance = {g_class = 0x6fd5d0},
ref_count = 30, qdata = 0x0}}, wrapper = 0x6fe330, display_server = {outputs =
0x0, wl_surface = 0xc17710, 
xdg_surface = 0x11b9d70, xdg_toplevel = 0x11b9dc0, xdg_popup = 0x0,
gtk_surface = 0x11b8430, wl_subsurface = 0x0, egl_window = 0x0,
dummy_egl_window = 0x0, xdg_exported = 0x0}, 
  egl_surface = 0x0, dummy_egl_surface = 0x0, initial_configure_received = 0,
mapped = 1, use_custom_surface = 0, pending_buffer_attached = 0, pending_commit
= 0, awaiting_frame = 0, 
  hint = GDK_WINDOW_TYPE_HINT_NORMAL, transient_for = 0x0, popup_parent = 0x0,

[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

Carlos Garnacho  changed:

   What|Removed |Added

 CC||jad...@gmail.com

--- Comment #4 from Carlos Garnacho  ---
CCing Jonas. IMHO we should be just warning and clamping min/max sizes to 0 if
we get dumb values, there's indeed something else going on if we end up getting
negative sizes from gtk+ code paths, but this seems like could be easily
triggered from application code, an async protocol error seems harsh to me in
this case.

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