treewide: wallpaper refactor

[SomeGuyNamedMay]

Checklist

• Module support
• Check function for types that actually restricts the type
• Add documentation
• Add testing

Potential Additions

• Move more configuration into the custom types such as polarity
• Additional constructors for things such as generating a wallpapers like penrose tiling's and the like
• Add a utility function for documenting constructors

Reference

The following depend or should be improved based on this PR:

• Recolored Icon Packs #131
• Adaptive Cursor Theme #182
• Create aliases for colors #207
• Different themes per application #208
• stylix: support various stylix.fonts.sizes units #251
• stylix: support slideshows and animated wallpapers #63
• Add trick with dynamic wallpaper generation #135
• treewide: adapt icon theme colors #157
• feat(modules/qutebrowser): apply dark mode depending on polarity #175

[SomeGuyNamedMay]

running into this weird error when building docs
error.txt

[SomeGuyNamedMay]

ready for review when you've got time.

[SomeGuyNamedMay]

@danth i think this is ready whenever you have time

[danth]

The MacOS tests should pass once you pull in 9197996 from the master branch.

[danth]

We need to update the configuration page on the website to use the new constructors

[SomeGuyNamedMay]

do we want to run checks on that wallpaper types that ensures there contents are of the correct types?

[danth]

That would be good if possible

[danth]

Converting these to functions (like toHex, toInt, toPolarity) will make our lib folder only depend on pkgs and the upstream lib, without reading anything from config. I'm hoping that will allow it to be passed as a module argument without causing infinite recursion.

[danth]

Duplicated function.

[Sntx626]

What's the state of this PR (draft)?

I'd be willing to help by rewriting and deduplicating the functions in lib/utils.nix.
Is there something else I can/should work on?

[danth]

@Sntx626 The main reason this isn't merged yet is because I'd like to remove the need to write out config.lib.stylix before every function, and just have it imported as { stylix, ... }. I tried to implement that a while back, but it led to infinite recursion.

The things you mentioned should help towards that.

It would also be useful to have more testing to make sure this doesn't break existing configs, or at least gives a helpful error message.

[trueNAHO]

The main reason this isn't merged yet is because I'd like to remove the need to write out config.lib.stylix before every function, and just have it imported as { stylix, ... }. I tried to implement that a while back, but it led to infinite recursion.

Considering that several internal feature PRs are waiting for this PR to land, it might be better to refactor config.lib.stylix with { stylix, ... } at a later time in a followup PR.

It would also be useful to have more testing to make sure this doesn't break existing configs, or at least gives a helpful error message.

However, considering the enormous rewrite, this should definitely happen before merging this PR.

Personally, this PR and its direct dependants might be one of the most drastic improvements to this project so far. Thanks for all the work so far!

For reference, the following PRs and issues depend on this PR:

• Recolored Icon Packs #131
• Adaptive Cursor Theme #182
• Create aliases for colors #207
• Different themes per application #208
• stylix: support slideshows and animated wallpapers #63
• treewide: adapt icon theme colors #157

For reference, the following PRs and issues should be improved based on this PR:

• Add trick with dynamic wallpaper generation #135
• feat(modules/qutebrowser): apply dark mode depending on polarity #175

Feel free to add any PRs or issues I missed and to update the description of this PR to properly link the PRs and issues.

[trueNAHO]

As mentioned in #251 (comment), the "black box" type introduced in 7a080cb is a genius type conversion abstraction.

However, I have my performance doubts about providing conversion functions instead of converted values. Specifically, does calling a conversion function multiple times convert multiple times or only once? Would static attribute sets not provide vastly better performance by caching their result? Additionally, the immutably converted data prevents any kind of conversion inconsistencies.

[Sntx626]

This is a valid concern.

However I think we should first measure the real-world impact of using the black box type and see if it is as bad as you think it could be. Or if there are other areas we should improve on first.

I also don't know enough about nix, but it could cache the result of the function and basically need to evaluate it only once. The optimization we seek might not even lay in this codebase.

Just an example of what would improve build times more:
I'm overriding all the colors myself. But stylix still generates a color palette from the bg image. I don't know if there's a way to turn that off, however running the genetic algorithm takes a lot of times. Which is not needed in my case afaik.

[Sntx626]

While functional, there's still room for improvement.

[Sntx626]

Currently the function takes the color attr set.
This makes it relatively simple to call the function.

However this argument is not type checked. This will result in misleading errors at best and is documented nowhere atm. (except for this comment...)

Should we take red, green and blue as arguments instead?

[Sntx626]

We should probably create a library independent of config and tell the user how to extend their lib with ours.

This way we can finally be free of the config argument.

[danth]

A completely separate library (stylix / stylixLib or something) may be easier than trying to extend the existing lib.

[SomeGuyNamedMay]

I think a separate library is probably a better idea, extending lib doesnt seem to be worth the effort

[trueNAHO]

Closing this stale PR in favor of the proposal in #477 (comment), now tracked in the Roadmap. Thanks to everyone for the all valuable groundwork!