treewide: implement image editor

[LovingMelody]

This PR implements #473

[danth]

I'm reluctant to merge this because, as previously discussed, it goes beyond Stylix's goal of "take these theming items, and apply them everywhere".

If we were to instead provide the palette generator as a function, then you could simply call the palette generator with your original wallpaper, do whatever editing based on that palette, then set the final image as stylix.image.

In extension to this, removing the generated palette as the default for stylix.base16Scheme would resolve #437, since we can remove other complications in the code.

Assuming both of the above changes were made, the following combinations would be possible:

• Manually choose a scheme (nothing new here)

  { pkgs, ... }:
  {
    stylix.image = ./wallpaper.png;
    stylix.base16Scheme = "${pkgs.base16-schemes}/...";
  }
  

• Generate a scheme from the wallpaper (base16Scheme must now be set explicitly)

  { pkgs, config, ... }:
  {
    stylix.image = ./wallpaper.png;
    stylix.base16Scheme = generatePalette config.stylix.image;
  }
  

• Generate a scheme from an image, then modify the image using that scheme, and set the result as the wallpaper (this is a new possibility)

  { pkgs, config, ... }:
  {
    stylix.base16Scheme = generatePalette ./wallpaper.png;
    stylix.image = runLutgen ./wallpaper.png config.stylix.base16Scheme;
  }
  

  Where runLutgen is the image editor function from this PR and would be defined outside of Stylix

With the palette generator as a function, we could potentially move it to its own repository - it's only part of Stylix because it's been here since the very beginning. Image editor functions such as runLutgen could be distributed separately too.

All of the above can be implemented in two phases:

1. Copy the palette generator to a new repository, with its own project name, documentation, etc. This would export the generatePalette function described above
2. Remove the palette generator from Stylix. If base16Scheme is unset, the error message should direct users to the new repository

So in the end, Stylix would handle purely the "applying" tasks, and the "generating" would be a separate project.

@LovingMelody @trueNAHO what do you think?

[LovingMelody]

Think this sounds good to me, unsure what this would look like or how this would affect users. Think the project has been known to have say palette generation by default.

[danth]

The original idea of Stylix was "give it an image, and everything else happens automatically" hence the palette generator exists, but I think we've strayed from that by adding more options and so on.

If we avoid any big changes, just providing the palette generator as a function should remove the need for this PR.

[trueNAHO]

Foreword

I would have written a shorter letter, but I did not have the time.

-- Blaise Pascal

Body

With the palette generator as a function, we could potentially move it to its own repository - it's only part of Stylix because it's been here since the very beginning.

IMHO, replacing the current mono repo architecture does not add much benefit, especially, when the only use case for those micro repositories is Stylix. Managing these separately does not seem worth it for now.

I'm reluctant to merge this because, as previously discussed, it goes beyond Stylix's goal of "take these theming items, and apply them everywhere".

If we were to instead provide the palette generator as a function, then you could simply call the palette generator with your original wallpaper, do whatever editing based on that palette, then set the final image as stylix.image.

Initially, I had the same opinion. However, considering that this PR will be replaced in the future anyway, I don't have any strong feelings about this PR:

Disregarding #63 and the palette extension roadmap, this implementation seems fine for now.

-- https://github.com/danth/stylix/issues/473#issuecomment-2241104865

Assuming both of the above changes were made, the following combinations would be possible:

Your proposal does not seem #63 future proof:

Modifying images at Nix build time stores them in the Nix store. With #63, this results in potentially unbounded duplicate data.

-- https://github.com/danth/stylix/issues/473#issuecomment-2240557282

Actually, I have been thinking about this problem for a while now.

What follows is my prototype draft proposal with #63, #442, and the palette extension roadmap in consideration. I hope it is somewhat understandable:

scheme = mkOption {
  # Another example would be:
  #
  #     example = "./image";
  example = "${pkgs.base16-schemes}/share/themes/catppuccin-latte.yaml";

  # TODO: This option should accept two types of values from which the schemes
  # will be generated:
  #
  # - 'pkgs.base16-schemes' file: Extract scheme from file
  # - image path: Generate scheme from image
  type = null;
}

