Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use gtk-layer-shell library for correct positioning of popups #441

Merged
merged 3 commits into from Dec 28, 2019

Conversation

alebastr
Copy link
Contributor

@alebastr alebastr commented Aug 28, 2019

Initial attempt to address #63 - positioning of menus and tooltips with wmww/gtk-layer-shell library.

While the approach of the library is simple — subscribe to 'realize' for all windows and patch surfaces for anything that looks like popup, there's too much magic happening inside so I'm not even going to try adapting the code directly to waybar.

How to test:

  • install sway 1.2 + wlroots 0.7.0
  • install gtk-layer-shell 0.1.0 (optional; meson will fetch it from github if not present)
  • checkout git branch from this pull request
  • configure waybar with -Dgtk-layer-shell=enabled (disabled by default)

What works:

  • menus are opened at mouse pointer position
  • tooltips are not grabbing focus and making waybar go insane
  • outputs are detected and new bars created for outputs
  • menus and submenus are displayed on correct output

What does not:

  • sway < 1.2
    Limitation of wlr-layer-shell protocol: if the compositor has no support for layer-shell popups, wlr-layer-shell.get_popup will make xdg_popup silently disappear, as it does not provide any results or errors.
  • keyboard focus on menu. If I understand discussions in wlr-protocols correctly, it's expected to work and the possible reason of the issue is Popups do not grab wmww/gtk-layer-shell#9
  • closing popup by clicking outside of it: this works only if you click on waybar ¯\(°_o)/¯. Again, that is caused by missing xdg_popup_grab call and should be fixed elsewhere.
  • specifying width and height in configuration: the bar will ignore these and resize to screen width/height
  • popups are rendered on the same layer as the main window, i.e. underneath other windows if the bar layer is set to bottom. This is a sway bug and it's already fixed in master (sway release 1.3)

@alebastr alebastr force-pushed the gtk-layer-shell branch 2 times, most recently from 61f6002 to 0620ca3 Compare September 1, 2019 06:44
@Alexays
Copy link
Owner

Alexays commented Sep 2, 2019

How are the outputs destroyed now?

@alebastr
Copy link
Contributor Author

alebastr commented Sep 3, 2019

std::remove_if in the end of Client::handleMonitorRemoved erases struct waybar_output from outputs_.
Implicitly defined destructor of struct waybar_output then releases reference to Gdk::Monitor and destroys xdg_output. As wl_output is owned by Gdk::Monitor, nothing here requires writing explicit destruction code.

@David96
Copy link
Contributor

David96 commented Oct 14, 2019

What would be the requirements for this to get merged? Is there any way to help?
I tried it out and so far it appears to be working (with the limitations already mentioned).

Only one thing that did bother me, although I'm not quite sure what the "right" behavior is: The popups use the same layer is the bar. Therefore having the bar layer set to bottom will have the popups show underneath the windows. Setting the layer to top, makes bemenu, a wayland dmenu alternative, (its layer is set to top, too) show underneath waybar. Not sure, whether bemenu should use overlay as layer. https://drewdevault.com/2018/07/29/Wayland-shells.html says that panels should use bottom as a layer though so it might be desireable to use overlay as the popup layer independent of what layer waybar is rendered at? Not sure whether this is possible.

@alebastr
Copy link
Contributor Author

On hold due to me being busy with work and other things. Also because I'm not happy with layer-shell limitations and GTK integration issues.
I can do a cosmetic run over the last commits and set it as ready to merge by end of week.

Regarding the popup layer: I was confused by this behavior as well. You may want to repost the question to swaywm/wlr-protocols, a proper place for discussions about layer-shell protocol.

@David96
Copy link
Contributor

David96 commented Oct 16, 2019

Alright, thanks for your response :)

@danyspin97
Copy link
Contributor

I am using gtk-layer-shell to test if it fixed #487. Unfortunately, I still get crashes when I disable outputs. Here it is the backtrace:

#0  0x00007ffff72a58a9 in send_delete_event () at /usr/x86_64-pc-linux-musl/lib/libgtk-3.so.0
#1  0x00007ffff6e56b48 in gdk_threads_dispatch () at /usr/x86_64-pc-linux-musl/lib/libgdk-3.so.0
#2  0x00007ffff6b0b747 in g_main_dispatch (context=0x5555556c1b40) at ../glib-2.60.0/glib/gmain.c:3189
#3  0x00007ffff6b0b747 in g_main_context_dispatch (context=context@entry=0x5555556c1b40) at ../glib-2.60.0/glib/gmain.c:3854
#4  0x00007ffff6b0bb28 in g_main_context_iterate (context=context@entry=0x5555556c1b40, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib-2.60.0/glib/gmain.c:3927
#5  0x00007ffff6b0bbac in g_main_context_iteration (context=context@entry=0x5555556c1b40, may_block=may_block@entry=1) at ../glib-2.60.0/glib/gmain.c:3988
#6  0x00007ffff6d2ea6d in g_application_run (application=0x5555556a91a0 [gtkmm__GtkApplication], argc=<optimized out>, argv=0x7fffffffe608) at ../glib-2.60.0/gio/gapplication.c:2516
#7  0x0000555555582ae2 in waybar::Client::main(int, char**) ()
#8  0x0000555555581bbc in main ()

It might not be related to Waybar though, I'll try to find the issue in the next days.

@eternal-sorrow
Copy link

On hold due to me being busy with work and other things.

So, are you still busy? Maybe someone else could continue the work on this? It's the most important improvement for waybar right now in my opinion. It also would fix a lot of bugs that are already known.

gtk-layer-shell wants Gdk::Monitor instead of wl_output;
change code to deal with Gdk objects and slightly simplify it.
Requires gtkmm 3.22.0+ (first release with Gdk::Monitor support).
To enable: use sway >= 1.2, compile waybar with `-Dgtk-layer-shell=enabled` meson option.
Original behavior could be restored at runtime by setting `"gtk-layer-shell": false` in waybar config.
@alebastr alebastr marked this pull request as ready for review December 28, 2019 00:56
@alebastr
Copy link
Contributor Author

I apologize for the delays. This PR is seriously cursed and various things kept happening whenever I wanted to get back to the code and finish it.

Changed in last revision:

  • use released gtk-layer-shell 0.1.0 instead of git snapshot
  • remove ipc asan fix. This rabbit hole is too deep and deserves a separate PR with larger scope of changes. Like completely replacing sigc++ signals from ipc code with something that actually safe to use across threads.
  • minor changes in the bar.cpp

@Alexays
Copy link
Owner

Alexays commented Dec 28, 2019

Thanks a lot @alebastr and yeah we need to replace sigc++ that is garbage with threads...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants