eylles / pywal16

16 colors fork of pywal
MIT License
157 stars 24 forks source link

Color contrast specification / cmd line option #60

Closed ambertia closed 1 month ago

ambertia commented 1 month ago

I'd be interested in the ability to control the contrast of colors in the palette relative to the source image using a command-line argument. I find that many of the colors can, at times, be difficult to read e.g. in partially transparent terminals through no fault of the application; I've done some work making a python script to evaluate the average pixel value of the source image and modify the colors in the palette to have >= a specific contrast ratio according to the formulae outlined in the W3 contrast specifications. I'd like to work on implementing the actual math I've written into pywal itself in a fork, and if you'd like I'd be glad to have it pulled in when I'm finished.

ambertia commented 1 month ago

swappy-20240802_161206 Here's an example of the palette from this wallhaven background being modified to a contrast ratio of 3.0. The W3 spec recommends a minimum of 3 for large text, 4.5 for standard text, and 7 for enhanced contrast on standard text; I find that even just ensuring it's at least 2 helps with making sure the colors are readable in the terminal.

eylles commented 1 month ago

Okay, i got no issue with that, but the actual implementation would have to be added into the generic color adjust function: https://github.com/eylles/pywal16/blob/ce46d3c8cfd7fa064dcdf4c6c36c54c2eb19f01a/pywal/colors.py#L67

As the way pywal works is that it is assumed every single backend can return at least 8 colors that are different enough (big assumption) then the generic color adjust function touches them up to create a ligh colorscheme (if flag is passed) or return either a legacy 9 colors scheme or the new 16 colors scheme with either the darken or lighten methods.

To adjust the contrasts between the 8 base colors (background color0, foreground color7, 6 assorted colors 1 to 6) such that the function can still flip the foreground and background to create a light colorscheme and/or darken the colors 1 to 7 or lighten the colors 9 to 14 to preserve the 2 methods used that allows users to choose either amore muted or an almost pastel look for the final palette, the constrast adjust would have to be performed before the other adjust operations.

At least that is my assesment at the moment.

ambertia commented 1 month ago

Another potential problem would be that so far I've been getting the average pixel value by using imagemagick to convert the image to 1x1, and as I understand not all of the backends require imagemagick, so it might require a different method of getting the average color or that option wouldn't work without imagemagick installed.

The other design decision would indeed be whether to have it before or after the other generic adjustments; in my mind it would be a completely optional post-processing step on the final colors. In my opinion if the theme is flagged to be light or dark, the generic adjustments might give the colors enough contrast that they wouldn't need to be changed any further; if the contrast adjustment was done first, it might end up modifying the colors further from what the backend originally output than necessary.

That being said I'm just going to keep messing with it on my end for the time being!

ambertia commented 1 month ago

It also occurred to me that it might be hard to use a command-line argument to control the desired contrast ratio if it was in the generic_adjust function, as the argument would have to get passed around different places; in contrast (pun acknowledged), if it was somewhere in the neighborhood of colors.get(): https://github.com/eylles/pywal16/blob/ce46d3c8cfd7fa064dcdf4c6c36c54c2eb19f01a/pywal/colors.py#L230 then it could be passed into this method alongside the saturation value in saturate_colors

eylles commented 1 month ago

That could work, for me there is no problem adding more arguments to the generic_adjust function, after all that boat sailed long ago when the ability to generate 16 colors was first added and then later when i added the lighten method, i know that is a hassle for any project using pywal16 as a library, but so far the only project i know does that is wpgtk and they added support for pywal16 back in january, pywal16 has changed internally quite a bit since so no idea how it works for them as i don't use wpgtk.

Rambling aside, doing the contrast adjust after the generic adjust is the way to go.

ambertia commented 1 month ago

Another stumbling block I'm running into again that I had run into before and forgotten about is the numbering of the color schemes, which is a bit more fundamental to how the application actually works. I've noticed that the first half of the colors are always darker, and the second half are always brighter. When modifying the colors to ensure their contrast this becomes a bit of a complicated problem; the way I had it set up before was to just process all the colors (except perhaps color0), but it would make more sense to only modify the foreground colors.

The thing that I've noticed is, this gets complicated when talking about light vs dark themes, because there's not really a strong delineation between "foreground" or "background" colors (except e.g. color0 clearly being a background color) in the schemes. Templates that use color2, for example, will always be using the darker version of the color compared to color10 regardless of whether the theme is dark or light.