slideshow = {
  enable = mkEnableOption "slideshow";

  timer = mkOption {
    # This value should be directly passed to the internal systemd service
    # periodically selecting random file from 'stylix.source'.
    #
    # This value defines the update interval.
  }
};

source = {
  path = mkOption {
    default = null;

    description = ''
      Directory containing background sources, including images, videos, GIFs,
      etc.

      This value may be any valid Bash expression.
    '';

    example = "$XDG_RUNTIME_DIR/wallpapers";
    type = types.nullOr types.str;
  };

  # TODO: Potentially better rename this option.
  modifier = mkOption {
    default = null;

    description = ''
      This is the the body of a function, where the first argument is the path
      of the random path found by the systemd service. The second argument is
      the file, where the modified file should be written to.
    '';

    example = "${pkgs.lutgen}/bin/lutgen apply "$1" --output "$2" -- ${colors}";
    type = types.nullOr types.str;
  }

  # TODO: In case, Stylix supports an application's wallpaper can only be set
  # through an option like 'wallpaper=<PATH>' and a systemd service (even a
  # one-shot one) is unable to set the wallpaper, this option should be used.
  # However, if Stylix does not have any such cases, then there is no need for
  # this option.
  wallpaperFallback = mkOption {};
};

Due to application color limitations, Stylix should internally provide and generate the following schemes:

scheme = {
  base16 = {
    0 = "<VALUE>";
    1 = "<VALUE>";
    2 = "<VALUE>";
    3 = "<VALUE>";
    4 = "<VALUE>";
    5 = "<VALUE>";
    6 = "<VALUE>";
    7 = "<VALUE>";
    8 = "<VALUE>";
    9 = "<VALUE>";
    10 = "<VALUE>";
    11 = "<VALUE>";
    12 = "<VALUE>";
    13 = "<VALUE>";
    14 = "<VALUE>";
    15 = "<VALUE>";
  };

  base24 = {
    0 = "<VALUE>";
    1 = "<VALUE>";
    2 = "<VALUE>";
    3 = "<VALUE>";
    4 = "<VALUE>";
    5 = "<VALUE>";
    6 = "<VALUE>";
    7 = "<VALUE>";
    8 = "<VALUE>";
    9 = "<VALUE>";
    10 = "<VALUE>";
    11 = "<VALUE>";
    12 = "<VALUE>";
    13 = "<VALUE>";
    14 = "<VALUE>";
    15 = "<VALUE>";
    16 = "<VALUE>";
    17 = "<VALUE>";
    18 = "<VALUE>";
    19 = "<VALUE>";
    20 = "<VALUE>";
    21 = "<VALUE>";
    22 = "<VALUE>";
    23 = "<VALUE>";
  };

  highlight = { /* VIM highlight groups */};
};

Depending on the stylix.scheme option all other internal schemes should be generated. For example, given a base16 scheme, base24 and highlight should be generated through interpolation. Given an image to stylix.scheme every scheme should be generated as accurately as possible from the image. Note that this may require re-implementing the current generatePalette logic.

If stylix.slideshow.enable is set to true, then a systemd service in the interval given by stylix.slideshow.timer should be created, and it should run the following code. Otherwise, if stylix.wallpaperFallback is not required, then this systemd service could simply be a one-shot service:

# TODO: For brevity, this example assumes that all 'stylix.source' files are
# images.

modifier() {
  ${stylix.scheme.modifier}
}

# This operation finds a random image from 'stylix.source'.
#
# NOTE: In my experience 'find | shuf -n 1' is not random enough and results in
# only a small subset of files ever being shown. But I have never actually
# measured this. I only noticed this.
#
# NOTE: This function is only here to simplify the pseudo-code.
find_random_image() {
  # TODO: Implement.
}

