[firefox] GTK popups such as tooltips/context menus flicker on surface commit

[kennylevinsen]

Firefox tracking bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1548496

Some Firefox "popup" surfaces (context menus, tooltips, plugin popups) toggle between visible and invisible for every surface commit. The easiest way to reproduce is to open a context menu with right flick a few times, and then move the cursor around, with every pixel moved causing the state to toggle for some surfaces. It can sometimes take a few tries.

The issue was introduced when Firefox changed its popup handling to create windows with GDK_WINDOW_TYPE_HINT_POPUP_MENU on Wayland, calling gtk_window_set_transient_for to assign the popup to its parent. Before, it used for wayland GDK_WINDOW_TYPE_HINT_UTILITY. The change was introduced in response to this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1532643, by this patch: https://phabricator.services.mozilla.com/D26112.

Tests:

• Sway: Flicker
• Rootston: OK
• Weston: OK
• GNOME: OK

WAYLAND_DEBUG for firefox, opening a context menu, moving the cursor while observing flicker, and then leaving the window:

[1420963,649] wl_pointer@3.motion(2139321, 19,292969, 2126,281250)
[1420963,798] wl_pointer@3.frame()
[1421395,778] wl_pointer@3.button(8692, 2139753, 273, 1)
[1421395,882] wl_pointer@3.frame()
[1421406,286]  -> wl_compositor@6.create_surface(new id wl_surface@62)
[1421406,310]  -> xdg_wm_base@20.create_positioner(new id xdg_positioner@73)
[1421406,315]  -> xdg_positioner@73.set_size(207, 262)
[1421406,320]  -> xdg_positioner@73.set_anchor_rect(21, 2128, 1, 1)
[1421406,328]  -> xdg_positioner@73.set_offset(0, 0)
[1421406,332]  -> xdg_positioner@73.set_anchor(5)
[1421406,335]  -> xdg_positioner@73.set_gravity(8)
[1421406,341]  -> xdg_positioner@73.set_constraint_adjustment(15)
[1421406,344]  -> xdg_wm_base@20.get_xdg_surface(new id xdg_surface@75, wl_surface@62)
[1421406,354]  -> xdg_surface@75.get_popup(new id xdg_popup@74, xdg_surface@30, xdg_positioner@73)
[1421406,362]  -> xdg_positioner@73.destroy()
[1421406,366]  -> wl_surface@62.commit()
[1421407,239]  -> wl_surface@62.frame(new id wl_callback@79)
[1421407,820] wl_display@1.delete_id(73)
[1421407,831] wl_surface@62.enter(wl_output@19)
[1421407,836] xdg_popup@74.configure(21, 1867, 207, 262)
[1421407,852] xdg_surface@75.configure(8693)
[1421407,860]  -> xdg_surface@75.ack_configure(8693)
[1421407,937]  -> wl_shm@4.create_pool(new id wl_shm_pool@73, fd 98, 216936)
[1421407,969]  -> wl_shm_pool@73.create_buffer(new id wl_buffer@51, 0, 207, 262, 828, 0)
[1421408,093]  -> wl_surface@62.attach(wl_buffer@51, 0, 0)
[1421408,102]  -> wl_surface@62.set_buffer_scale(1)
[1421408,106]  -> wl_surface@62.damage(0, 0, 207, 262)
[1421408,113]  -> xdg_surface@75.set_window_geometry(0, 0, 207, 262)
[1421408,134]  -> wl_surface@62.frame(new id wl_callback@77)
[1421408,140]  -> wl_surface@62.commit()
[1421408,433] wl_buffer@51.release()
[1421420,128] wl_display@1.delete_id(79)
[1421420,146] wl_display@1.delete_id(77)
[1421420,150] wl_callback@79.done(2139777)
[1421420,155] wl_callback@77.done(2139777)
[1421423,861]  -> wl_compositor@6.create_surface(new id wl_surface@77)
[1421423,893]  -> wl_subcompositor@37.get_subsurface(new id wl_subsurface@79, wl_surface@77, wl_surface@62)
[1421423,904]  -> wl_subsurface@79.set_position(0, 0)
[1421423,910]  -> wl_subsurface@79.set_desync()
[1421423,914]  -> wl_compositor@6.create_region(new id wl_region@59)
[1421423,921]  -> wl_surface@77.set_input_region(wl_region@59)
[1421423,927]  -> wl_region@59.destroy()
[1421423,932]  -> wl_surface@77.set_buffer_scale(1)
[1421423,937]  -> wl_surface@77.commit()
[1421423,952]  -> wl_surface@77.damage(0, 0, 207, 262)
[1421423,964]  -> wl_surface@77.frame(new id wl_callback@65)
[1421423,971]  -> wl_surface@77.set_buffer_scale(1)
[1421423,977]  -> wl_surface@77.attach(wl_buffer@54, 0, 0)
[1421423,986]  -> wl_surface@77.commit()
[1421424,366] wl_display@1.delete_id(59)
[1421426,266] wl_buffer@54.release()
[1421437,092] wl_display@1.delete_id(65)
[1421438,672] wl_callback@65.done(2139794)
[1421438,697]  -> wl_surface@77.damage(7, 239, 16, 17)
[1421438,713]  -> wl_surface@77.frame(new id wl_callback@65)
[1421438,732]  -> wl_surface@77.attach(wl_buffer@54, 0, 0)
[1421438,747]  -> wl_surface@77.commit()
[1421442,817] wl_buffer@54.release()
[1421453,943] wl_display@1.delete_id(65)
[1421455,157] wl_callback@65.done(2139811)
[1421492,039] wl_pointer@3.button(8694, 2139849, 273, 0)
[1421492,131] wl_pointer@3.frame()
[1422492,169] wl_pointer@3.leave(8695, wl_surface@28)
[1422492,249]  -> wl_pointer@3.set_cursor(8686, wl_surface@15, 1, 1)
[1422492,313]  -> wl_surface@15.attach(wl_buffer@52, 0, 0)
[1422492,357]  -> wl_surface@15.set_buffer_scale(1)
[1422492,380]  -> wl_surface@15.damage(0, 0, 10, 16)
[1422492,443]  -> wl_surface@15.commit()
[1422492,466] wl_pointer@3.frame()
[1422508,868] wl_surface@15.enter(wl_output@19)
[1422508,930]  -> wl_pointer@3.set_cursor(8686, wl_surface@15, 1, 1)
[1422508,983]  -> wl_surface@15.attach(wl_buffer@52, 0, 0)
[1422509,037]  -> wl_surface@15.set_buffer_scale(1)
[1422509,057]  -> wl_surface@15.damage(0, 0, 10, 16)
[1422509,103]  -> wl_surface@15.commit()
[1422509,124] wl_pointer@3.enter(8696, wl_surface@28, 19,292969, 2126,281250)
[1422509,193]  -> wl_pointer@3.set_cursor(8696, wl_surface@15, 1, 1)
[1422509,244]  -> wl_surface@15.attach(wl_buffer@52, 0, 0)
[1422509,292]  -> wl_surface@15.set_buffer_scale(1)
[1422509,313]  -> wl_surface@15.damage(0, 0, 10, 16)
[1422509,367]  -> wl_surface@15.commit()
[1422509,390] wl_pointer@3.frame()
[1422509,431] wl_pointer@3.motion(2140865, 18,675781, 2126,281250)
[1422509,492] wl_pointer@3.frame()
[1423509,309] wl_pointer@3.leave(8697, wl_surface@28)
[1423509,394]  -> wl_pointer@3.set_cursor(8696, wl_surface@15, 1, 1)
[1423509,460]  -> wl_surface@15.attach(wl_buffer@52, 0, 0)
[1423509,511]  -> wl_surface@15.set_buffer_scale(1)
[1423509,537]  -> wl_surface@15.damage(0, 0, 10, 16)
[1423509,586]  -> wl_surface@15.commit()
[1423509,609] wl_pointer@3.frame()
[1423821,194] wl_surface@15.enter(wl_output@19)
[1423821,257]  -> wl_pointer@3.set_cursor(8696, wl_surface@15, 1, 1)
[1423821,312]  -> wl_surface@15.attach(wl_buffer@52, 0, 0)
[1423821,364]  -> wl_surface@15.set_buffer_scale(1)
[1423821,388]  -> wl_surface@15.damage(0, 0, 10, 16)
[1423821,441]  -> wl_surface@15.commit()
[1423821,459] wl_pointer@3.enter(8698, wl_surface@28, 18,675781, 2126,281250)
[1423821,533]  -> wl_pointer@3.set_cursor(8698, wl_surface@15, 1, 1)
[1423821,608]  -> wl_surface@15.attach(wl_buffer@52, 0, 0)
[1423821,661]  -> wl_surface@15.set_buffer_scale(1)
[1423821,686]  -> wl_surface@15.damage(0, 0, 10, 16)
[1423821,790]  -> wl_surface@15.commit()
[1423821,809] wl_pointer@3.frame()
[1423821,858] wl_pointer@3.button(8699, 2142176, 273, 1)
[1423821,918] wl_pointer@3.frame()
[1423825,517]  -> xdg_popup@74.destroy()
[1423825,559]  -> xdg_surface@75.destroy()
[1423825,574]  -> wl_surface@62.destroy()
[1423825,595]  -> wl_buffer@51.destroy()
[1423825,612]  -> wl_shm_pool@73.destroy()
[1423825,713]  -> wl_subsurface@79.destroy()
[1423825,753]  -> wl_surface@77.destroy()
[1423827,984] wl_display@1.delete_id(74)
[1423828,003] wl_display@1.delete_id(75)
[1423828,024] wl_display@1.delete_id(62)
[1423828,029] wl_display@1.delete_id(51)
[1423828,052] wl_display@1.delete_id(73)
[1423828,056] wl_display@1.delete_id(79)
[1423828,060] wl_display@1.delete_id(77)
[1423915,691] wl_pointer@3.button(8700, 2142273, 273, 0)
[1423915,875] wl_pointer@3.frame()
[1424916,256] wl_pointer@3.leave(8701, wl_surface@28)
[1424916,332]  -> wl_pointer@3.set_cursor(8698, wl_surface@15, 1, 1)
[1424916,396]  -> wl_surface@15.attach(wl_buffer@52, 0, 0)
[1424916,448]  -> wl_surface@15.set_buffer_scale(1)
[1424916,474]  -> wl_surface@15.damage(0, 0, 10, 16)
[1424916,526]  -> wl_surface@15.commit()
[1424916,548] wl_pointer@3.frame()
[1425245,292] wl_surface@15.enter(wl_output@19)
[1425245,360]  -> wl_pointer@3.set_cursor(8698, wl_surface@15, 1, 1)
[1425245,419]  -> wl_surface@15.attach(wl_buffer@52, 0, 0)
[1425245,467]  -> wl_surface@15.set_buffer_scale(1)
[1425245,493]  -> wl_surface@15.damage(0, 0, 10, 16)
[1425245,537]  -> wl_surface@15.commit()
[1425245,556] wl_pointer@3.enter(8702, wl_surface@28, 18,675781, 2126,281250)
[1425245,641]  -> wl_pointer@3.set_cursor(8702, wl_surface@15, 1, 1)
[1425245,706]  -> wl_surface@15.attach(wl_buffer@52, 0, 0)
[1425245,788]  -> wl_surface@15.set_buffer_scale(1)
[1425245,805]  -> wl_surface@15.damage(0, 0, 10, 16)
[1425245,841]  -> wl_surface@15.commit()
[1425245,860] wl_pointer@3.frame()
[1425245,910] wl_pointer@3.motion(2143601, 18,046875, 2126,281250)

