awesomeWM / awesome

awesome window manager
https://awesomewm.org/
GNU General Public License v2.0
6.34k stars 597 forks source link

Minor widgets improvments #1149

Open Elv13 opened 7 years ago

Elv13 commented 7 years ago

Just to keep in mind some details discussed on IRC, I will do that eventually

Elv13 commented 7 years ago

Here's a cheap separator based powerline layout https://gist.github.com/Elv13/e04e2b0f1e0292eaf7702ff2e6197ddf

Obviously, it has all the issues and bugs I known it would have, but it is good enough for static layouts. I added an example about how to use it for the taglist. It kind of work, but can't be used with anything but transparent colors

slider10

For this to work properly, it will need a full blown layout implementation. Meh, another widget... But umm, it is a popular request and according to reddit /r/unixporn, that widget style has a dedicated fanbase. My config implementation derserve to be burned down to ashes for abuse of dark magic. So a new implementation has to be written.

A good implementation have to finally make use of the overlapping/out of area capability our widget system has but nothing uses. Then with some before_draw_child clip magic, it should be possible to implement a real powerline layout.

psychon commented 7 years ago

To highlight the issues that you know about, modify like this:

s.mywibox:setup{
    {
        widget = wibox.widget.base.make_widget(),
        forced_width= 0.5
    },
       [the other widgets here]
}

Result is: not_good Oh, the joy of anti-aliasing...

Elv13 commented 7 years ago

@psychon What is the best way to get a child widget area from before_draw_child? It seems to only provide the index

psychon commented 7 years ago

Here is my hack for a powerline layout (based on yours). It just calls self:layout() to get information about the children and then accessed implementation details. Not good... I guess I didn't expect that someone monkey-patched the fixed layout like this.

The way that it solves the "thin lines" problem is by painting the whole area in one go with an arrow on the right side and "just a box" on the left side. The order of drawing things is so that the proper result, well, results.

A slightly less ugly hack might be to change the API for the :draw, :before_draw_children, :before_draw_child etc APIs so that they also get the wibox.hierarchy as an argument...

Elv13 commented 7 years ago

I guess I didn't expect that someone monkey-patched the fixed layout like this.

\hidden Always expect @Elv13 to have monkey patched everything in all the possible ways

A slightly less ugly hack might be to change the API for the :draw, :before_draw_children, :before_draw_child etc APIs so that they also get the wibox.hierarchy as an argument...

I will, as your hack solve the thin line issue, but not the other two (and they are more important than the thin line). Separator based powerline is a dead end. I spent enough time on them for my config to have exposed their issues. The ugliest implementation used the Awesome 3.4 API capi.image.draw_line to draw them pixel by pixel. I also had a widget that used a Taylor series to draw its separator :). Beside that, I had a whole range of powerline implementation since 3.4. The most reliable one was the one that removed the clip in 3.5.*. Because awesome always repainted the widget in a very predictable way, it was "perfect".

psychon commented 7 years ago

but not the other two (and they are more important than the thin line).

What are those? "No support for transparent colors" should be one of them. What's the other one?

Hm... One could draw the background to an intermediate surface with operator SOURCE, similar to what my hack above does. That might still leave some artifacts where the two "bands of colors" touch in a powerline fashion. The intermediate surface could then be drawn normally to the target: https://gist.github.com/psychon/211ead3b0ff22e4cf773d9bd0993bb77 transparent

The most reliable one was the one that removed the clip in 3.5.*.

How exactly did that work? Did you have a spacer between the widgets and each widget drew part of the background there, or what? You can simulate something like this in current master by placing a widget outside of the current widget in :layout(). That way, the before/after children APIs get a larger clip and the widget that is placed this way also gets more space.

Elv13 commented 7 years ago

What are those?

see the second post bullet list

psychon commented 7 years ago

Ups, sorry.

  • separator based impl have painting issues with sub-pixel widgets (edit: see @psychon post below)

Just means that the background cannot be drawn piecewise. This can be worked around. :-)

  • separator based impl can't be used for the taglist because you can't control the separator color

I think your franken-powerline-fixed-layout solves this as well? Edit: Well, ok, but the taglist also adds its own separators. Is that the problem?

  • separator based impl need to watch insert/remove/swap and refresh all background widgets