# TODO: Depending on the user environment (like X or Wayland), this function
# abstracts how to set the background source.
#
# Additionally, in order to avoid "bloating" Stylix size, users should be able
# to set some Stylix option (not included or mentioned anywhere else in this
# message) saying what tool should set the background source.
#
# For example, if Stylix uses package 'A' by default to set the background
# source depending on the user environment, but the user does not use package
# 'A', then they might want to force Stylix to use package 'B', which they
# already use. This would reduce overall Stylix install size.
#
# NOTE: This abstraction could be implemented in Stylix, to avoid this logic
# from happening at runtime. Instead, the correct code could be generated by
# Nix, and then simply injected into this Bash script. This could somewhat
# improve performance and complexity.
set_stylix_source() {
  # TODO: Implement.
}

# This 'file="$(mktemp)"; <OPERATION> "$file"; rm "$file"' pattern avoids
# arbitrary and fragile tests to ensure that we are not overwriting or falsely
# removing the original user files located in 'stylix.source'.
#
# NOTE: If 'stylix.scheme.modifier' is set to 'null', then the logic could be
# simplified to 'set_stylix_source "$(find_random_image)"'. This case should be
# handled in Nix, and accordingly, the correct code should be injected into this
# Bash script. Also note that the previous 'modifier' Bash function in this case
# is no longer necessary.
file="$(mktemp)"
set_stylix_source "$(modifier "$(find_random_image)" "$file")"
rm "$file"

Note that we could add Stylix assertions to avoid certain invalid cases. For example, stylix.slideshow.enable = true; makes no sense when stylix.source.path is set to null.

[LovingMelody]

Depending on the stylix.scheme option all other internal schemes should be generated. For example, given a base16 scheme, base24 and highlight should be generated through interpolation. Given an image to stylix.scheme every scheme should be generated as accurately as possible from the image. Note that this may require re-implementing the current generatePalette logic.

Theres no reason to generate a base16 scheme & a base24 scheme when we can just use a fallback table for base24 -> base16

[trueNAHO]

Theres no reason to generate a base16 scheme & a base24 scheme when we can just use a fallback table for base24 -> base16

This is faster, but less accurate.

Unless generatePalette becomes unbearably slow, the base16, base24 and highlight schemes should be directly generated from the image to avoid any form of precision loss. This implies calling generatePalette for each scheme:

Given an image to stylix.scheme every scheme should be generated as accurately as possible from the image. Note that this may require re-implementing the current generatePalette logic.

However, a given scheme, like pkgs.base16-schemes, does not contain more information than the scheme itself. Consequently, it seems reasonable to upscale (from base16 to base24) or downscale (from base24 to base16) depending on the provided scheme (base16, base24 or highlight). This process may reuse original values or interpolate new ones to hopefully achieve better results. I believe interpolation would yield better results:

For example, given a base16 scheme, base24 and highlight should be generated through interpolation.

Since I have not yet looked deeper into VIM highlight groups, I am unsure how its upscaling or downscaling would be performed. I assume a combination of value repetition and interpolation, depending on the highlight group's purpose.

Extending generatePalette to extract base24 and highlight schemes from an image is still an open question:

Note that this may require re-implementing the current generatePalette logic.

[jaredmontoya]

As a user that based his nix flake config around stylix and treats it as a source of truth for theming I was waiting for this PR to be merged for some time as I am very interested in the said feature.

I don't know if my user feedback is welcome here and is free to be ignored, but I think that writing up an implementation spec to provide something for people to potentially agree on might be helpful

As far as I understand stylix now works like this:

• image has to be set.
• if image is set it is used as a source for the color scheme.
• if color scheme is set explicitly the image is only used for the background.

This is where I would take stylix if I was the one to direct it's development:

• image has to be set
• if image is set it is used as a source for both base16Scheme and base24Scheme.
  • base16Scheme is left for compatibility with legacy base16 modules that were not ported to base24Scheme.
    • base16Scheme can be deprecated in the future and all modules will have to be ported.
• if base24Scheme or base16Scheme is set explicitly it means that the user does not want to use the image as color scheme source.
  • if user sets only base16Scheme stylix will throw an error asking to also set a base24Scheme
    • I don't know if upscaling is a good idea, in case it is this could be different.
  • if user sets only base24Scheme the user can also optionally set base16Scheme variant but if not set it will be downscaled from the original base24Scheme.
