Project

General

Profile

Bug #11535

various UI failures when running under Wayland

Added by Dan Torop over 2 years ago. Updated about 2 years ago.

Status:
Triaged
Priority:
Low
Assignee:
-
Category:
General
Target version:
-
Start date:
03/09/2017
Due date:
% Done:

20%

Affected Version:
git master branch
System:
Debian
bitness:
64-bit
hardware architecture:
amd64/x86

Description

There are various UI failures when running darktable with Wayland as the GDK backend. For now, PR 1453 hides this by forcing darktable to prefer GDK's X11 backend. But it appears that at least some of the UI failures are due to improper code in darktable which Wayland, being stricter, exposes.

Failures include:

  1. Many menus appear extremely narrow. This is due to some extant code calling gtk_widget_show_all() after gtk_menu_popup_at_widget. I've put up a patch for this in https://github.com/dtorop/darktable/commits/wayland-fixup.
  2. Buttons at bottom of lighttable (display profile) and darkroom (raw overexposed, over/underexposed, softproof, gamut check) do not work. They resulting popups cannot be closed, the comboboxes do not function, and the rest of the UI becomes unresponsive. Furthermore, the popups appear in the top left of the screen. This seems to be due to a few problems, including that Wayland reports window positions as (0,0) and doesn't seem to allow giving them an absolute position, plus that the bauhaus widgets are failing to be on top of the transient window containing the popup. The answer may be rewriting the code to make popups closer to bauhaus widgets: GTK_WINDOW_POPUP rather than GTK_WINDOW_TOPLEVEL. It'll take some more juggling to be sure the bauhaus widgets sit on top of the popup.
  3. Scrolling on the image with the trackpad in lightroom and and darkroom modes doesn't work. This may be the same as Bug #11041.
  4. At times in darkroom mode keyboard input will be ignored. Perhaps some widget grabs the keyboard and doesn't release it?

Hajo Schatz and others have pointed out some of these problems, and Hajo Schatz provided the workaround of forcing XWayland as the GDK backend.

I'll look a bit more into this. If addressing any of this results in legitimate improvements to darktable's code, and allows for not forcing use of XWayland, this seems worth attention.

gtk-popover-profile.png (35.4 KB) Dan Torop, 03/14/2017 04:11 PM

Associated revisions

Revision 2e600fd3
Added by Dan Torop over 2 years ago

flag menus for display before displaying them