self:connect_signal("widget::layout_changed", function() self:emit_signal("widget::redraw_needed") end) Edit: Well, this works with my version where the spacers don't draw anything themselves, but instead the layout uses before_draw_children to draw the powerline background.

  • shape API based impl work fine, but is limited to 2 colors and one direction

Ok.

  • my "radical" module implementation reset the clip and force a full widget repaint

The signal hack above also forces a full widget repaint. ;-)

psychon commented 7 years ago

Could you check if this implementation does everything that you want/need (except for supporting arrows in the opposite direction, but that should be easily doable for a proper implementation)? https://gist.github.com/psychon/903f276deb89feeec11f7a9b01f0678b But yeah, I guess a "proper powerline" needs to be a new layout. Edit: Here is how this code looks for me (after adding some more textboxes so that the transparency support is more obvious): transparent Edit: Here is a slightly better implementation including the already proposed API change (hierarchy as new argument to the draw methods): https://gist.github.com/psychon/42eba545ef78f03faec86688dcbd8838

Elv13 commented 7 years ago

Could you check if this implementation does everything that you want/need

This isn't for me, I already have everything I need ;)

But yeah, I guess a "proper powerline" needs to be a new layout.

Here's what I had in mind: https://gist.github.com/Elv13/b221c8402a1b6e1257e242923fe324e8 (note that this depends on the hierarchy changes discussed earlier)

slider10

See how the taglist is now "normal". The downside with this is the margin responsibility fall downstream. The final version of this layout would have its own built-in one using the spacing. It also need to implement both direction and the other shapes such as the parallelogram.

slider10

(from one of the other hacky Radical themes, not this code)

I guess it has input issues. Can you fork it and use the "out of area" fit feature? It isn't used anywhere yet, so your better placed than me to know it's internal.

Edit: Here is a version with most missing features https://gist.github.com/Elv13/b1c3af9a536aeaed57b57a3dc0730750, not as clean and simple as the last, but much more useful.

Edit 2: I created a [WIP] PR for this an other stashed changes #1153

otommod commented 6 years ago

I don't know if this issue is still actively... active, but I'd like to add that textclock should probably make format a property, so that you can create it declarative.

psychon commented 6 years ago

@otommod If the following patch works (does it? I don't know. Also, I don't know how to document this...), you could submit a pull request:

diff --git a/lib/wibox/widget/textclock.lua b/lib/wibox/widget/textclock.lua
index 829ee98c..e8fd0869 100644
--- a/lib/wibox/widget/textclock.lua
+++ b/lib/wibox/widget/textclock.lua
@@ -42,13 +42,14 @@ function textclock.new(format, timeout, timezone)
     timezone = timezone and TimeZone.new(timezone) or TimeZone.new_local()

     local w = textbox()
+    w.format = format
     w.force_update = textclock.force_update
     function w._private.textclock_update_cb()
-        local str = DateTime.new_now(timezone):format(format)
+        local str = DateTime.new_now(timezone):format(w.format)
         if str == nil then
             require("gears.debug").print_warning("textclock: "
                     .. "g_date_time_format() failed for format "
-                    .. "'" .. format .. "'")
+                    .. "'" .. w.format .. "'")
         end
         w:set_markup(str)
         w._timer.timeout = calc_timeout(timeout)
otommod commented 6 years ago

How can I check it? In fact, how do I even run awesome locally? I tried the test suite, but I don't think it used the local libs.

psychon commented 6 years ago

I always use make && tests/run.sh /dev/null (which runs /dev/null as a test and kills everything after something like two minutes since the test timed out; the make is important). Unless you do things with $LUA_PATH in the environment, this really should be using local files. You could try by adding a call to error() somewhere and see if this really causes an error.

otommod commented 6 years ago

Oh, you have to run make even when modifying lua. EDIT: #2153

psychon commented 6 years ago

Yup. The Lua code that is in git contains things that look like @FOO@. These are expanded to get "runnable Lua" (although in most cases this just inserts some documentation stuff). That's why you need to run make: so that the expanded Lua code in build/lib is updated.

actionless commented 6 years ago

background container with forced width/height should be drawn even if there is no child widget

will we still need to have constraint widget? or we could have one implementation and to have it just as an alias?