[emersion]

Can you update the Firefox issue to make it clear it's sway specific and likely a sway bug?

The WAYLAND_DEBUG trace looks good to me. I believe this is a damage issue.

[kennylevinsen]

Added link to this issue from the Firefox bug report, and mentioned that it only seems to reproduce in Sway out of the 4 compositors I tested.

[kennylevinsen]

The post-bug surface creation is very busy, but so far the only difference I have been able to isolate is that the new code utilizes xdg_popup.

Snippet:

[3312253.900] wl_display@1.delete_id(56)
[3312321.644] wl_pointer@3.button(18, 9792548, 272, 0)
[3312321.760] wl_pointer@3.frame()
[3312642.210] wl_pointer@3.button(19, 9792869, 273, 1)
[3312642.306] wl_pointer@3.frame()
[3312660.750]  -> wl_compositor@6.create_surface(new id wl_surface@60)
[3312660.829]  -> xdg_wm_base@18.create_positioner(new id xdg_positioner@55)
[3312660.849]  -> xdg_positioner@55.set_size(207, 309)
[3312660.884]  -> xdg_positioner@55.set_anchor_rect(542, 1085, 207, 309)
[3312660.911]  -> xdg_positioner@55.set_offset(0, 0)
[3312660.931]  -> xdg_positioner@55.set_anchor(5)
[3312660.944]  -> xdg_positioner@55.set_gravity(8)
[3312660.954]  -> xdg_positioner@55.set_constraint_adjustment(63)
[3312660.984]  -> xdg_wm_base@18.get_xdg_surface(new id xdg_surface@70, wl_surface@60)
[3312661.002]  -> xdg_surface@70.get_popup(new id xdg_popup@59, xdg_surface@32, xdg_positioner@55)
[3312661.069]  -> xdg_positioner@55.destroy()
[3312661.096]  -> wl_surface@60.commit()
[3312662.061]  -> wl_surface@60.frame(new id wl_callback@57)
[3312662.871] wl_display@1.delete_id(55)
[3312662.906] wl_surface@60.enter(wl_output@17)
[3312662.917] xdg_popup@59.configure(542, 1085, 207, 309)
[3312662.950] xdg_surface@70.configure(20)
[3312662.990]  -> xdg_surface@70.ack_configure(20)
[3312663.121]  -> wl_shm@4.create_pool(new id wl_shm_pool@55, fd 97, 255852)
[3312663.192]  -> wl_shm_pool@55.create_buffer(new id wl_buffer@68, 0, 207, 309, 828, 0)
[3312663.519]  -> wl_surface@60.attach(wl_buffer@68, 0, 0)
[3312663.558]  -> wl_surface@60.set_buffer_scale(1)
[3312663.569]  -> wl_surface@60.damage(0, 0, 207, 309)
[3312663.594]  -> xdg_surface@70.set_window_geometry(0, 0, 207, 309)
[3312663.671]  -> wl_surface@60.frame(new id wl_callback@69)
[3312663.683]  -> wl_surface@60.commit()
[3312664.033] wl_buffer@68.release()
[3312664.443] wl_buffer@49.release()