This eliminates the "narrow menus" bug in Wayland (see #11535), though
there continue to be other Wayland incompatibilities. It also seems to
be best practice for GTK, and was already occurring in some of the
code.

Also consistently cast via GTK_WIDGET().

Revision a48cce49
Added by Dan Torop over 2 years ago

lighttable: make profile floating window Wayland compatible

This makes lighttable's profile/intent chooser floating window work
under both the X11 and Wayland backends for GDK. Prior to this, under
Wayland, the floating window would not appear in the proper
position (it tended to appear in the top left of the screen), the
combobox controls on it did not function, and it would be impossible
to close the floating window or interact further with the application.

The problem is that the floating window was a GDK_WINDOW_TOPLEVEL, and
under Wayland, at least via its current GTK/GDK interface,
applications are given no knowledge or control of the positions of top
level windows (see https://wiki.gnome.org/Initiatives/Wayland/GTK%2B
"Window positioning"). Making the window a GDK_WINDOW_POPUP (which
allows positioning it under Wayland) has its own problems, as I could
not figure out a good way to tell when it loses focus via the user
clicking outside of it, and without some more hacks the bauhaus
combobox control appeared behind the floating window.

Instead, this commit uses a built-in GTK widget, GtkPopover, to
display the profile floating window. GtkPopovers are drawn with a tail
pointing to the widget which they are relative to, and are drawn
centered over that widget. This commit uses a couple hacks to get
around this and match the prior floating window UI: it suppresses the
drawing of the GtkPopover while still propagating draw events to its
children, and it forces an offset of the popover such that it is NW of
profile button. The forcing of the offset may not be reliable,
though. It would make this commit simpler to just center the popover
over the profile button.

GtkPopovers close themselves when the escape key is pressed, which is
a change in behavior from the prior floating window implementation.

Unlike the existing floating window implementation, GtkPopovers
default to animating their appearance/disappearance. The
"transitions-enabled" property which gets rid of this only exists in
GTK+ >= 3.16, and currently darktable supports >= 3.14. Hence for GTK+
< 3.16, the popover will have a brief animation when it
closes. Because it is shown via gtk_widget_show_all(), there is no
transition when it opens.

It does not seem possible to disable GtkPopover transitions via the
CSS transition property, and overriding the close transition on
popdown would require messing with a bunch of event handling code in
GtkPopover.

See https://bugzilla.redhat.com/show_bug.cgi?id=1264355 for a
discussion of the difficulties of popup windows in Wayland.

It may turn out that it is better to cannibalize the GtkPopover
code (which directly creates a GDK window to do its work) or the
example code in the above mentioned bug report rather than hacking
GtkPopover. In that case, as floating windows are also used for the
toolbar at the bottom of the darkroom mode, perhaps it makes sense to
make a floating window widget in src/dtgtk.

See #11535.

Revision 5e8ac3d4
Added by Dan Torop over 2 years ago

darkroom: use GtkPopover for toolbar buttons

As per lighttable view. Simplifies code and adds Wayland
compatibility compared to homebrew floating window. Does change UI
look, though, by adding a "tail".

Only wrinkle is to account for the same popover being attached to both
profile buttons.

Also, don't get panel_width from conf multiple times.

See #11535.

Revision 458f4edf
Added by Dan Torop over 2 years ago

bauhaus: make keyboard grab work under Wayland

Prior to this commit, once a bauhaus popup appeared under Wayland,
darktable would not be responsive to the keyboard until the
application quit.

See #11535.

This commit also fixes a bug when bauhaus comboboxes were used within
a GtkPopover presented from a toolbar. If one opened a GtkPopover from
the toolbar, then opened a bauhaus combobox, then moved the mouse
slightly outside of the combobox and clicked, the popover would
disappear but the combobox would remain until one moved the mouse
further away from the combobox.

Prior to the use of GtkPopover, on X11 the behavior was different (and
buggy): clicks outside of the combobox would have no effect. And in
this case one would have to repopen the combobox via the toolbar
button for dt to once again note mouse events (and allow for closing
of the floating window).

This commit uses a GTK rather than GDK grab. Also don't worry about
just grabbing the keyboard, there is no harm in grabbing the pointer
as well, and the code is simpler.

In the prior code, to detect when to close the bauhaus popup,
motion-notify-event and button-press-event over the root window were
always monitored. This code only checks for these events over the
popup window, which means that these events will only fire when the
popup is visible (which should make things slightly more efficient).

In the prior code, "prelight" events would occur in the root window
when the mouse moved out of the popover but it wasn't far enough to
close the popover. These will no longer occur.

I have mixed feelings about this patch, as it fixes problems, but I
don't understand why the GDK grabs fail for Wayland, nor why the X11
behavior was subtly broken. I've experimented a bit with no
results. It is clear that the GdkSeat interface was built to aid the
transition to Wayland. See
https://bugzilla.gnome.org/show_bug.cgi?id=759309 and the "Popups and
Grabs" section of https://wiki.gnome.org/Initiatives/Wayland/GTK%2B.

Someone with a more sympathetic relationship to GTK/GDK than I perhaps
could offer another route here? I'm not sure if there's some magic
invocation to make grabbing work across backends, or if as GTK learns
to work with Wayland that it just gets more and more uncomfortable
with popup windows which aren't based upon extant GTK widgets.

See https://bugzilla.redhat.com/show_bug.cgi?id=1264355 for some
discussion of the difficulties of porting pop-up windows to Wayland.

This patch is tested under X11 and Wayland/Xwayland, but not under
Quartz.

History

#1 Updated by Tobias Ellinghaus over 2 years ago

  • Status changed from New to Triaged
  • % Done changed from 0 to 20

#2 Updated by Dan Torop over 2 years ago

One more Wayland display problem: the popup menu icon on top of lightroom modules and darkroom iops retains the highlit mouse-over state even once the menu is closed. This is also true for new/duplicate instance etc. button on darkroom iops.

#3 Updated by Roman Lebedev over 2 years ago

Dan Torop wrote:

the popup menu icon on top of lightroom modules

uhm :)

