Skip to content

Conversation

Olf0
Copy link
Contributor

@Olf0 Olf0 commented Jul 2, 2025

For its original source, see https://codeberg.org/marbalf/Sailfish-icons/issues/3

For the last state of the collected (in 2021) remnants of prior Patchmanager icons:
https://github.com/sailfishos-patches/patchmanager/tree/5fe27617d86002c640b93fd865c8ee841bcae4d7/.icons

For its original source, see https://codeberg.org/marbalf/Sailfish-icons/issues/3

Signed-off-by: olf <Olf0@users.noreply.github.com>
@Olf0 Olf0 self-assigned this Jul 2, 2025
@Olf0
Copy link
Contributor Author

Olf0 commented Jul 2, 2025

@nephros, I still have to overhaul / adapt the .icons/README.md, hence I marked this PR as a draft.

But:

  1. I also would like to deduplicate the three identical copies of icon-m-patchmanager2.svg (see ls -lR src/icons/svgs) and wonder if we should also adapt the name?
  2. There is actually another, but renamed copy, see GitHub code search for repo:sailfishos-patches/patchmanager path:/\.svg$/: src/bin/dialog/patchmanager-icon.svg
  3. Is there a (still valid) reason why it is not simply called patchmanager.svg? O.K., it is an app icon, so I can see some appeal in patchmanager-icon.svg, which also would retain one of the two extant names, but what are the upsides of / reasons for icon-m-patchmanager2.svg?
  4. Are all these four locations really (still) needed? I.e. isn't it possible / easier to adapt the references pointing to these locations than to duplicate or symlink the file proper?
  5. If I reduce it to a single location with symbolic links from the other places, which would be the preferred location for the real file?

Code locations which reference these two icon file names:

P.S.: Luckily and nicely repo:sailfishos-patches/patchmanager path:/atchmanager.*\.png$/ yields no hits, hence I assume this repo does not contain any PNG export of this Patchmanager app icon, right?

@nephros
Copy link
Contributor

nephros commented Jul 2, 2025

"Namen sind Schall und Rauch" ;)

So we actually have two icons in use:

  • One is the "Theme Icon", the monochrome patchmanager icon showing up in Settings
  • the other the App Icon - the colored one - currently used nowhere by PM itself(?), but used for the app package icon, Chum Metadata, Openrepos etc.

IIRC the theme location is used by Jolla Settings to find its icons - any other place will not work.