The reason this is even a question I asked to myself is that you can see the contrast-ification making the front 8 and back 8 colors much closer together, which in some ways defeats the purpose of having 16 colors, but if I'm trying to style a terminal to make the text stand out on a dark background I need the specific colors used in the template to have the required contrast.

I think for now, as an optional post-processing step, I'd make it work the way it had already where it just modifies (or at least checks on) almost all of the colors except say 0 and 15 that get explicitly used in different templates as foreground/background. I bring it up mostly as food for thought.

eylles commented 1 month ago

well, the "frontrow" colors (1 to 7) being darker than the "backrow" colors (9 to 15) is an intentional part of the "format" for 16 colors colorschemes, as usually the colors 1 to 6 are the "normal" colors while colors 9 to 14 are the "bright" colors, color 0 is usually the same as background (but not always) and color 15 is the same as foreground (not always)

an example of a popular theme that follows that unspoken spec would be dracula, particularly the xresources dracula theme is a great example.

ambertia commented 1 month ago

I have a working build with everything in the scope of what I set out to do, but there is one caveat (which I don't think would end up being that much of a deal, but it's worth bringing up)

I added the contrast argument on my branch as one of the variables used for the cache name, but adding that will invalidate existing cached color schemes. It shouldn't cause any unexpected overlap or unintentional loading of old cached themes, but it would make themes have to be generated again next time the command was used.

If there were a design decision later down the line to move the contrast checking before the generic adjustments, then all it would take is the function calls to get moved around and the contrast value being passed through those extra functions as we discussed!

I ran the unit tests with python -m unittest discover tests from the pywal16 directory and everything output OK

eylles commented 1 month ago

I added the contrast argument on my branch as one of the variables used for the cache name, but adding that will invalidate existing cached color schemes. It shouldn't cause any unexpected overlap or unintentional loading of old cached themes, but it would make themes have to be generated again next time the command was used.

okay let me see if i get that correctly, does it work in a similar way to the cols16 method? say for example, i pass an image to pywal16, using the --cols16 flag with the 'darken' argument and the result colorscheme will be named like path_to_waifu_image_png_16_darken_wal_None_42069_1.1.0.json but if i pass 'lighten' then the name will be like path_to_waifu_image_png_16_lighten_wal_None_42069_1.1.0.json.
so with the --contrast flag if i were to pass say '2' the colorscheme name would be something like: path_to_waifu_image_png_16_darken_wal_None_2_42069_1.1.0.json
but passing a 3 would be: path_to_waifu_image_png_16_darken_wal_None_3_42069_1.1.0.json
right? or did i fail to understand?

I ran the unit tests with python -m unittest discover tests from the pywal16 directory and everything output OK

i will be honest with you, since i've been working on the pywal16 fork i've never used the unit tests, only discovered those existed a couple days before releasing 3.6.0, all this time i've been rawdogging the testing of pywal16 by running it with every image i can, with every flag relevant to what i'm doing and keeping an open eye if it blows up and how. anyway, i will take if that your testing was thoroughly and it works as intended.

if you got any question please feel free to ask, as a codebase pywal was written in 2017, way before the advent of LSPs, treesitter and all the current coding niceties we are used as of current year, thus cruft is bound to exist on the less worked parts of pywal.

ambertia commented 1 month ago

Yeah I didn't think the unit tests had been touched in a while and they had gotten set up a long long time ago, I was mostly curious if they would pick up on anything I missed 😅.

As for the caching, I passed the string from the command line argument in directly: wal -i "path_to_img_png" --cols16 -n caches to path_to_img_png_16_darken_dark_None_None_None_1995855_1.1.0.json (extra None compared to the same command without the branch changes) whereas wal -i "path_to_img_png" --cols16 -n --contrast 3 will cache to path_to_img_png_16_darken_dark_None_None_3_1995855_1.1.0.json

This works but isn't necessarily the greatest way to do it because passing --contrast 3 and --contrast 3.0 will cache to different files even though their meaning is equivalent

eylles commented 1 month ago

This works but isn't necessarily the greatest way to do it because passing --contrast 3 and --contrast 3.0 will cache to different files even though their meaning is equivalent

Ah that is easy, just make the argument a float with float() so that the contrast value will always be a float and the inputs 3 and 3.0 will be cached onto the same colorscheme file.

Open the pr and i'll merge.