streetturtle / awesome-wm-widgets

Widgets for Awesome Window Manager
http://pavelmakhov.com/awesome-wm-widgets/
MIT License
2.15k stars 277 forks source link

cpu widget gets placed off screen sometimes #408

Open saevarb opened 1 year ago

saevarb commented 1 year ago

Problem

After starting or restarting awesome, and clicking the CPU widget, the popup gets placed off screen when using multiple monitors(or even when using a small xephyr window), but only in the beginning. Hiding it and then clicking again correctly respects screen boundaries.

Right after restarting and clicking widget

image

After hiding popup and clicking again

image

Yellow line indicates screen boundary.

Additional info

awm version

awesome v4.3-1606-g0e5fc4575-dirty (Too long)
 • Compiled against Lua 5.4.6 (running with 0.9.2)
 • API level: 4
 • D-Bus support: yes
 • xcb-errors support: no
 • execinfo support: yes
 • xcb-randr version: 1.6
 • LGI version: /usr/share/lua/5.4/lgi/version.lua
 • Transparency enabled: yes
 • Custom search paths: no

Also happens on stable awm.

Notes

I looked at the code and saw that it is using popup:move_next_to which claims to take care of this, and also found this issue comment which is related.

I'm pretty new to awm and lua, but I did play around with some awful.placements but the only thing that really worked was just placing it in the top right corner, and while that would be fine for me, it's not a general solution that works if people are using e.g. vertical bars or at the bottom.

Aire-One commented 1 year ago

Hello @saevarb,

I did some poking around to understand what's going on here.

[...] but only in the beginning. Hiding it and then clicking again correctly respects screen boundaries.

Basically, when you click on the widget, a timer is started to regularly "query" all the data and draw/update the widget displayed in the popup. The very first time the popup is visible, the timer has never run before, hence the widget is empty and Awesome tries to place a { width = 1, heigth = 1 } popup.

As a little experiment, if I change the following code

https://github.com/streetturtle/awesome-wm-widgets/blob/2889ef4d9ec17e9b5abd75918b7b18b798163482/cpu-widget/cpu-widget.lua#L141-L145

with a delayed call for the popup:move_next_to (0,5 so that the timer callback has enough time to complete the work once)

                            -- popup:move_next_to(mouse.current_widget_geometry)
                            gears.timer {
                                timeout = 0.5,
                                single_shot = true,
                                autostart = true,
                                callback = function()
                                    popup:move_next_to(mouse.current_widget_geometry)
                                end,
                            }
                            -- Restart the timer, when the popup becomes visible
                            -- Emit the signal to start the timer directly and not wait the timeout first
                            popup_timer:start()
                            popup_timer:emit_signal("timeout")

The popup is correctly place from the first call!

This is NOT the solution you are looking for, but at least it points out the real issue. This timer logic was introduced with PR https://github.com/streetturtle/awesome-wm-widgets/pull/246, so CC @atopion :)

I think the path forward is to separate the UI logic from the business logic. AKA the popup_timer:connect_signal('timeout' callback shouldn't be building the widget, but only updating the values. With this kind of separation, it's easier to have a "first draw" with default values to initialize the popup content.