-
Notifications
You must be signed in to change notification settings - Fork 23
Add new SVG icon, along with PNG exports #514
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
base: master
Are you sure you want to change the base?
Conversation
For its original source, see https://codeberg.org/marbalf/Sailfish-icons/issues/3 Signed-off-by: olf <Olf0@users.noreply.github.com>
@nephros, I still have to overhaul / adapt the But:
Code locations which reference these two icon file names: P.S.: Luckily and nicely |
"Namen sind Schall und Rauch" ;) So we actually have two icons in use:
IIRC the theme location is used by Jolla Settings to find its icons - any other place will not work. As for naming the I would vote to keep that name for the Theme Icon to stay aligned with all the other Settings apps ( |
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 |
Now, this one. AFAICS that is used nowhere in the application itself. There is a file at patchmanager/src/qml/AboutPage.qml Line 66 in d000636
Then we have https://github.com/sailfishos-patches/patchmanager/blob/d0006367cc276bf2e3739f7e2830f0df4ea3de28/src//bin/dialog/dialog.qml#L200 which uses Also, using SVG files as image sources in our old QML needs the (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:
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. |
To start with the easy part, because that is mostly already done, yesterday:
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).
… is just a URL pointing to wherever we decide to put the SVG icon file proper to.
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 There is actually one more location I remembered:
We have a whole lot of directly exported PNGs (86², 108², 1282, 172², 256², 480², 560² square-pixels) of the new icon in the
Oh, I really wonder why the GitHub-search Edit: I did the counter-check by looking for the same RegEx matching to file contents (instead of paths): 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 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.
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?
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.
No, I sure do not intend to cause any regressions, even if they are not noticeable for most.
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²:
|
O.K., we have these hits inside source code (i.e. not paths: think of We also have to clearly distinguish between "patch"-icon (monochrome) and Patchmanager('s) app icon (coloured). We have these transformations configured:
ProposalsAd 4.: Let
Ad 3.: Let the initial source (in first step) use 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 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
|
Ad 1.: After reviewing the corresponding PRs, I do understand (again) why
Because of that I still do not understand why we need the SVG fed into Edit / P.S.: The answer is likely (as with so many Jolla tools): |
Obviously I am missing something WRT my considerations to simplify by "Ad 1.":
I would also appreciate very much to receive a comment from you WRT my proposal "Ad 2.". |
All right, except for the unaddressed "Ad 2." things seem to be working fine again. |
There was a problem hiding this 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:
- 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)? - 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
.
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