[emersion]

but so far the only difference I have been able to isolate is that the new code utilizes xdg_popup.

What do you mean?

My bet would be that this is not a protocol issue, but just a sway internal rendering issue.

[kennylevinsen]

but so far the only difference I have been able to isolate is that the new code utilizes xdg_popup.

What do you mean?

My bet would be that this is not a protocol issue, but just a sway internal rendering issue.

I also believe that to be the case. Due to not being very familiar with sway/wlroots codebases, I took a quite generic debugging approach: Attempting to trace sway/wlroots behavior between the working and non-working Firefox revisions to locate what difference there may be between the working and non-working surfaces.

The commit/damage part of the two revisions appeared fairly identical to me (including some fprintf debug in sway and wlroots), so my train of thought made me assume that whatever difference there may be, including damage issues on render, would originate from surface configuration. That the broken revision utilizes xdg_popup surfaces could be such a difference.

Still digging around, though.

[kennylevinsen]

The problem is subsurfaces of popups.

Firefox creates an xdg_popup with an xdg_positioner whose anchor_rect is set to the desired position. The result of this is a view child in sway, whose root coords is correctly calculated by popup_get_root_coords, the get_root_coords implementation assigned to the view child.

Then, firefox creates a subsurface on this xdg_popup through view_child_handle_surface_new_subsurface. This subsurface does not end up having a handle to the view child of the parent, and utilizes its own get_root_coords implementation which takes its own view child, and any parent subsurfaces into account.

The result is that the subsurface does not take surface->popup->geometry of the parent into account, which is the geometry containing the popup offset, resulting in a 0,0 root for the subsurface.

We need to get a hold of get_root_coords of the parent view child. I made a quick test where a view child can optionally store a pointer to its parent, so that subsurface_get_root_coords can call the parents' root coords implementation, and that did indeed solve the issue. It also segfaults like crazy, likely due to the parent being destroyed before a call to subsurface_get_root_coords, but it was a test.