• The fact that user has set a color scheme explicitly might mean that they also want the image to follow the color scheme

stylix = {
  # Modifies the image or images set by stylix.image and
  # stylix.additionalImages before they ever reach hyprpaper, grub, etc ...
  imageEditor = {

    # If explicitly enabled by the user while no color scheme was explicitly provided by the user
    # will use base24Scheme extracted from the image and apply it to the image before it reaches hyprpaper, grub, etc...
 
    # Enabled by default if the color scheme is set explicitly.
    # if the user set the color scheme explicitly but does not want the image to be modified the user can disable imageEditor.
    enable = true;

    # the only image editor at the moment. lib.types.enum [ "lutgen" ];
    editor = "lutgen";
  };
};

• Slideshow can be implemented like this:

stylix = {
  # used for sourcing color scheme in case user does not set one,
  # if the color scheme is set and image editor is enabled, the image is modified,
  # if the image editor is disabled the image is not modified.
  image = ./primary_image.png
  # defaults to empty list, slideshow disabled, is optional
  additionalImages = [
    # not used to generate the color scheme
    # if imageEditor is enabled they are modified, if disabled they stay unmodified
    ./additional_image_1.png
    ./additional_image_2.png
  ];
};

[trueNAHO]

I was waiting for this PR to be merged for some time as I am very interested in the said feature.

Actually, #102 already proposed some of this functionality, although it has essentially been abandoned due to the PR being too large.

I think that writing up an implementation spec to provide something for people to potentially agree on might be helpful

Yes, discussing the public API is a good idea.

As far as I understand stylix now works like this:

• image has to be set.
• if image is set it is used as a source for the color scheme.
• if color scheme is set explicitly the image is only used for the background.

Exactly.

This is where I would take stylix if I was the one to direct it's development:

• image has to be set

According to #200 and #442, declaring a background source should be optional.

• if image is set it is used as a source for both base16Scheme and base24Scheme.

stylix.image should probably be superseded by my previously proposed stylix.source.path option. Otherwise, this aligns with my proposal:

Given an image to stylix.scheme every scheme should be generated as accurately as possible from the image.

• base16Scheme is left for compatibility with legacy base16 modules that were not ported to base24Scheme.

  • base16Scheme can be deprecated in the future and all modules will have to be ported.

Ideally, VIM highlight groups should become the primary Stylix scheme format:

AFAIK, the current color scheme roadmap is to extend it to VIM highlight groups, although base24 support can be implemented in the meantime.

-- https://github.com/danth/stylix/issues/469#issuecomment-2240043203

To avoid inconsistent module-specific base16 and base24 support, Stylix should provide these schemes since some CLI applications might only support them.

• if base24Scheme or base16Scheme is set explicitly it means that the user does not want to use the image as color scheme source.

I am unsure whether Stylix should enforce this behaviour with stylix.image being potentially superseded by stylix.source.path.

• if user sets only base16Scheme stylix will throw an error asking to also set a base24Scheme

It might be annoying for to declare the same scheme in base16 and base24, especially with certain schemes not being published in both formats.

* I don't know if upscaling is a good idea, in case it is this could be different.

Downscaling or upscaling would be far more convenient. Additionally, this ensures scheme consistency, avoiding nasty incorrect coloring.

• if user sets only base24Scheme the user can also optionally set base16Scheme variant but if not set it will be downscaled from the original base24Scheme.

It might be better that the scheme is extracted from a single source of truth from which all other schemes are generated.

• The fact that user has set a color scheme explicitly might mean that they also want the image to follow the color scheme

Personally, re-coloring background sources to the Stylix theme sounds neat. However, I am sure many people would not want this.

