Skins :: fix qss icons with kIconThemes 5.80 #3902

ronso0 · 2021-05-24T00:28:19Z

1

missing skin icons due to broken kIconThemes https://www.ubuntuupdates.org/pm/kiconthemes were fixed
BUT that lib forces all image: ... icons to 1:1 aspect ratio while especially LateNight uses different formats.

revival of #3863

For those affected by https://bugs.launchpad.net/mixxx/+bug/1922966
(Arch, Ubuntu 21.04 with kiconthemes installed)

the background shorthand is the only one-line way to achieve the previous look
(alternative: separate background-image, background-position and background-repeat lines...)

 #Hotcue1 WPushButton[displayValue="1"][dark="false"] {
-  image: url(skin:/palemoon/buttons/btn__1_active.svg) no-repeat center center;
+  background: url(skin:/palemoon/buttons/btn__1_active.svg) no-repeat center center;
 }

BUT since we omit background-color transparent bg color is assumed, thus all button background colors must be moved after setting the icons.

In ALL skin

all icons show up with appropriate size
all button states have the correct background color
the logo in the launch screen is sized correctly

2

76db8ba
(included here because don't want to spend the time to rebase. this PR will be finished soon anyway)
In Tango the template and icon paths are skin:../Tango/buttons/... to allow reusing them for Tango64.
After #3877 those paths need to be skin:/../Tango/buttons/... so they're correctly resolved to skins/Tango/buttons/...

3

24c3eaf
use 'background-image' for the default launch image style

ronso0 · 2021-05-24T00:35:54Z

ToDo

double check Shade
fix LateNight Classic
check Deere (little affected at most)

ronso0 · 2021-05-24T23:34:19Z

done.

Holzhaus · 2021-05-24T23:38:36Z

Thanks. I can test this tomorrow, but who can verify that the fix works? That requires testing by someone who can reproduce the issue in the first place. @ywwg? @daschuer?

ronso0 · 2021-05-24T23:44:43Z

yes @ywwg in case he didn't patch the icon lib, yet.

I tested this myself with Ubuntu 21.04 and that kIconlib installed. This is mostly about making the LateNight main interface look good and Tango usable again.
there are a few minor glitches left with some expand/collapse icons in Deere for example, or some library preview buttons,..
but the effort fixing those is not worth it -- considering that only a few distros are affected and that lib will hopefully be fixed in a few weeks.

ywwg · 2021-05-25T01:03:18Z

The only issue I see is no bpm lock icon or library triangles on deere 64. Otherwise good!

daschuer · 2021-05-25T21:14:39Z

I can confirm that the launch image is no longer squeezed.
The search bar icon is also fixed.
Cue/Play/pause fixed.

The preview button is still squeezed.
The effect button is still broken.

How about creating a PR with only the known working fixes?

I have just contacted the package maintainers Rick and Jose again. Maybe we can solve the root cause with them.
And live with the remaining issues until than.

ronso0 · 2021-05-26T21:54:30Z

Deere64 icons are now visible with the path fix used for Tango already url(skin:/icon/any.svg > url(skin:/../Deere/icon/any.svg

ready!

I have just contacted the package maintainers Rick and Jose again. Maybe we can solve the root cause with them.

They simply need to update the package in the ubuntu main repo to 82. It's already in kubuntu-backport
https://www.ubuntuupdates.org/pm/kiconthemes

And live with the remaining issues until than.

Only issue I'm aware of are squeezed icons / enforced to 1:1 and not scaled: <, >, table sort arrows and the previewbutton in the library. Yes, we can live with that..

daschuer · 2021-05-26T22:54:31Z

Can you revert the non working attempt before merge?

ronso0 · 2021-05-26T23:16:38Z

sure, I can squash commits per skin tomorrow.

ronso0 · 2021-05-26T23:17:41Z

it's not non-working, it's two separate steps btw

daschuer · 2021-05-26T23:51:19Z

There is no need to squash anything. I just want make sure not to have changes that do not improve the situation but have an regression risk.
During my test I cannot confirm that the preview column is fixed, but there are related changes. The same is true for the effect buttons.

ronso0 · 2021-05-27T12:54:25Z

I checked again: the preview and bpm icons are there in all skins.
we didn't you see it?

daschuer · 2021-05-27T13:41:39Z

The preview icon is there but squashed the same goes for the effect combo box.

I have a plain Ubuntu Hirsute with the faulty library installed. I can't reproduce the icon missing issue, only the squashing.

ywwg

looks good!

…SVG icons with kIconThems_5.80

…icons with kIconThems_5.80

…cons with kIconThems_5.80

…icons with kIconThems_5.80

ronso0 · 2021-05-28T02:30:39Z

I can't reproduce the icon missing issue, only the squashing.

that was a regression caused by #3877 affecting Deere64 and Tango64 where (incorrect/incomplete) icon urls in the base skin were not properly resolved. see PR description

The preview icon is there but squashed the same goes for the effect combo box.

To re-cap the consequneces of that broken lib:
previously, we used mostly image for all skin icons

all SVG icons would be displayed and scaled properly, regardless if they're oversized (icon_4px.svg) or not
the icon size would be taken from the actual size of the SVG, the resizable Preview button the icon (~ 2:1 aspect ratio) would be scaled depending on the column width

now, (with broken kIconThemes)

relative paths for SVG icons were ...ignored? thus provide absolute paths with Skins: transform qss icon urls into absolute paths #3877
image always assume an aspect ratio of 1:1 (distort all non-square icons) thus use background now, where we accept the consequences:
background: color icon repeat position-x position-y does actually scale icons to fit Mixxx widgets -- but that doesn't work for Qt sub-controls, like WEffectSelecotor::down-arrow or QHeaderView::up-arrow, there they're used at real size.
That does matter Deere where almost all icons are 48x48 pixles. So drop-down arrows, sort arrows etc. would be huge when using background there. with image they're only sueezed.

For the preview button I now decided to go with the fixed-size icons with correct aspect ratio set in default.qss (included at compile time, icons in mixxx.qrc) which will not grow when the library font is increased
instead of a auto-scaling icon which is squeezed (square).

I don't want to spent hours with resizing icons for a skin I don't care about tbh
So, yes, as you said: there are issues users with libKIcon..80 need to live with until that lib is finally updated

Re squeezed effect combobox: please post an example screen.

daschuer · 2021-05-28T05:53:21Z

I like to merge this asap. It is kind of unfair to put so much work on you (us), just because the package maintainers are not able to upload the fixed version to the Hirsute Repro.

My original concern was not to commit half baked fixes that will hit us later once the library is updated.

If you can confirm that this is not the case let's just merge this and carry on. Do you?

daschuer · 2021-05-28T05:59:17Z

res/skins/Deere/style.qss

  border: 1px solid transparent;
  border-radius: 2px;
 }

 QPushButton#LibraryPreviewButton:!checked {
-  image: url(skin:/icon/ic_library_preview_play.svg);
+  /* TODO ronso0 Restore once fixed lib linKIconThemes (vers. <..80) is in *ubuntu main repos */
+  /*image: url(skin:/../Deere/icon/ic_library_preview_play.svg);*/


Does this case a regression for all?
If so I prefere to revert this. The same applies to the other todos.

this is the only way to get the Preview icons undistorted and adpating to the column width (though not growing with the row height). It's the same for all skins now, and special button styles are not re-implemented, thus the TODOS

so Yes, a regression. with the fixed lib image should work as it did before, and I will revert that asap after the patched libs are in main repos.

daschuer · 2021-05-28T07:04:47Z

I have now copied the 5.82 version to our PPA.
Issue fixed :-/

Which changes from this PR are still relevant?

ronso0 · 2021-05-28T13:57:14Z

My original concern was not to commit half baked fixes that will hit us later once the library is updated.

The fixed lib should restore the previous working status of image, not screw it up even more.
There are no halfbaked fixes, and I don't expect any regressions -- unless the fixed lib scres up background, too. but the patch is only about SVG and relative paths, so if I undertstood it correctly the fixed/restired lib shouldn't interfere with our skin stylesheets anymore at all. Basically we could revert the entire PR, but 99% of the changes make skins future-proof.

There are a few places (icons in Deere only IIRC, as well as the Preview button) where we should revert the fixes afterwards to make it look good again.

Ready.

ronso0 · 2021-05-28T14:12:56Z

I have now copied the 5.82 version to our PPA.
Issue fixed :-/

This is not a fix -- it's just a mirror of the kubuntu ppa.
To make use of that/or our ppa users need to

complain about it somewhere (best case Mixxx forums)
someone knowing about the reason AND the fix has to be around, and then I'd rather recommend them adding the kubuntu ppa instead

As explained this makes skins more immune against future broken libs, and we should revert the few changes marked with TODO soon. I have this package on the radar and will do so asap.

daschuer · 2021-05-28T18:17:11Z

. ronso0 ***@***.***> schrieb am Fr., 28. Mai 2021, 16:13:

…

I have now copied the 5.82 version to our PPA. Issue fixed :-/ This is not a fix -- it's just a mirror of the kubuntu ppa. To make use of that/or our ppa users need to - complain about it somewhere (best case Mixxx forums) - someone knowing about the reason AND the fix has to be around, and then I'd rather recommend them adding the kubuntu ppa instead As explained this makes skins more immune against future broken libs, and we should revert the few changes marked with TODO soon. I have this package on the radar and will do so asap. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3902 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AANR6ITVTFHYVV57QMI5Z4LTP6QHTANCNFSM45MHRDPA> .

ronso0 · 2021-05-28T19:25:24Z

@daschuer

From your last response only

.

is visible.

Ready for merge?

daschuer · 2021-05-28T21:15:02Z

The PPA solution is a automatic fix for all PPA users.
However, it did not work well, because of missing dependencies of the new package.
I think we need a patched 5.80 version, instead of a full 5.82 version that depends on others.
I have deleted that package form our PPA.

daschuer · 2021-05-28T21:18:53Z

So let's merge this now. Thank you for developing the fix.

uklotzde · 2021-05-30T20:21:09Z

Some merge conflicts with main need to be resolved.

ronso0 · 2021-05-30T21:12:30Z

I'll take care of that tomorrow.

ronso0 · 2021-05-30T23:53:07Z

Done.

coolerminds · 2021-05-31T03:59:40Z

awesome!

return to 'image: url(..) no-repeat center center;' for most skin icons to avoid unscaled, blurry icons when Mixxx is scaled >100% with the 'background: ..' shorthand at the cost of a lot of squeezed icons for (Linux) users still having broken libKIconthemes5 5.80 installed.

because with scale factors >100% the (vector) icons set with 'background[-mage]' are not truely scaled up, only their 100% pixmap, which results in blurry/ pixelated icons. Users still with broken libKIconThemes5 v5.80 have to live with squeezed icons until they (can) update that lib.

return to 'image: url(..) no-repeat center center;' for most skin icons to avoid unscaled, blurry icons when Mixxx is scaled >100% with the 'background: ..' shorthand at the cost of a lot of squeezed icons for (Linux) users still having broken libKIconthemes5 5.80 installed. A patched version of that lib is in the Mixxx 'stable' ppa.

partially revert #3902 Skins: fix qss icons with kIconThemes 5.80

github-actions bot added the skins label May 24, 2021

ronso0 added this to the 2.3.0 milestone May 24, 2021

ronso0 force-pushed the skins-qss-icons branch 2 times, most recently from 8d1bb8c to 24c3eaf Compare May 24, 2021 23:28

ronso0 marked this pull request as ready for review May 24, 2021 23:34

uklotzde added the blocker label May 26, 2021

ywwg self-requested a review May 27, 2021 23:05

ywwg approved these changes May 27, 2021

View reviewed changes

ronso0 added 7 commits May 28, 2021 04:29

LateNight: replace 'image' with 'background' to work around squeezed …

e824681

…SVG icons with kIconThems_5.80

Shade: replace 'image' with 'background' to work around squeezed SVG …

6e696a1

…icons with kIconThems_5.80

Tango: replace 'image' with 'background' to work around squeezd SVG i…

34dfa09

…cons with kIconThems_5.80

Tango: adjust relative icon paths

8cc9236

Deere: replace 'image' with 'background' to work around squeezed SVG …

950c8ee

…icons with kIconThems_5.80

Deere: adjust relative icon paths

bbcc804

launchimage: use 'background-image' for default style

0414ad9

Skins: use default library Preview icon (set in default.qss)

6f5cc7d

ronso0 force-pushed the skins-qss-icons branch from 9eabe4e to 6f5cc7d Compare May 28, 2021 02:29

daschuer reviewed May 28, 2021

View reviewed changes

daschuer merged commit 41999e0 into mixxxdj:2.3 May 28, 2021

ronso0 deleted the skins-qss-icons branch May 28, 2021 21:29

ronso0 added a commit that referenced this pull request Jun 1, 2021

Merge pull request #3931 from ronso0/revert-qss-icon-fix_single

bfdbabb

partially revert #3902 Skins: fix qss icons with kIconThemes 5.80

mixxxbot mentioned this pull request Aug 23, 2022

Missing SVG skin icons with libKF5IconThemes5 5.80 #10390

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Skins :: fix qss icons with kIconThemes 5.80 #3902

Skins :: fix qss icons with kIconThemes 5.80 #3902

ronso0 commented May 24, 2021 •

edited

Loading

ronso0 commented May 24, 2021 •

edited

Loading

ronso0 commented May 24, 2021

Holzhaus commented May 24, 2021

ronso0 commented May 24, 2021

ywwg commented May 25, 2021

daschuer commented May 25, 2021

ronso0 commented May 26, 2021

daschuer commented May 26, 2021

ronso0 commented May 26, 2021

ronso0 commented May 26, 2021

daschuer commented May 26, 2021

ronso0 commented May 27, 2021

daschuer commented May 27, 2021

ywwg left a comment

ronso0 commented May 28, 2021

daschuer commented May 28, 2021

daschuer May 28, 2021

ronso0 May 28, 2021

daschuer commented May 28, 2021

ronso0 commented May 28, 2021

ronso0 commented May 28, 2021

daschuer commented May 28, 2021 via email

ronso0 commented May 28, 2021

daschuer commented May 28, 2021

daschuer commented May 28, 2021

uklotzde commented May 30, 2021

ronso0 commented May 30, 2021

ronso0 commented May 30, 2021

coolerminds commented May 31, 2021

Skins :: fix qss icons with kIconThemes 5.80 #3902

Skins :: fix qss icons with kIconThemes 5.80 #3902

Conversation

ronso0 commented May 24, 2021 • edited Loading

1

2

3

ronso0 commented May 24, 2021 • edited Loading

ToDo

ronso0 commented May 24, 2021

Holzhaus commented May 24, 2021

ronso0 commented May 24, 2021

ywwg commented May 25, 2021

daschuer commented May 25, 2021

ronso0 commented May 26, 2021

daschuer commented May 26, 2021

ronso0 commented May 26, 2021

ronso0 commented May 26, 2021

daschuer commented May 26, 2021

ronso0 commented May 27, 2021

daschuer commented May 27, 2021

ywwg left a comment

Choose a reason for hiding this comment

ronso0 commented May 28, 2021

daschuer commented May 28, 2021

daschuer May 28, 2021

Choose a reason for hiding this comment

ronso0 May 28, 2021

Choose a reason for hiding this comment

daschuer commented May 28, 2021

ronso0 commented May 28, 2021

ronso0 commented May 28, 2021

daschuer commented May 28, 2021 via email

ronso0 commented May 28, 2021

daschuer commented May 28, 2021

daschuer commented May 28, 2021

uklotzde commented May 30, 2021

ronso0 commented May 30, 2021

ronso0 commented May 30, 2021

coolerminds commented May 31, 2021

ronso0 commented May 24, 2021 •

edited

Loading

ronso0 commented May 24, 2021 •

edited

Loading