Discussion: Potential gears.wallpaper improvements

[Sorky]

Idea: Move "set_wallpaper" into gears.wallpaper [remove code users don't need to touch in rc.lua]

Plus: Allow table : beautiful.wallpaper = {'bg_1.png', 'bg_2.png'}

Plus: Allow functions to do things like Get_Random_Filename

Plus: Allow beautiful.wallpaper to work without a path [use beautiful.theme_path variables]

Plus: Emphasize the use of gears.wallpaper.maximized, tiled, centered & fit

Plus: Update the doco for all the uses of beautiful.wallpaper and clarify the file path

Possible change from...

local function set_wallpaper(s)
    -- Wallpaper
    if beautiful.wallpaper then
        local wallpaper = beautiful.wallpaper
        -- If wallpaper is a function, call it with the screen
        if type(wallpaper) == "function" then
            wallpaper = wallpaper(s)
        end
        gears.wallpaper.maximized(wallpaper, s, true)
    end
end

-- Re-set wallpaper when a screen's geometry changes (e.g. different resolution)
screen.connect_signal("property::geometry", set_wallpaper)

awful.screen.connect_for_each_screen(function(s)
    -- Wallpaper
    set_wallpaper(s)

to...

-- Re-set wallpaper when a screen's geometry changes (e.g. different resolution)
screen.connect_signal("property::geometry",function()
    gears.wallpaper.maximized(beautiful.wallpaper, s, true, beautiful.wallpaper_path)
end)

awful.screen.connect_for_each_screen(function(s)
    -- Wallpaper
    gears.wallpaper.maximized(beautiful.wallpaper, s, true, beautiful.wallpaper_path)

Plus incorporate what was removed with updates in gears.wallpaper...

Benefits summary...

• Add table of backgrounds for multiple screen systems
• Allow function such as 'Get_Random_Filename' to be used
• Allow filename only for beautiful.wallpaper [ie: add beautiful.wallpaper_path or use beautiful.theme_path]
• Reduce code users shouldn't need to touch in rc.lua
• Update and clarify documentation

[Elv13]

Ref: https://github.com/awesomeWM/awesome/pull/2507

Allow function such as 'Get_Random_Filename' to be used

I am not sure about this. It seems like a feature creep rabit hole.

Update and clarify documentation

Absolutely

Reduce code users shouldn't need to touch in rc.lua [...] Add table of backgrounds for multiple screen systems

This goal is something we could make progress on.

May I propose to turn gears.wallpaper into an (object) class (moved to awful.wallpaper?). This object could have the tiled/stretched/centered mode as mutually exclusive boolean properties. The object would also have a screen property and track its own screen changes internally. Also, such class would have the ability to use programmatic wallpaper or wallpaper built out of a widget hierarchy (like the xresources wallpaper or @actionless hotkey popup directly burned into the wallpaper).

This approach is closer to the "declarative API paradigm" we try to move toward and completely remove the imperative logic from rc.lua. Coupled with the request::wallpaper proposed in #2507, we would have an API that provides all the features you proposed above (the random wallpaper would be achieved by adding a timer object and a new gears.filesystem.get_random_file_from_directory).

This seems to me a more manageable way forward and a stronger base to add new features if it turns out to be beneficial.

edit:

-- Change the wallpaper every 30 minutes with a random file
screen.connect_signal("request::wallpaper", function(s)
    local w = awful.wallpaper {
        stretch   = false,
        placement = awful.placement.centered,
        screen    = s,
        file_path = beautiful.theme_path .. "/walls/nice_wallpaper.png",
    }

    gears.timer {
        interval  = 1800,
        autostart = true,
        callback  = function()
            w.file_path = gears.filesystem.get_random_file_from_directory(
                beautiful.theme_path .. "/walls/"
            )
        end,
    }
end)

Note that the example above is much larger than what would be in the default rc.lua.

[Sorky]

May I propose to turn gears.wallpaper into an (object) class (moved to awful.wallpaper?).

I was only thinking of a smallish change but that sounds interesting

If a random file piece is wanted...

local os = { clock = os.clock }
local math = { random = math.random, randomseed = math.randomseed }
local table = { insert = table.insert }
math.randomseed( os.clock() % 1 * 1e6 )
--- Get a random file from a given directory.
-- @param string path The directory to search.
-- @tparam optional string extensions Only search files with specific extensions
--   [eg: { "jpg", "png" }. If ommited or nil, all files are considered.
-- @return A string containing a randomly selected filename.
function filesystem.get_random_file_from_directory(path, extensions)
    local files_in_path, valid_extensions = {}, {}

    -- Transform { "jpg", ... } into { [jpg] = true, ... }
    if extensions then
        for i, j in ipairs(extensions) do valid_extensions[j:lower()] = true end
    end

    -- Build a table of files from the path with the given extensions
    local file_list = Gio.File.new_for_path(path):enumerate_children("standard::name", 0)
    for file in function() return file_list:next_file() end do
        local file_name = file:get_attribute_as_string("standard::name")
        if not extensions or valid_extensions[file_name:lower():match("%.(.*)$")] then
            table.insert(files_in_path, file_name)
        end
    end

    -- Return a randomly selected file name
    return files_in_path[math.random(#files_in_path)]
end

[Elv13]

Cool, please open a pull request for this.

I was only thinking of a smallish change but that sounds interesting

That's usually a better idea, but in this case the API is very imperative and just plain bad, let's replace it. So to recap what we need:

• Have a tiled property
• Use awful.placement for the other stretch/center/align stuff (will need a new awful.placement to keep the aspect ratio, I will do it since I am the only one who understand the code)
  • Expose some of the placement properties like honor_workarea, honor_padding and margins.
• Support an optional draw() mode for Cairo
• Support a widget hierarchy
• Have some nicer beautiful options
• Detect screen geometry and removal internally
• Have a new "repainted" signal on the awful.wallpaper object
• Have a gears.filesystem.get_random_file_from_directory function
• Deprecate the old gears.wallpaper for Awesome 5.x once the new one is merged
• Have some tests/examples based snippets for the documentation.

edit: further refine the spec

[Sorky]

What's the preference here - to do this or not?

local os = { clock = os.clock }
local math = { random = math.random, randomseed = math.randomseed }
local table = { insert = table.insert }

[Elv13]

I am personally neutral. We usually only do it for the core API. On the other hand, it's a common pattern in Lua libraries. It's in theory faster, but again, that's a premature optimization. Others may disagree and ask you to remove it. So maybe better only doing this for capi and nothing else.

[warptozero]

@Sorky %.(.*)$ matches from the first dot it encounters, which will exclude filenames with dots in them from being valid. So you might want to change it to %.([^.]*)$.

[Sorky]

@Sorky %.(.*)$ matches from the first dot it encounters, which will exclude filenames with dots in them from being valid. So you might want to change it to %.([^.]*)$.

Good catch - I should have used what I did here: https://github.com/awesomeWM/awesome/pull/2601

[Sorky]

I even did some micro-timing...

local var1 = "%.([^.]*)$"
local var2 = ".file.ext"
local var3 = nil
for i = 0, 2^22 do var3 = var2:match(var1) end
print(var1,var2,var3)

time lua lua1

%.([^.]*)$      .file.ext       ext

real    0m1.308s
user    0m1.304s
sys     0m0.004s

time lua lua2

.*%.(.-)$       .file.ext       ext

real    0m1.020s
user    0m1.016s
sys     0m0.004s

time lua lua3

.*%.(.*)$       .file.ext       ext

real    0m0.914s
user    0m0.910s
sys     0m0.004s

[warptozero]

I even did some micro-timing...

So this is probably way overkill for what it's worth, but this lead me to also run a small test. And it looks like that, when correctness is defined as returning either the desired extension or nil, .+%.([^.]+)$ is the only correct pattern I came up with.

However returning an empty string is also fine in this case, and considering the timing is insignificant on the scale of a few thousand files, the most intuitive pattern that is correct enough might be the best one.

Anyhow this is probably not really worth the effort, but I found it interesting.

[warptozero]

An improved wallpaper module sounds awesome btw.

@Elv13 So if I understand it correctly: after compiling all the widgets and handeling the placement and such, the wallpaper gets rendered as an image the size of the screen?

[Elv13]

@warptozero Yes, this method:

https://github.com/awesomeWM/awesome/blob/a7474412daf2b0a84129e7c25af2a46ddecef826/lib/wibox/widget/init.lua#L40-L45

allows to dump widgets into a surface context. But before that we have to handle the wallpaper context. This is currently handled by:

https://github.com/awesomeWM/awesome/blob/a7474412daf2b0a84129e7c25af2a46ddecef826/lib/gears/wallpaper.lua#L54-L106

awful.placement supports wallpaper friendly placement properties here:

https://github.com/awesomeWM/awesome/blob/a7474412daf2b0a84129e7c25af2a46ddecef826/lib/awful/placement.lua#L28-L35

[Sorky]

Nice test! Adding '.filename' is an excellent corner case as well. It makes sense to me not treat it as an extension and as such ".+" is definitely a better start to the regex

The returning either the desired extension or nil is a bit too restrictive as "" is also an acceptable return given the usage is as a table key where nil and "" achieve the same result but I figure However returning an empty string is also fine in this case means you recognized that

Modifying your test to allow for it...

correct patterns:
.+%.(.*)$
.+%.(.-)$
.+%.([^.]*)$
.+%.([^.]-)$
.+%.([^.]+)$

For which .+%.(.*)$ is the fastest

However if .+%.([^.]+)$ is seen as being preferable I'll happily use that

Anyhow this is probably not really worth the effort, but I found it interesting.

Same here ;-)

[psychon]

The returning either the desired extension or nil is a bit too restrictive as "" is also an acceptable return given the usage is as a table key where nil and "" achieve the same result.

They do?

$ lua -e 'a = {} a[""] = 42 a[nil] = 42'
lua: (command line):1: table index is nil
stack traceback:
    (command line):1: in main chunk
    [C]: in ?

[Sorky]

In context [the table is not written to by the user] they "achieve the same result"

Unless the user passes in {""} as the selected extensions in which case it potentially doesn't do what is requested [actually it currently does for files that end in "." but not for those without an extension]

Given I didn't consider it before I think I should probably allow for it