stylix = {
  # Modifies the image or images set by stylix.image and
  # stylix.additionalImages before they ever reach hyprpaper, grub, etc ...
  imageEditor = {

    # If explicitly enabled by the user while no color scheme was explicitly provided by the user
    # will use base24Scheme extracted from the image and apply it to the image before it reaches hyprpaper, grub, etc...

    # Enabled by default if the color scheme is set explicitly.
    # if the user set the color scheme explicitly but does not want the image to be modified the user can disable imageEditor.
    enable = true;

    # the only image editor at the moment. lib.types.enum [ "lutgen" ];
    editor = "lutgen";
  };
};

Compare this to my proposal.

• Slideshow can be implemented like this:

stylix = {
  # used for sourcing color scheme in case user does not set one,
  # if the color scheme is set and image editor is enabled, the image is modified,
  # if the image editor is disabled the image is not modified.
  image = ./primary_image.png
  # defaults to empty list, slideshow disabled, is optional
  additionalImages = [
    # not used to generate the color scheme
    # if imageEditor is enabled they are modified, if disabled they stay unmodified
    ./additional_image_1.png
    ./additional_image_2.png
  ];
};

This does not scale well, as the sources are copied to the Nix store:

Modifying images at Nix build time stores them in the Nix store. With #63, this results in potentially unbounded duplicate data.

-- https://github.com/danth/stylix/issues/473#issuecomment-2240557282

[danth]

Unless generatePalette becomes unbearably slow, the base16, base24 and highlight schemes should be directly generated from the image to avoid any form of precision loss.

This could be problematic since the palette generator is pseudo-random, so each format could have a completely different set of colors, leading to inconsistency between applications. IMO, we should generate highlight since it includes the most colors, then downscale to base24 and base16 for compatibility.

[trueNAHO]

I don't mind merging this neat functionality for the time being as it will be replaced in the future anyway:

I'm reluctant to merge this because, as previously discussed, it goes beyond Stylix's goal of "take these theming items, and apply them everywhere". If we were to instead provide the palette generator as a function, then you could simply call the palette generator with your original wallpaper, do whatever editing based on that palette, then set the final image as stylix.image.

Initially, I had the same opinion. However, considering that this PR will be replaced in the future anyway, I don't have any strong feelings about this PR:

Disregarding #63 and the palette extension roadmap, this implementation seems fine for now. -- #473 (comment)

If @danth approves, I would be happy to review and test this PR again. Otherwise, we close this PR and keep it for future reference.

[danth]

considering that this PR will be replaced in the future anyway

@trueNAHO Could you clarify what you meant by this? (Sorry, I've lost track of the conversation since it's been a while)

[trueNAHO]

considering that this PR will be replaced in the future anyway

[...] Could you clarify what you meant by this? (Sorry, I've lost track of the conversation since it's been a while)

No worries.

Essentially, this PR does not scale once #63 is resolved as the number of modified input files would be unbounded:

Modifying images at Nix build time stores them in the Nix store. With #63, this results in potentially unbounded duplicate data.

-- https://github.com/danth/stylix/issues/473#issuecomment-2240557282

In the future, an implementation based on my prototype draft proposal would replace the architecture introduced by this PR:

What follows is my prototype draft proposal with #63, #442, and the palette extension roadmap in consideration.

-- https://github.com/danth/stylix/pull/477#issuecomment-2254170449

Ultimately, the public option introduced by this PR would be removed in the future. However, considering that this might take a while to happen, I am fine with merging the functionality introduced by this PR for now.

I leave the decision up to you:

If [danth] approves, I would be happy to review and test this PR again. Otherwise, we close this PR and keep it for future reference.

[LovingMelody]

I don't see a real reason to add this if it's just going to get removed. Your draft PR overall seems fine, but I'm not fond of the systemd service's implementation that you have outlined. The directory should probably cached & linked to a generation in case the modifier is computationally expensive.

[trueNAHO]

I don't see a real reason to add this if it's just going to get removed.

As mentioned, I don't have strong feelings on merging this, but avoiding another breaking change would be ultimately better.

Your draft PR overall seems fine, but I'm not fond of the systemd service's implementation that you have outlined. The directory should probably cached & linked to a generation in case the modifier is computationally expensive.

