Fix svg rendering on macOS retina

[m0dB]

This fixes #12361 by upscaling the btn__ svgs with a factor 2, by multiplying to document size of each btn__ svgs by two and inserting a 2x scale transform, for LateNight palemoon and classic. (This way this could be automated, rather than opening and resizing each svg manually)

It is surprising that this is necessary, but apparently Qt renders these svgs, when rendering with stylesheets, at the document size in pixels and only scales down, never up. I have been looking for a code-based solution but to no avail.

Apparently this is not necessary on windows / linux, so it looks like the HDPI handling is different then on macOS. I expect that this upscaled svgs have no negative effect on windows / linux, but please confirm.

Latenight palemoon:

Before:

With upscaled svgs:

Latenight classic:

Before

After

[JoergAtGithub]

On Windows 11 it still looks fine. I didn't notice a change by this PR.

[Swiftb0y]

This is a workaround, isn't it? We should file a bug report with them. Does the issue persist in Qt6 builds?

[JoergAtGithub]

Qt5 seems to be different here than Qt6: https://doc.qt.io/qt-5/highdpi.html

Did you had a look at

mixxx/src/main.cpp

Line 145 in 2c2e706

QGuiApplication::setHighDpiScaleFactorRoundingPolicy(

Maybe these settings can influence the behavior.

[m0dB]

This is a workaround, isn't it?

Yes.

We should file a bug report with them. Does the issue persist in Qt6 builds?

I still need to try this.

[m0dB]

Qt5 seems to be different here than Qt6: https://doc.qt.io/qt-5/highdpi.html

Did you had a look at

mixxx/src/main.cpp

Line 145 in 2c2e706

QGuiApplication::setHighDpiScaleFactorRoundingPolicy(

Maybe these settings can influence the behavior.

IMO these flags are all set as they should and changing them didn't help.

[ywwg]

this may be a workaround for a QT bug but I think it's a fine workaround and worth merging in to fix the issue

[ywwg]

looks good in linux

[daschuer]

The rendering is done here:
https://github.com/qt/qtbase/blob/da6e958319e95fe564d3b30c931492dd666bfaff/src/widgets/styles/qstylesheetstyle.cpp#L1402
It is done form a QIcon with a list of pre-rendered QPixmaps in different sizes.
https://github.com/qt/qtbase/blob/c5ba3ac95cb7dc157a4d84ecc32cdadd7d45d387/src/gui/image/qicon.cpp#L268
Is can be populated from style sheets with the @ prefix for different scaling.

This means we have no real benefit from the scaling of a svg. It is treated like png, just with the extra penalty of rendering from svg. Your current solution looks good for 2x but not for other scaling and comes with a performance penalty for smaller (non 2x) screens.

I am not sure how the situation is with the "BackPath" skin property. Did you check that?
Like here:

mixxx/res/skins/Tango/mixer/mixer_main.xml

Line 25 in 2c2e706

<BackPath>skin:../Tango/knobs_sliders/knob_bg_main.svg</BackPath>

Will it work? What do you think?

[m0dB]

To be clear: I would much prefer a solution at the c++ level, but I really don't know what else to try, also after looking at the Qt code.

This means we have no real benefit from the scaling of a svg. It is treated like png, just with the extra penalty of rendering from svg.

We definitely do have benefit from rendering svgs, and they do scale properly! If you look at the screenshot @JoergAtGithub posted in reply to my bug report, you'll see that with 400% scaling the icons are rendered perfectly.

As far as I can see, this issue occurs because the device pixel ratio (2 on macOS retina) is not taken into account. But normal scaling works fine.

BTW, I tried setting the iconSize of the style option to 2x but that didn't make any difference. But note that I am not very confident with the styling code, so maybe I got that wrong.

Your current solution looks good for 2x but not for other scaling

Why do you say that? Do you have examples of that? Other devs report no visual change.

and comes with a performance penalty for smaller (non 2x) screens.

That might be true. (Proposed solution below)

I am not sure how the situation is with the "BackPath" skin property. Did you check that? Like here:

But this would require changing a lot of skin code, wouldn't it? I don't want to go there.

Proposed alternative solution:

Since this only is a problem on macOS, and on mac retina displays are common enough that a potential performance penalty is not relevant, and seeing that I know which files need to be adjusted (btn_* expect btn__embedded*), instead of modifying the original files, I could add a step that modifies the files when the are copied to the resource, only on macOS. That way we don't "polute" the original svgs, and the workaround is restricted to macOS.

[daschuer]

First of all, I have no strong demands which solution we go. I just want to make sure we have taken a decision with all informations on the table.

@m0dB would you mind to test with one knob, if the unscaled svg is blurry removing it from the style and adding it to the skin?
I think it is here:

mixxx/res/skins/LateNight/style_classic.qss

Line 1732 in 2c2e706

image: url(skin:../LateNight/classic/buttons/btn__arrow_left_down.svg) no-repeat center center;

mixxx/res/skins/LateNight/controls/button_1state_right.xml

Line 17 in 2c2e706

<PushButton>

We definitely do have benefit from rendering svgs, and they do scale properly!

The issue is that they are rendered and scaled with their native size to a QPixmap in QIcon, then they are sacaled from that to the final size. This is the reason we see blurriness whenever an upscaling or an odd number scaling happens.

I think with "BackPath", we render directly to the final size. So we safe CPU and memory for one rendering.

But this would require changing a lot of skin code, wouldn't it? I don't want to go there.

Yes, that is the issue. I can imagine that all the scaling done here was already tedious.
So let's first check if the "BackPath" solution really solves the issue as well. Than we have a clear view.

[m0dB]

With BackPath the svg renders at the correct resolution:

[ronso0]

I am not sure how the situation is with the "BackPath" skin property. Did you check that?

This means we'd have to drop the convenient qss solution and set all icons in xml, including potential scheme suffix :|

With BackPath the svg renders at the correct resolution:

The drawback of setting BackPath is obviously that it doesn't center the icon and allows no additional position correction like in qss.
Also, BackPath only works for WPushButton and other Mixxx widgets, but not for QComboBox drop-down indicators, QSpinBox buttons etc.

@m0dB

instead of modifying the original files, I could add a step that modifies the files when the are copied to the resource, only on macOS. That way we don't "polute" the original svgs, and the workaround is restricted to macOS.

if that is feasible it very much appreciated and I'd definitely prefer that over the skin changes, either SVG or xml.
Could you give that a shot?

[daschuer]

The issue is clearly sourced from the Qt. It has the same code in Qt 5 and 6. So all targets should be affected. This means we need a solution for all.

Can we apply the scaling hack at runtime?
This sounds adventures....

We could patch Qt ...

Or just go with the proposed solution here ..

[ywwg]

Since this is a release blocker, trying to patch QT would be much too slow (and would still have a problem on older versions). We should report a bug upstream to be sure, but a workaround is more appropriate for now.

[ronso0]

Since this is a release blocker

Hmm? @m0dB is it looking good in Mixxx 2.3?

[ywwg]

By release-blocker, I mean that we should not release 2.4 with the GUI looking wrong in macOS on HiDPI, do you agree?

[ywwg]

ah, I misunderstood from another screenshot, this is a cosmetic issue but doesn't cause misplaced / missized svgs. So this is not a blocker, just a refinement. (Still seems ok to include in 2.4 imho)

[m0dB]

Right.

So my plan is this:

(only if APPLE)

• replace my extremely dirty perl script that modifies the problematic svgs and write something cleaner in python

• add instructions to the cmakelist to run my script and output the modified svgs in a folder in the build directory

• when running from the build directory, add this folder to the skin search path, so these modified svgs take precedence

• in the cmake install step, copy the modified svgs so they end up in the installation

I think that should work just fine.

I will try to do this later this week. In the mean time, it might be good to merge this so it gets some exposure and we detect it if it causes any graphical artifacts (I didn't see any though), and revert it once I have my dynamic solution in place.

[daschuer]

Don't we need the dynamic solution on the users machine? We don't know which scale factor the user has.

[daschuer]

Is adding both with a @2x prefix an option?

[ywwg]

Can we test this with the basic 2x blowup and a 150% scale on a mac? I suspect it will look fine but it's worth checking. I would prefer not to pre-render every possible scaling setting.

[JoergAtGithub]

I tested on 400% and it looked good. Can you confirm this?

[ronso0]

Here it's looking good at 200% and 400% (though Qt icons in the preferences look blurry, checkboxes, radio buttons, up/down arrows etc.)

So we have

• broken
  • macOS (Qt version @m0dB ?)
  • Windows (version? Qt version?)
• good
  • Windows 11 (official vcpkg?)
  • Ubuntu 20.04, Qt 5.12.8

[m0dB]

I can confirm the same blurriness on windows and Linux as well at 200 % scaling:

is that what or without this PR?

[daschuer]

This branch looks good like this at 200 %

At 300 % still good.

AT 125 % (The default of windows Full-HD Laptops) it is blurry:

Some icons are not scaled:

[daschuer]

is that what or without this PR?

Yes #12364 (comment)

All screenshots in #12364 (comment) are with this PR.

I am in doubt that the Qt version and the OS makes a big difference, because the related QT code has not been touched.

The difference is that unlike Windows, macOS has only integer scale factors by default. But we can use any scaling from the Mixxx preferences. So form this perspective only a direct rendering of the svg into the target size pixmap will be a full solution.

[m0dB]

Is adding both with a @2x prefix an option?

You mean postfix, right? It doesn't seem to open the @2x file at all.

[ronso0]

I assume @daschuer refers to this, but that is never called (or is it called by Qt?). Even though I vaguely remember discussing this high-res image solution I never tested it IIRC. Is it only for Mixxx widgets? Maybe it's broken and no one noticed..

mixxx/src/widget/paintable.cpp

Line 278 in 8e41e53

QString Paintable::getAltFileName(const QString& fileName) {

On the Qt side there is https://doc.qt.io/qt-6/scalability.html mentioning @2x, though IIUC it refers to images in the qrc file.
And https://doc.qt.io/qt-5/qicon.html#addFile, not sure if that is called when setting images via stylesheets.

[daschuer]

Ok, I think we have now discussed most aspects.
This PR is definitely an improvement, but unfortunately still does not render the SVG directly to the desired size.

So we have the choice to merge this and accept the remaining edge cases and the extra CPU on start up or replace it with the "BackPath" + center solution where possible.

A macOS only upscaling on our build server seems to be not required.

What do you think?

[m0dB]

I got a bit lost with all the observations. So there is no visual degradation on non-retina setups?

In any case, I do see a simple solution to modify the path to the svgs on the fly: LegacySkinParser::stylesheetAbsIconPaths
We could have the 2x svgs in a separate directory and where required (aka on macOS), in this function we could change the path in the style automatically.

On the long run, moving the svgs out of the style seems to be best solution, but I find it hard to say how much work that would be on a qss/xml level, and if everything now done in the styles could be done with the xml tags, and if not, what would need to be added.

[daschuer]

All targets share the same problem:

• qss SVGs are first rendered in their original size and than scaled to their target size
• Even on macOs we have fractional scaling like 125 % with the help of the spin box in the interface preferences.

On the long run, moving the svgs out of the style seems to be best solution

This gives the best visual result and performance.
The costs are, that not every Image can be moved, we loose convenience with maintenance in one place, and it requires extra work and testing.

[ronso0]

In any case, I do see a simple solution to modify the path to the svgs on the fly: LegacySkinParser::stylesheetAbsIconPaths

👍
This can be applied to external style sheets and <Style> attributes in xml.
Note that icons are also set in default.qss, picked from the qrc file, so we'd also need to consider

• other paths, e.g. url(:/images/
• other properties qproperty-icon: url(:/..

I got a bit lost with all the observations.

I think we mixed up native app scaling (macOS retina, Windows 125% @daschuer mentioned) and Mixxx' internal scaling (which I tested, and @JoergAtGithub as well?)

On the long run, moving the svgs out of the style seems to be best solution, but I find it hard to say how much work that would be on a qss/xml level, and if everything now done in the styles could be done with the xml tags, and if not, what would need to be added.

It is not possible to set all icons in xml, for example AFAIK scrollbars, the treeview decoration and the buttons in the library panes (AutoDJ, Recording, Analyze, Missing, Hidden) and menu checkboxes.
And, as I mentioned, there are drawbacks in terms of flexibility when setting images in xml.
I just can't believe Mixxx is the only app affected by this, hence there must be another fix available.

I'm reluctant to invest that much work into a skin system we plan to replace anyway.

[m0dB]

(Deleted, I got it)

[m0dB]

I have put all 2x scaled svgs in a subdirectory 2x/ (at the level of the original file) and in LegacySkinParser::stylesheetAbsIconPaths I check for these files and use them when the file exists. If we want to make the usage of these files conditional, we can do that easily now.

Does this work? Should this be macOS only or no need?

[ronso0]

Thank you. I don't see any impact on startup time caused by the scaling SVG -> pixmap with my i7, still floating around 6s. Would be good to know how this affects low-spec machines. @daschuer could you test with your eeePC? If there's a noticeably increased startup time it should be conditional.

Some small issues: icons set on QWidgets need the size fixed in qss to be scaled correctly.
Without fixed size (current state):

fixed size, one dimension is sufficient, aspect ratio is maintained, smaller dimension is dominant appearantly
ronso0@5b48d4e

Still loking good: spinboxes in decks and library, all menus (main menu, decks, library), AutoDJ controls, tracks table in-line editors, effect selectors.
This is probably a last-minute fix so anyone testing this please check this thoroughly on your machine, too.

[ronso0]

AutoDJ controls

They're also too big, don't know why that looked okay earlier.
I'll provide another commit.

[ronso0]

I've set the scaling to 200% in Mixxx and that also draws oversized AutoDJ icons. Probably doesn't mean anything because I didn't see the initial bug with Qt 5.12.8 🤷

@m0dB Are the AutoDJ control icons rendered correctly in 2.4 for you?
If yes, they need to be excluded from this fix. Because, appearantly setting the width/height for those QPushButtons doesn't have any effect on the icon size. And qproperty-iconSize doesn't affect the image, only qproperty-icon. But I didn't manage to set the icons for both checked/unchecked with qproperty-icon, it's a mystery... (oh there are some related old, unresolved Qt bugs..)

If no, hmmm..

[m0dB]

Thanks for dedicating some time to this, @ronso0 . I will merge your commit that fixes the size.

As for the AutoDJ icons: I see what you see: on 2.4 they are rendered without non-retina resolution, with the 2x scaled svgs they are rendered too big.

[m0dB]

What is strange though: on the autodj buttons the icons are too big, but they are rendered using the retina dpi.

[daschuer]

I cannot measure a significant difference in start-up time and memory usage. Maybe the original rendering is only a temporary.

So we may pick a size, that is a common denominator of all common scalings.
I think on windows it is
100 % 125 % 150 % 200 %

4x would work, need to check.

We "just" need to make sure that the final rendering is exactly 1/4 of the source. Otherwise we will se blurry again at different scaling. This is unfortunately not yet the case everywhere.

[m0dB]

Okay, I am finding something very interesting: these images have both width and height and viewBox. I am experimenting with 2x/btn__autodj_enable_off.svg : When I remove the <g transform="scale(2)"> , the image renders correctly. If I double the viewBox width and height, the image becomes half the size. It might be that with the correct combination of width, height and viewBox values we don't need the scaling at all?

(Edit: viewBox, not viewport, duh)

[m0dB]

Right! For example btn__broadcast_off.svg renders fine with:

<svg width="132" height="32" viewBox="0 0 66 16" version="1.1" xmlns="http://www.w3.org/2000/svg">

and no need to insert the <g transform="scale(2)">

originally:

<svg width="66" height="16" version="1.1" xmlns="http://www.w3.org/2000/svg">

This might work everywhere! I can't look into this right now, but this might also make the fixed sizes you added to the qss unnecessary.

[daschuer]

issues at 125 % using this branch:
(gray bottom)
(starecase)
Interestingly they are not visible at 250 % even if it is also not a integer fraction.

Maybe we can stick with 2x for all and just tweak the original svg for better double scaling.

[m0dB]

replaced by #12407