#4 Updated by Dan Torop over 2 years ago

I'd be curious for thoughts on https://github.com/dtorop/darktable/tree/wayland-fixup-popover, which is an attempt to fix problem #2 above. As the very long commit message explains, I'm using a GtkPopover instead of a GTK_WINDOW_TOPLEVEL. This means that GTK does a bunch of the work and simplifies the code, but to reproduce the appearance of the original floating window requires a few contortions. Broad choices would be:

  1. Go with the GtkPopover.
  2. Cannibalize the best parts of GtkPopover to make a floating window implementation that works under Wayland and X11.
  3. Scratch Wayland support.

Thoughts/advice most welcome!

#5 Updated by Tobias Ellinghaus over 2 years ago

In general I have no problem with the popups being placed differently or having tails pointing to the button. But the lack of interaction from keypresses is not good. So while option 1 sounds nice and simple I guess we have to dig into adding our own widget.

#6 Updated by Dan Torop over 2 years ago

I think the lack of interaction from keypresses is something different -- a bauhaus incompatibility with Wayland, perhaps something about keyboard grabs. Keypresses work fine under X11. Certainly the implementation using GtkPopover is pleasingly simple. I pushed out another change to https://github.com/dtorop/darktable/tree/wayland-fixup-popover which uses more GtkPopover default behaviors (with a tail, centered over the profile button. A screenshot attached of how it looks.

This is probably the most robust implementation, but it does look a bit Gnome/GTK-ish.

I'll look into the keypress problem. If I can get that sorted out, perhaps GtkPopover is viable. Otherwise, yes, adding a widget (to src/dtgtk?) I guess....

#7 Updated by Dan Torop over 2 years ago

Recent commits to https://github.com/dtorop/darktable/tree/wayland-fixup-popover enable Wayland trackpad scrolling. Wayland sends GDK_SCROLL_SMOOTH events (rather than GDK_SCROLL_UP/GDK_SCROLL_DOWN) when using a trackpad. It turns out that these events have been around since GTK+ 3.4, but they're only now useful to parse. I still need to fix scrolling in more cases (e.g. in main view of lighttable/darkroom and on bauhaus sliders).

I still haven't looked into why under Wayland darktable loses keyboard events once a bauhaus control opens, but will.

The use of GtkPopover instead of a hand-rolled widget is growing on me. It eliminates a bunch of code, and it becomes more darktable-ish once it gets a CSS style. If the centered placement and the tail indeed aren't offensive, it could work. It also seems that GtkWindow has a bunch of internal-only code to help with popovers, which a hand-rolled floating window wouldn't have the use of.

One question is what is the oldest version of GTK which needs to be supported? If pre-3.4, then I'd need to guard some scrolling code in ifdef's. GtkPopover has been around since 3.12, with the ability to suppress transitions appearing in 3.16. It's easy to guard the transition-disabling code in ifdefs, but an extra wrinkle if pre-3.12 GTK needs support.

#8 Updated by Roman Lebedev over 2 years ago

Dan Torop wrote:

One question is what is the oldest version of GTK which needs to be supported?

As of right now - 3.14
https://github.com/darktable-org/darktable/blob/c510ef7953e905f9142fbcc16cbd64a0f053dc58/src/CMakeLists.txt#L219

