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

Add module "sway/hide" for swaybar integration #860

Closed
wants to merge 12 commits into from

Conversation

somini
Copy link

@somini somini commented Sep 21, 2020

Rebases and continues work by @xPMo in https://github.com/xPMo/Waybar/tree/sway-hide

This allows auto-showing the bar when the modifier is pressed

Closes #255

Setup instructions:

  • Set the mode of the bar to "hide" in sway configuration file
  • Add "sway/hide" into modules-left in Waybar configuration file

}

void Hide::onEvent(const struct Ipc::ipc_response& res) {
auto payload = parser_.parse(res.payload);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a mutex here, might fix your crash

Copy link
Author

@somini somini Sep 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My C++ knowledge is almost 0, I just copy pasted some bits from other places that looked correct. 😞

2253f3e crashes less, but it still fails. Logs:

**
Gtk:ERROR:../gtk/gtk/gtkcssinheritvalue.c:33:gtk_css_value_inherit_free: code should not be reached
**
Gtk:ERROR:../gtk/gtk/gtkcssinheritvalue.c:33:gtk_css_value_inherit_free: code should not be reached
Bail out! Gtk:ERROR:../gtk/gtk/gtkcssinheritvalue.c:33:gtk_css_value_inherit_free: code should not be reached
Bail out! Gtk:ERROR:../gtk/gtk/gtkcssinheritvalue.c:33:gtk_css_value_inherit_free: code should not be reached
Aborted (core dumped)

Sounds like hiding a painting bar is a no-no.

I think the mutex should be on the setVisible module for the bar itself, but waiting for the first paint. Is there some kind of event queue where we can add the hide/show events, instead of being done ASAP?

Copy link
Author

@somini somini Sep 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased this to the latest master and force-pushed

nyyManni and others added 10 commits September 21, 2020 23:07
Configuration option `layer` can now take a value "overlay", which draws the bar
on top of the windows without creating an exclusive zone.
This allows auto-showing the bar when the modifier is pressed

Closes Alexays#255

Setup instructions:
- Set the `mode` of the bar to "hide" in sway configuration file
- Set the `layer` of the bar to "overlay" in Waybar configuration file
- Add "sway/hide" into `modules-left` in Waybar configuration file
Currently, it won't build.
I am unfamiliar with C++, I don't know what's wrong.
This is also a rebase-fix
Call this when toggling visibility with SIGUSR1, and for `sway/hide`
events.
This doesn't solve the issue, but it crashes less often...
Keep this as similar to "upstream" as possible.
Copy link
Contributor

@alebastr alebastr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can try to rework waybar::Bar in order to provide methods to apply certain parts of the bar_config object, but that'll have to wait until the weekend.

Switching to Gio::Socket in the main loop might resolve all threading issues. But that's a lot of refactoring for sway ipc code...

src/modules/sway/hide.cpp Outdated Show resolved Hide resolved
@@ -22,6 +22,9 @@ waybar::AModule* waybar::Factory::makeModule(const std::string& name) const {
if (ref == "sway/window") {
return new waybar::modules::sway::Window(id, bar_, config_[name]);
}
if (ref == "sway/hide") {
return new waybar::modules::sway::Hide(id, bar_, config_[name]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should that be a module?
Maybe we can implement it as a generic SwayBarConfigHandler class that is created by the waybar::Client instance when the SWAYSOCK is defined and controls all the bars.
The benefit is that on multi-monitor configurations bar events would be handled once.

src/bar.cpp Outdated Show resolved Hide resolved

void Hide::onEvent(const struct Ipc::ipc_response& res) {
auto payload = parser_.parse(res.payload);
if (payload.isMember("mode")) {
Copy link
Contributor

@alebastr alebastr Sep 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that each waybar instance supposed to have bar id (waybar -b bar-0) and events from sway could be addressed to the specific bar or to all bars.
Somewhere around here you should check if the current waybar instance needs to handle the event.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to use just call setVisible on the bar_ object, but I got an error about mismatching const which is above my meagre C++-fu.

This is a CSS-only change. It should be properly hidden.
@somini
Copy link
Author

somini commented Sep 21, 2020

@alebastr Thanks for the review. From the GTK logs, a mutex on the setVisible method on the bar itself might work, as long as we can ignore event that appear during the transitions (i.e. when the mutex is locked). Sort of like a rate-limiter. It's better to ignore events than crashing.

I don't know enough C++ to do much more than this, but I'm available for testing.

@somini
Copy link
Author

somini commented Sep 19, 2021

Continued in #1241

@somini somini closed this Sep 19, 2021
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.

Only show waybar when modifier key is pressed
5 participants