Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert RGBA-format PNG files during install #68

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented Jan 9, 2025

This PR is the first of two possible solutions to the issue of RGBA-format PNG files making their way into the repo. Files with an alpha channel cause problems for the GNOME Shell animated backgrounds code, so they need to be converted to RGB for the transitions to display properly. (See #51, #67.)

This PR addresses that by using ImageMagick to process the files as they're being installed, ensuring that only images without an alpha channel are installed, no matter what format the repo's source images are in.

As such, it does add ImageMagick as a new build dependency for the repo.

These commits also represent a general updating and enhancing of the entire build process, which IMHO are useful with or without the ImageMagick processing. And if we decide not to go this route, I can drop the conversion rules from this PR and leave the rest of the fixes in place.) Notable changes include:

Makefile(s)

  • Use recommended1 recursion via phony subdir rules and $(MAKE) -C, instead of scripted loop. Installs can now be run in parallel with make install -jN (though this would only have any effect if both of the default/ and extras/ directories were being installed together.)
  • Move most variables to top-level Makefile, and export to sub-makes so they can be shared between the two paths.
  • Break each Makefile's install rule up into install-base plus a set of install-(DE) rules, one for each Desktop Environment. All install-* rules are prerequisites of the main install rule.

f41-backgrounds.spec

  • Updated to add ImageMagick as a BuildRequires.
  • Turned with_extras into a real bcond_with, so that installation of the extras/ dir can be activated with rpmbuild ... --with extras
  • Fixed the handling of with_extras so that activating it has the proper effect
  • Corrected the name of the license file installable from the repo

Installed image filenames (XFCE package)

The XFCE files are all installed to the same directory. Previously, the Makefile installed the images/symlinks as follows:

  • From the default/ directory:
    01-day.png was installed (by copying) as f41.png
    01-night.png was installed (by copying) as f41-01-night.png
  • From the extras/ directory:
    01-dark-blue.png was symlinked as f41-01-dark-blue.png
    01-light-blue.png was symlinked as f41-01-light-blue.png

(I have no idea why some are copied and others are symlinked, seems like the default/ files could be symlinked as well. I can easily change that if it's useful, but I've left it the way it is so far.)

...And then in the spec file, only f41.png was packaged in f41-backgrounds-xfce4, with the rest (including the default dark image) theoretically going into f41-backgrounds-extras-xfce4 if that weren't hardcoded as disabled.

Because the ImageMagick processing rules don't lend themselves to renaming the output file like that, and because it didn't really make sense to me that only one of the two default theme files was being packaged in the -xfce4 subpackage, this PR changes the install so that:
01-day.png is installed as f41-01-day.png
01-night.png is installed as f41-01-night.png
01-dark-blue.png is symlinked as f41-extras-01-dark-blue.png
01-light-blue.png is symlinked as f41-extras-01-light-blue.png

The spec file will now create the main -xfce subpackage to contain f41*.png, excluding f41-extras*.png (if with_extras is enabled). The excluded files will go into the -extras-xfce subpackage instead. This seems more consistent, and doesn't relegate the f41 dark background to second-class status.

So, this PR does change the installed name of the XFCE background (from f41.png to f41-01-day.png, but it also provides the f41-01-night.png image that's included for all other DEs, but was being left out of the XFCE package for some reason.

RESOLUTIONS

While I was combining the redundant variables from the two directories' Makefiles, I noticed that the list of resolutions being used in the extras/ dir had one additional size in the list, that wasn't present in the defaults/ directory's Makefile.

That size is 1920x1280, which is a very weird resolution that I've personally never seen or heard of on any screen I've used. So, maybe it was an error that had been corrected in defaults/ but not in extras/. In any case, because I used the more complete list it's now present in the set of symlinks created from both Makefiles — if it shouldn't be there, let me know and I'll remove it.

Fixes #67

- Use recommended[1] recursion via phony subdir rules and `$(MAKE) -C`,
  instead of scripted loop. Install can now be run in parallel with
  `make install -jN`.
- Move many variables to top-level makefile, and `export` to sub-makes.
- Break each Makefile's `install` rule up into `install-base` plus a
  set of `install-(DE)` rules, one for each Desktop Environment. All
  `install-*` rules are prerequisites of the main `install` rule.
- Use ImageMagick to process PNG-format images during install, to ensure
  alpha channels are removed to avoid breaking GNOME Shell timed
  background transitions.
- Update RPM spec file to add ImageMagick as a BuildRequires.

[1]: https://www.gnu.org/software/make/manual/html_node/Phony-Targets.html
The XFCE files are all installed to the same directory. Previously,
the Makefile installed the images/symlinks as follows:
- From the default/ directory:
    01-day.png was installed (by copying) as f41.png
    01-night.png was installed (by copying) as f41-01-night.png
- From the extras/ directory:
    01-dark-blue.png was symlinked as f41-01-dark-blue.png
    01-light-blue.png was symlinked as f41-01-light-blue.png

...And then in the spec file, only f41.png was packaged in
f41-backgrounds-xfce4, with the rest (including the default dark
image) going into f41-backgrounds-extras-xfce4.

Because the ImageMagick processing rules don't lend themselves to
renaming the output file, and because it doesn't really make sense
to package only _one_ of the two default theme files in the -xfce4
subpackage, this commit changes the install so that:
  01-day.png is installed as f41-01-day.png
  01-night.png is installed as f41-01-night.png
  01-dark-blue.png is symlinked as f41-extras-01-dark-blue.png
  01-light-blue.png is symlinked as f41-extras-01-light-blue.png

The spec file will now create the main -xfce subpackage to contain
f41*.png, _excluding_ f41-extras*.png. Those files will go into the
-extras-xfce subpackage instead. This seems more consistent, and
doesn't relegate the f41 dark background to second-class status.
@luyatshimbalanga
Copy link
Contributor

Thank you the pull request. For the resolution, 1920x1280 size is basically 3:2 ratio. Let's remove it for the time being until an user owning that very rare device reports an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

F41 timed transitions broken (again) due to alpha channel (again)
2 participants