I suppose soon it can be bumped even higher. (once debian stable is released)

#9 Updated by Dan Torop over 2 years ago

As of right now - 3.14

Thank you! I should have found that... It should be no problem to make everything >= 3.14 safe.

#10 Updated by Dan Torop over 2 years ago

Still working, but committed code to wayland-fixup-popover branch which should make scrolling work in bauhaus for both comboboxes and sliders. It turns out that there are numerous other bits which will need fixes for scrolling: gui/gtk.c, atrous, colorzones, zonesystem, colorcorrection, lowlight, monochrome, levels, tonecurve, basecurve, equalizer, histogram, filter, and filmstrip.

It also turns out that once GDK_SMOOTH_SCROLL_MASK is enabled, GTK will generate only GDK_SCROLL_SMOOTH events, not GDK_SCROLL_UP/DOWN/LEFT/RIGHT, whether under X11 or Wayland. This isn't bad, the former should produce equivalent (or better) information. For now I'll leave in the old scroll handling code, but if it's possible to verify that smooth-scroll-only works back to GTK 3.14 (it does, according to the documentation...), then it could be worth simplifying things by removing GDK_SCROLL_MASK and the code to catch GDK_SCROLL_UP/DOWN/LEFT/RIGHT.

Still haven't dealt with why Wayland loses keypresses once a bauhaus popup appears.

#11 Updated by Dan Torop over 2 years ago

Nearing a decent set of fixes for scrolling under Wayland. Once that's done, I'll make a PR just for this, as it's self-contained and should improve trackpad interaction whether using X11 or Wayland. I found and fixed a few (minor) bugs along the way, and I'm trying to make behavior and implementation consistent throughout.

Some scrolling still won't work: All the GtkComboBoxes don't know about smooth scrolling. It's possible to override this by hand for each. One could also argue that this is a GTK bug and should be fixed there.

#12 Updated by Dan Torop over 2 years ago

There's a fix for the keyboard grab problem at https://github.com/dtorop/darktable/tree/wayland-fixup-keyboard-grab. It simplifies some code, but it slightly changes behavior for both Wayland and X11. All events are grabbed via GTK, rather than just keyboard events via GDK. (It's possible to just grab keyboard events via GTK, but this doesn't seem useful, and adds code.)

If the mouse leaves the bauhaus popup but isn't far enough out for the popup to close, the code in git master would trigger hover/prelight events for widgets below the mouse. This is no longer so in my commit.

The code in git master watches the root window for button & motion events. My commit only checks for these events from the bauhaus popup, which should get rid of some unnecessary checking of events when the popup is closed.

I'm still not certain if this is the best approach, and won't make a PR until I sit on this for a bit, or if there's advice/thoughts it is great.

Most of the pieces are now in place for working well with Wayland. PR 1463 allows Wayland trackpad scroll events to work. PR 1456 fixes popup menu width. https://github.com/dtorop/darktable/tree/wayland-fixup-popover has code to fix the profile button at bottom of lighttable, and it should be easy to apply that to the darkroom view as well. The prelight behavior for icons is still broken for Wayland and I haven't looked into that.

#13 Updated by Dan Torop over 2 years ago

To cross-reference to other work on this, https://bugzilla.redhat.com/show_bug.cgi?id=1264355 has a helpful discussion between a Firefox developer and Wayland/GTK developers about handling popups on Wayland with GTK. This is relevant both to the buttons at the bottom of lighttable/darkroom and to bauhaus widgets. Olivier Fourdan gives some code to handle popups at mouse location with a grab [1] and general tooltip style popups with no device grabbing [2].

Some relevant quotes for Olivier Fourdan:

...for tooltips (which do not imply any sort of grab), gtk uses a private API gtk_window_set_use_subsurface() which is private and not usable in regular applications. [3]

Ideally, Firefox should not try to re-invent those widgets and should rely on gtk+ widgets, ie tooltips and popup menus are available in gtk and work on Wayland so Firefox should use those instead of trying to come up with its own. [4]