As for naming the icon-m- prefix is a convention used in Theme icons (i.e. those placed in /usr/share/themes/... and not /usr/share/icons/....

I would vote to keep that name for the Theme Icon to stay aligned with all the other Settings apps (grep icon /usr/share/jolla-settings/entries/*.json).
Maybe drop the 2 from the name but that's a detail.

@nephros
Copy link
Contributor

nephros commented Jul 2, 2025

As for the duplicated copies of the Theme Icon SVG source:

These (source) file paths/locations (and IIRC all three!) are required for the build system to consistently produce the right PNGs in the right locations. (Remember #472 (comment), #479 etc.)

Other wise, yes we can use symlinks for the source files, that works.

(Side note: I have still to see documentation what the use/difference of the icons and icons-monochrome is since SFOS 4.6. As long as this is an unknown, we leave everything where we know it works.)

@nephros
Copy link
Contributor

nephros commented Jul 2, 2025

  • the other the App Icon - the colored one - currently used nowhere by PM itself(?), but used for the app package icon, Chum Metadata, Openrepos etc

Now, this one.

AFAICS that is used nowhere in the application itself.

There is a file at "/usr/share/patchmanager/data/patchmanager-big.png" which is used in

source: "/usr/share/patchmanager/data/patchmanager-big.png"

Then we have https://github.com/sailfishos-patches/patchmanager/blob/d0006367cc276bf2e3739f7e2830f0df4ea3de28/src//bin/dialog/dialog.qml#L200 which uses patchmanager-icon.svg (without any path).
It's possible that this doesn't even work, not sure. We do ship that svg at /usr/share/patchmanager/data/patchmanager-icon.svg but I can't imagine dialog.qml being able to find it.

Also, using SVG files as image sources in our old QML needs the qt5-qtsvg-plugin-imageformat-svg package, which is not installed by default, and I don't think PM depends on it.

(All of this is pretty harmless though, users are unlikely to see the PM Dialog Cover, and if they do they won't notice a missing icon.)

Apart from these we have only external users of the icon:

  • Github project icon
  • OpenRepos icon (?)
  • Chum meta PackageIcon
  • ... and however RPM determines its package icon

Which means we need at least one generated PNG got the external users.

For app internal use, we could switch to a single source (e.g. patchmanager-app-icon.svg) and also use sailfish_svg2png to generate pngs at build time, and replace the above icon source locations in the code with proper "image://theme/patchmanager-app-icon" resource URLs.

@Olf0
Copy link
Contributor Author

Olf0 commented Jul 2, 2025

To start with the easy part, because that is mostly already done, yesterday:

Apart from these we have only external users of the icon:

  • Github project icon
  • Done by uploading the "480x480 px PNG" (as a compromise of size vs. image resolution = quality for this purpose) for the GitHub-"organisation" "SailfishOS Patches / sailfishos-patches" yesterday.
  • GitHub-"projects" do not use an icon automatically, still I created and uploaded a "social media icon" (1280x640 px PNG) in its project settings yesterday. This is based on the "560x560 px PNG", centred on a larger, transparent canvas.
  • OpenRepos icon (?)

Done uploading yesterday, for that I always create a "256x256 px PNG", which is the maximum icon size at OpenRepos (larger icons become downscaled when uploading them).

  • Chum meta PackageIcon

… is just a URL pointing to wherever we decide to put the SVG icon file proper to.
I shall adapt that URL, when a decision is met.

  • ... and however RPM determines its package icon

LOL, you sound like what I thought 7 or 8 years ago, when I packaged my first RPMs for SailfishOS, only to discover that this is a thing of the distant past: Only GIF (1, 2, 4, 8 bit indexed 24 bit RGB colour space, "optional" LZW compression) and XPM2 (a historic, text-based icon image format for X10 and X11 without compression, which results in huge file sizes) are supported and no GUI-based RPM-utility of the past > 15 years displays them; but you can still let rpmbuild embed such a GIF or XPM2 file in an RPM file it generates, and it will still immediately complain, if anything with this image file is not adhering to the RPM specification.

There is actually one more location I remembered:

  • Patchmanager icon at app.Transifex.com
    Specifically for Transifex (and GitHub) I generate "560x560x px² on a (* √2 =) 792x792 px² canvas" icon images by default, because both show a circular aperture of a square image: This lets the whole image be visible, but it also becomes smaller than intended. But as a circular aperture of the Patchmanager icon is actually looking quite good (a fully green background with the "+ / -"-patch prominently in the foreground), I used the "480² px² PNG" for GitHub, and this, the "560² px² PNG" or the "256² px² PNG" (forgot which resolution) for Transifex, yesterday.

Which means we need at least one generated PNG got the external users.

We have a whole lot of directly exported PNGs (86², 108², 1282, 172², 256², 480², 560² square-pixels) of the new icon in the png subdirectory and its social media sub-subdirectory (480x480 px² on a 1500x500 canvas, 560x560 px² on 960x640 px², 1280x640 px² and 792x792 px² canvasses): See PR #514 (this one)!

  • the other the App Icon - the colored one - currently used nowhere by PM itself(?), but used for the app package icon, Chum Metadata, Openrepos etc

Now, this one.

AFAICS that is used nowhere in the application itself.

There is a file at "/usr/share/patchmanager/data/patchmanager-big.png" which is used in

source: "/usr/share/patchmanager/data/patchmanager-big.png"

Oh, I really wonder why the GitHub-search repo:sailfishos-patches/patchmanager path:/atchmanager.*\.png$/ yields no hits, then? Is patchmanager-big.png auto-generated or renamed in the build process?

Edit: I did the counter-check by looking for the same RegEx matching to file contents (instead of paths): repo:sailfishos-patches/patchmanager /atchmanager.*\.png$/ O.K. so we seem to be aware of all hits, already.

Edit2: I lastly checked the content of an RPM file (i486 build with SDK-for-440) of our current Patchmanager release (3.2.12): All files ending with .svg, and all the ones with .png ([a], [b])
Actually, this view depicts it even better, showing all .svg and .png except for [b] in a single view.

Anyway, thank you for the heads up, I will try to research and consider what to do with it, then come up with a proposal here.

Then we have https://github.com/sailfishos-patches/patchmanager/blob/d0006367cc276bf2e3739f7e2830f0df4ea3de28/src//bin/dialog/dialog.qml#L200 which uses patchmanager-icon.svg (without any path). It's possible that this doesn't even work, not sure. We do ship that svg at /usr/share/patchmanager/data/patchmanager-icon.svg but I can't imagine dialog.qml being able to find it.

Exactly these are the cases, why I involved you: I have no idea which mechanism lets such code find such an icon, at which location(s?) it might be found etc.? Also: Is a symbolic link in the Git repo sufficient or may something ever require the real file at some location?

Also, using SVG files as image sources in our old QML needs the qt5-qtsvg-plugin-imageformat-svg package, which is not installed by default, and I don't think PM depends on it.

Oh, I did not consider to alter any "real code" by this PR, at most file paths in code.

Actually this is exactly where I would like to draw the border of the scope of this PR: file deduplication and housekeeping ("cleaning up"), but not switching every Patchmanager icon consuming instance to a single SVG source image (yet). Still, this is not a holy rule which has to be obeyed, even if it would come handy to break it; it is just that I want to delimit the scope of this PR.

(All of this is pretty harmless though, users are unlikely to see the PM Dialog Cover, and if they do they won't notice a missing icon.)

No, I sure do not intend to cause any regressions, even if they are not noticeable for most.

For app internal use, we could switch to a single source (e.g. patchmanager-app-icon.svg) and also use sailfish_svg2png to generate pngs at build time, and replace the above icon source locations in the code with proper "image://theme/patchmanager-app-icon" resource URLs.

I actually prefer to have pre-computed PNG icons embedded in the RPM file, at least in Jolla's classic icon resolutions of 86², 108², 128² and 172² px²:

  • I gave up on trying to get Jolla's sailfish_svg2png to generate PNGs at build time for Storeman many years ago, because I repeatedly failed to, despite seeing that some other SailfishOS-related projects apparently use it successfully. But while researching the reasons for this, I discovered …
  • … that the quality of the PNGs generated by sailfish_svg2png ranges below what Inkscape generates with "Antialiasing" in the "Advanced" options of its PNG export dialog set to CAIRO_ANTIALIAS_GOOD (default), i.e. better than CAIRO_ANTIALIAS_FAST, but noticeably worse than CAIRO_ANTIALIAS_BEST.
  • Thinking about it (long ago), this is a classic "compute vs. memory" trade-off ("rainbow tables" for cryptographic hashes being the most prominent example): As the PNG file sizes actually shrink with a better antialiasing algorithm (because of less blurry images to LZW-compress), and the total sum of an icon as PNGs in all Jolla's classic icon resolutions (86², 108², 128² and 172² px²) is ca. 50 KByte (always less than 100 KByte, sometimes even just ~ 35 KByte), a better visual fidelity can be achieved by pre-computing them, and ultimately it only saves space in the Git repository (not the RPM file, or even on device when installed), I think this is a no-brainer!
    Hence, if the spec file currently uses sailfish_svg2png (I do not remember), I may really consider changing that by this PR, or a direct follow-up one.

  1. Your opinion on this?
  2. And your specific opinion on the file name(s)? I would like to settle on patchmanager-icon.svg and patchmanager-icon_ABCxABC.png. Symbolic links (we are only talking: "… in the Git repo"!) to either of these sure can be named differently if some naming convention requires this.
    Side note: For some apps it also made sense to have (the analogue of) a patchmanager-icon.png which soft-links to patchmanager-icon_86x86.png or vice versa; I intend to take a close look and come up with a suggestion.
  3. … and on the file paths: Where should the SVG and / or PNGs "real file(s)" lie, when the other locations soft-link to it?
    But answering that is intermingled with the question: Which freedoms do we have to configure the icon paths in configuration and / or code files?
    The latter is IMO the crucial point of this PR for which I hope you can find the time to provide some support (know-how, research, knowledge, etc.).

@Olf0
Copy link
Contributor Author

Olf0 commented Jul 5, 2025

O.K., we have these hits inside source code (i.e. not paths: think of grep, not find) for .svg and .png, and these hits for the same extensions in the paths of the RPM file content for all files except [b] the theme icons (i.e. a and b combined really cover all files; the same search for file paths in the source code repository is broken, see repo:sailfishos-patches/patchmanager path:/atchmanager[^\/]*\.(pn|sv)g/!).

We also have to clearly distinguish between "patch"-icon (monochrome) and Patchmanager('s) app icon (coloured).

We have these transformations configured:

  1. src/icons/icons.pro
    AFAIU, this configuration lets the PNG files of [b] ("theme icons") being generated by sailfish-svg2png from the SVG of the "patch"-icon at [src/icons/]svgs/icon-m-patchmanager2.svg.
    It also lets this SVG being copied to /usr/share/icons/hicolor/scalable/apps/, from there it is packaged by the spec file (see packaged .svg files).
    Note that this transformation is all only about the "patch"-icon, and that its source path is configurable (arbitrarily, including filename, preferably as a path relative to the directory of icons.pro) and its second output path, too.

    • Who consumes /usr/share/icons/hicolor/scalable/apps/icon-m-patchmanager2.svg?
      (The answer presumably is "Some component of SailfishOS", correct? But how does it correlate an icon with a specific app?)
  2. Section icon of src/plugin/plugin.pro
    This copies the [src/plugin/]icon-m-patchmanager.png to /usr/share/patchmanager/icons/, from where it is packaged by the spec file (see [a]). Input and output path are configurable.

    • But I cannot find any consumer of /usr/share/patchmanager/icons/icon-m-patchmanager.png?!?
  3. Section icons of src/bin/dialog/dialog.pro
    This lets the Patchmanager app icon being copied from [src/bin/dialog/]patchmanager-icon.svg to /usr/share/patchmanager/data/ where the spec file puts it into the generated RPM file.

  4. OTHER_FILES = of src/share/share.pro
    … starts with patchmanager.png and patchmanager-big.png, i.e. it consumes these files in src/share/ and likely copies them into /usr/share/patchmanager/data/ from where the spec file puts it into the generated RPM file.

    • /usr/share/patchmanager/data/patchmanager-big.png is utilised by src/qml/AboutPage.qml. This path is fully configurable and instead of an PNG a SVG might be used.
    • /usr/share/patchmanager/data/patchmanager.png is not referenced by anything and hence unused, right?

Proposals

Ad 4.: Let src/qml/AboutPage.qml use the SVG (already) at /usr/share/patchmanager/data/patchmanager-icon.svg, and clean up:

  • Delete the two files src/share/patchmanager*.png.
  • Let src/share/share.pro use src/icon/patchmanager-icon.svg (i.e. ../icon/patchmanager-icon.svg instead of the two patchmanager*.png) to copy it to /usr/share/patchmanager/data/; "yes", this is already done by step 3, but I do want to avoid ordering issues.

Ad 3.: Let the initial source (in first step) use src/icon/patchmanager-icon.svg (i.e. ../../icon/patchmanager-icon.svg) and delete the local copy of it (i.e. src/bin/dialog/patchmanager-icon.svg).

Ad 2.: If no consumer can be identified, the whole construct can be removed!

Ad 1.: Yes, I do concur that reviewing #472 (comment), #479 etc. likely helps to understand why identical copies at three locations are used. One would assume that a single SVG icon and specifying that as input for the build / deployment process (alternatively "copying right before building" in the spec file) should be feasible.


P.S.

As I do not trust GitHub's search functionality any longer (its path search appears to be outright broken, but also its code content search has flaws), I rechecked on an freshly created, local git clone:

user1@localhost:~/git/patchmanager> find $(find . -maxdepth 1 -type d -name '[^.]*') -name '*atchmanager*.png' -o -name '*atchmanager*.svg'
./src/bin/dialog/patchmanager-icon.svg
./src/icons/svgs/icon-m-patchmanager2.svg
./src/icons/svgs/icons-monochrome/icon-m-patchmanager2.svg
./src/icons/svgs/icons/icon-m-patchmanager2.svg
./src/plugin/icon-m-patchmanager.png
./src/share/patchmanager-big.png
./src/share/patchmanager.png
user1@localhost:~/git/patchmanager> grep -rE 'atchmanager[^/]*\.(pn|sv)g' $(find . -maxdepth 1 -type d -name '[^.]*')
./rpm/patchmanager.spec:PackageIcon: %{url}/raw/master/src/share/patchmanager-big.png
./rpm/patchmanager.spec:%{_datadir}/%{name}/icons/icon-m-patchmanager.png
./src/bin/dialog/dialog.pro:icons.files = patchmanager-icon.svg
./src/bin/dialog/dialog.qml:                source: "patchmanager-icon.svg"
./src/bin/dialog/patchmanager-icon.svg:   sodipodi:docname="settings-patchmanager2.svg"
./src/bin/dialog/patchmanager-icon.svg:   inkscape:export-filename="C:\Users\Stephan\Desktop\settings-patchmanager2-l.png"
./src/icons/icons.pro:svg.files = svgs/icon-m-patchmanager2.svg
./src/icons/svgs/icon-m-patchmanager2.svg:   sodipodi:docname="settings-patchmanager2.svg"
./src/icons/svgs/icon-m-patchmanager2.svg:   inkscape:export-filename="C:\Users\Stephan\Desktop\settings-patchmanager2-l.png"
./src/icons/svgs/icons-monochrome/icon-m-patchmanager2.svg:   sodipodi:docname="settings-patchmanager2.svg"
./src/icons/svgs/icons-monochrome/icon-m-patchmanager2.svg:   inkscape:export-filename="C:\Users\Stephan\Desktop\settings-patchmanager2-l.png"
./src/icons/svgs/icons/icon-m-patchmanager2.svg:   sodipodi:docname="settings-patchmanager2.svg"
./src/icons/svgs/icons/icon-m-patchmanager2.svg:   inkscape:export-filename="C:\Users\Stephan\Desktop\settings-patchmanager2-l.png"
./src/plugin/plugin.pro:icon.files = icon-m-patchmanager.png
./src/qml/AboutPage.qml:                source: "/usr/share/patchmanager/data/patchmanager-big.png"
./src/share/share.pro:OTHER_FILES = patchmanager.png \
./src/share/share.pro:    patchmanager-big.png \
user1@localhost:~/git/patchmanager> 

@Olf0
Copy link
Contributor Author

Olf0 commented Jul 6, 2025

Ad 1.: After reviewing the corresponding PRs, I do understand (again) why sailfish-svg2png comes extremely handy here:

  • It automatically generates the PNGs in Jolla's resolutions at the correct (crazy path) locations.
  • Since SailfishOS 4.6.0 it also generates these PNGs at the correct, different (crazy path) locations.

Because of that I still do not understand why we need the SVG fed into sailfish-svg2png at three locations, especially as only a single source location can be specified in src/icons/icons.pro?

Edit / P.S.: The answer is likely (as with so many Jolla tools): sailfish-svg2png does not care about specifying anything, it simply expects the SVG input file(s) at these varying locations. Hence the reference to the SVG file is likely only for the copying action, which relies on QML / QT project mechanisms (which seem to be designed much more sanely).

@Olf0
Copy link
Contributor Author

Olf0 commented Jul 6, 2025

Obviously I am missing something WRT my considerations to simplify by "Ad 1.":

Without further guidance from you, I think I will take the easy route and resurrect the directory structure under src/icons/, only replacing the original files with links.

I would also appreciate very much to receive a comment from you WRT my proposal "Ad 2.".

@Olf0
Copy link
Contributor Author

Olf0 commented Jul 6, 2025

All right, except for the unaddressed "Ad 2." things seem to be working fine again.

@Olf0
Copy link
Contributor Author

Olf0 commented Jul 6, 2025

@Olf0 Olf0 marked this pull request as ready for review July 6, 2025 18:09
Copy link
Contributor Author

@Olf0 Olf0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O.K., it builds fine again, now. The content of the generated RPM file (see there) looks fine, too.

Now, @nephros, two questions remain:

  1. Did I do the right thing WRT "Ad 2.", i.e. /usr/share/patchmanager/icons/icon-m-patchmanager.png (removing it[s copying], because it has no apparent consumers)?
  2. Is /usr/share/icons/hicolor/scalable/apps/icon-m-patchmanager.svg really necessary, i.e. does it have consumers? (This is a minor aspect of "Ad 1.".)
    Side note: It is exactly this SVG icon we export as PNG "theme icon" in various resolutions.

Edit / P.S.: I see that I missed to create a new .icons/README.md.

@Olf0 Olf0 requested a review from nephros July 6, 2025 18:19
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.

2 participants