I already considered caching, but did not mention it in the prototype.

As mentioned, linking input or output directories to a Nix generation in the Nix store is not ideal due to storage duplication.

When I implemented my custom wallpaper setter, I already considered caching, but never implemented it due to reasonably fast execution time. However, since the source.modifier option from the prototype is externally configurable and may be arbitrarily slow, execution time should not be assumed.

Caching could be implemented by preemptively selecting, modifying, and caching the next <CACHE_NUMBER> background sources in the background using nice --adjustment=<ADJUSTMENT> ${source.modifier}, where 1 >= <ADJUSTMENT> <= 19 holds. If slideshow.timer triggers with an empty <CACHE_DIRECTORY> queue, a new background task should start without the nice wrapper to prioritize getting and setting a new background source.

Additionally, benchmarking and caching the source.modifier execution time (<SOURCE_MODIFIER_BENCHMARK>) at Nix build time would further reduce latency by starting a new high-priority (without the nice wrapper) background task <SOURCE_MODIFIER_BENCHMARK> earlier than slideshow.timer if <CACHE_DIRECTORY> is currently empty. For example, if slideshow.timer = 30 and <SOURCE_MODIFIER_BENCHMARK> = 10, then at time slideshow.timer - <SOURCE_MODIFIER_BENCHMARK> = 20 a new high-priority background task should be started if <CACHE_DIRECTORY> is currently empty.

[LovingMelody]

As mentioned, linking input or output directories to a Nix generation in the Nix store is not ideal due to storage duplication.

In your prototype, every time the service triggers it creates a new temp directory and creates the file there leading to more duplication than there would be in the nix store, which may even be desired behavior for it to be stored in the store ( I agree it shouldn't be the default ).

The reason I point to generations is so that files outside of that generation can be removed when it's no longer valid as well and not just on reboot similar to agenix.

[trueNAHO]

As mentioned, linking input or output directories to a Nix generation in the Nix store is not ideal due to storage duplication.

In your prototype, every time the service triggers it creates a new temp directory and creates the file there leading to more duplication than there would be in the nix store, which may even be desired behavior for it to be stored in the store ( I agree it shouldn't be the default ).

Are you referring to the following snippet:

# This 'file="$(mktemp)"; <OPERATION> "$file"; rm "$file"' pattern avoids
# arbitrary and fragile tests to ensure that we are not overwriting or falsely
# removing the original user files located in 'stylix.source'.
#
# NOTE: If 'stylix.scheme.modifier' is set to 'null', then the logic could be
# simplified to 'set_stylix_source "$(find_random_image)"'. This case should be
# handled in Nix, and accordingly, the correct code should be injected into this
# Bash script. Also note that the previous 'modifier' Bash function in this case
# is no longer necessary.
file="$(mktemp)"
set_stylix_source "$(modifier "$(find_random_image)" "$file")"
rm "$file"

In this case, mktemp actually creates a temporary file placeholder to which the modified file will be written to using the modifier function. Afterwards, the temporary file is removed again using rm "$file". Passing the input file and the modified around through stdout would avoid filesystem operations and improve performance. However, due to lack of implementation detail (due to it still being a prototype) and simplicity, the modified file is simply written to the filesystem.

The reason I point to generations is so that files outside of that generation can be removed when it's no longer valid as well and not just on reboot similar to agenix.

What exactly do you mean by generations and what would be removed? Are you referring to garbage collecting the Nix store?

AFAIK, the agenix cache defaults to $XDG_RUNTIME_DIR, which in turn defaults to /run/user/<UID>. AFAIK, unlike /tmp being cleared on reboot, /run/user is cleared on user logout.

My prototype (without the caching mechanism) leaves the input sources stored on the filesystem untouched and creates no files living longer than a second.

[trueNAHO]

IIRC, the plan was to replace this PR with a general solution based on the previously proposed prototype once we reach this stage in the Roadmap in order to avoid additional breaking changes.

What should we do about this PR? Merge or close?

[LovingMelody]

This can be closed, I'll continue maintaining this fork till we get there