And from the documentation of gtk_window_new:

If you’re implementing something like a popup menu from scratch (which is a bad idea, just use #GtkMenu), you might use #GTK_WINDOW_POPUP. [5]

What I get from this is that applications moving to Wayland are encouraged to subclass GTK+ widgets rather than rely on hinting GtkWindows or dealing with the GDK level. For now things sort-of-work for bauhaus widgets as they are implemented, though I've found no way but to switch to GTK rather than GDK grabs to get keyboard events. I suppose it would be possible to recode bauhaus on top of GtkMenu or GtkComboBox, and it might make things more succinct and future-proof, but it also would be a great deal of work and would really commit to using GTK as the underlying widget system.

I'm not entirely clear why darktable sometimes uses bauhaus comboboxes but in other cases uses GtkComboBox. From looking at commit history, it looks like GtkComboBox was there from the start, and once a working bauhaus variant showed up, there was a bunch of work to port to the bauhaus combobox instead. Is the end goal to use bauhaus combobox widgets everywhere that GtkComboBox is now used?

I'm feeling better about using GtkPopover to handle the buttons at the bottom of lighttable/darkroom, as that seems to fit both within darkttable's UI and GTK's widget scheme.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1264355#c15
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1264355#c17
[3] https://bugzilla.redhat.com/show_bug.cgi?id=1264355#c4
[4] https://bugzilla.redhat.com/show_bug.cgi?id=1264355#c12
[5] https://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-new

#14 Updated by Dan Torop over 2 years ago

Another UI problem under Wayland, involving auto-repeated keys when in lighttable's preview mode.

To replicate:

  • start darktable with Wayland as the GDK backend (e.g. via export GDK_BACKEND=wayland && darktable)
  • in lighttable, choose a collection with more than a few images
  • click on the first image
  • hold down the Z key to enter preview mode
  • press right arrow once to move to the next image
  • wait, keeping the Z key held down

Expected behavior:

  • lighttable will hold on the preview of the second image in the collection

Actual behavior:

  • lighttable will, after a moment, rapidly move through images to the end of the collection

#15 Updated by Dan Torop over 2 years ago

An update on this bug:

  • PR 1456, merged into master, fixed narrow menus in Wayland
  • PR 1463, merged into master, should fix trackpad scrolling under Wayland
  • branch wayland-fixup-popover should fix toolbar popup windows in lighttable and darkroom modes
  • branch wayland-fixup-keyboard-grab fixes keyobard grabs in bauhaus widgets
  • I haven't yet looked into broken prelight behavior or keyboard repeat

The two branches mentioned are in https://github.com/dtorop/darktable, PR for each forthcoming once I test a bit more.

#16 Updated by Dan Torop over 2 years ago

The preview mode repeat problem mentioned in https://redmine.darktable.org/issues/11535#note-14 may actually be due to Wayland's GDK backend. I filed https://bugzilla.gnome.org/show_bug.cgi?id=781285. Compiling GTK with the patch https://bug781285.bugzilla-attachments.gnome.org/attachment.cgi?id=349836 fixes the problem in darktable.

#17 Updated by Dan Torop about 2 years ago

A further update:

- PR 1472, merged into master, fixes the toolbar popup window and keyboard grab
- the keyboard repeat problem should be fixed in GTK+ 3.22.16
- I still haven't yet looked into the broken prelight behavior

I think that's all that's pending to make Wayland work, unless other exciting bugs manifest.

It doesn't make sense to revert the preference for XWayland (PR 1453) until GTK+ >= 3.22.16 makes its way into distributions, and even then probably only after testing GTK_CHECK_VERSION(3,22,16).

It appears that Nvidia closed-source drivers won't work with Wayland until/if Wayland has EGLStreams support? I haven't been following this closely, but if so Wayland support may be a moot point for many users of Nvidia GPUs.

Also available in: Atom PDF