Add missing icon sizes
ClosedPublic

Authored by ngraham on Sep 13 2017, 10:10 PM.

Details

Summary

Add missing scalable icon sizes

BUG: 384473

Test Plan

Add missing scalable icon sizes at index.theme. If not all size are covered by scalable icons some desktop environment other than PLASMA may show icons not properly scaled.

See also bug #384473 for more detailed explaination and screenshot
https://bugs.kde.org/show_bug.cgi?id=384473

Update:
From the MATE Desktop environment you may use 'Appearance' (the binary is (/usr/bin/mate-appearance-properties) to modify its look-and-feel. There is not a Breeze theme listed there here but from the 'Theme' tab you may click the 'Customise...' button and from the new window you may select 'Breeze' in 'Controls', 'Pointer' and 'Icons' tabs.

Then restart MATE to see the applied changes. Here is how MATE desktop appears with Breeze icons before the changes.

As you may see (scalable) icons from Breeze theme are bigger than others (which are .svg icons too). In caja application this is even more evident especially for 'computer' icon

After I inspected the caja code I found it uses scalable icons and I found the problem is Breeze does not provide for some icon sizes for some scalable icons required by caja so it uses a bigger sized icon (all works fine with MATE default theme, of course). So I modified the Breeze index.theme as reported here and ,as you may see from the screenshots below, all icons are of the right size

desktop

caja

I used the fixed Breeze theme both in PLASMA and LXQt and I not noticed any differences from before to after so I believe my changes are not dangerous for other DE.

Diff Detail

Repository
R266 Breeze Icons
Lint
Lint Skipped
Unit
Unit Tests Skipped
mandian created this revision.Sep 13 2017, 10:10 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 13 2017, 10:10 PM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
mandian planned changes to this revision.Sep 13 2017, 10:11 PM
mandian requested review of this revision.Sep 16 2017, 6:13 PM
cfeck added a subscriber: cfeck.Oct 4 2017, 12:42 PM
ngraham added a subscriber: ngraham.Oct 4 2017, 5:02 PM

Thanks for the patch, Mandian! Can you mention some details of your testing process in the Test Plan section, and maybe attach some before-and-after screenshots? Also make sure this change doesn't cause any regressions when running software in a non-MATE (i.e. Plasma) environment.

mandian edited the summary of this revision. (Show Details)Oct 10 2017, 10:01 PM
mandian edited the test plan for this revision. (Show Details)
mandian planned changes to this revision.Oct 10 2017, 10:07 PM

Hi ngraham
I added some more details and screenshots to test plan.
Please let me know is more is needed.

mandian requested review of this revision.Oct 10 2017, 10:08 PM

This seems okay to me, but I'm pretty new and I'd really like sign-off from someone more experienced first.

ngraham edited the summary of this revision. (Show Details)Oct 23 2017, 1:50 AM
ngraham accepted this revision.Oct 23 2017, 1:53 AM

I ran with this change for a day in stock Neon and couldn't notice any regressions (or any changes at all, actually).

VDG folks, any objections? If I don't hear anything before Nov 1, I think I'll commit this.

This revision is now accepted and ready to land.Oct 23 2017, 1:53 AM
hein added inline comments.Oct 30 2017, 7:19 AM
icons/index.theme
297 ↗(On Diff #19506)

Hmm why lower this one:

Well the issue is that they are not "scalable" - I mean the goal should rather be to make them functionable on all sizes but being SVG's means that they may become misaligned when rendered. (Ie if you make an icon made for one size slightly bigger or smaller it will risk suddenly becoming blurry).

If @andreask thinks its ok as a risk I am all for it - we should keep in mind though to ensure that all icons have wide breadth of sizes and if possible have a follow up on all icons used and which ones are "just resized" and which are done in the correct sizes.

It like changes for size of icons in MimeTypes is not more needed because the bug is in caja and not in Breeze theme. Do you prefer I submit a new patch without these two changes?

icons/index.theme
297 ↗(On Diff #19506)

I did this because caja fails to load the right size for icons in MimeTypes. After a further inspection actually I think it is due to a bug in caja and not in Breeze Icon Theme so this and the following modifies can be dropped out.

Feel free to modify this patch.

ngraham requested changes to this revision.Nov 11 2017, 9:55 PM
This revision now requires changes to proceed.Nov 11 2017, 9:55 PM
mandian updated this revision to Diff 22206.Nov 11 2017, 10:13 PM

I cleaned the patch and I included icon-dark too.

ngraham accepted this revision.Nov 11 2017, 10:15 PM

Looks good to me now. @hein?

This revision is now accepted and ready to land.Nov 11 2017, 10:15 PM
hein accepted this revision.Nov 12 2017, 7:59 AM

Same.

Do you need someone to commit this for you or do you have access?

Just as a heads up, check whether the icons become fuzzy in certain sizes on Mate for example

Even if simplified icons may look better with small size I did not notice any fuzzy effects in smaller icon.

I can commit this. @mandian, can you provide a full name? As for an email address, is mandian@openmailbox.org good?

Cool! Just please keep an eye out. Pixel-aligning icons is a PITA but worthwhile so if there is something that ends up blurry please ping

@ngraham the mail is the one you know but please do not expose it in a public page (I'd like to avoid spam bots). The name I use is the same you know too (I use this to sign the packages I build for OpenMandirva).

@jensreuterberg sure! I'll ask for help on this to the OpenMandriva Artwork Team.

ngraham added a comment.EditedNov 13 2017, 10:23 PM

I'm afraid all commits require a full name and a real, public email address. Take a look at the git logs and you'll see. They all have real names and email addresses. This is for copyright purposes, I believe (but I am no copyright expert). An alternative is for someone else to commandeer the diff and commit it entirely under their own name (I would be willing to do this). You would renounce your copyright in this case.

You would renounce your copyright in this case.

If it is only a matter of copyright yes, of course :).

This is a patch I wrote when I was packaging MATE desktop. What I only care is it works well :).

ngraham commandeered this revision.Nov 14 2017, 3:00 AM
ngraham edited reviewers, added: mandian; removed: ngraham.

Yoink.

This revision was automatically updated to reflect the committed changes.

Great!
Thank you all!