Skip to content
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

Scaled svg cache #13679

Merged
merged 4 commits into from
Oct 19, 2024
Merged

Scaled svg cache #13679

merged 4 commits into from
Oct 19, 2024

Conversation

daschuer
Copy link
Member

This is on top of #13678

It is used to demonstrates a scaled SVG cache.

@JoergAtGithub Can you benchmark that on your side. With manual testing I do not notice an improvement.
There are currently test debug messages, to trace how it works. they need to be removed before benchmarking.

When we have confirmed, I will streamline the whole class to properly consider the device pixel ratio everywhere.

@JoergAtGithub
Copy link
Member

I just tried it, and can confirm, that this solves the issue as well!

@daschuer
Copy link
Member Author

The initial booting takes longer with caching enabled:

debug [Main] Stat("SkinLoader::loadConfiguredSkin","count=1,sum=2.023e+09ns,average=2.023e+09ns,min=2.023e+09ns,max=2.023e+09ns,variance=0ns^2,stddev=0ns")
debug [Main] Stat("SkinLoader::loadConfiguredSkin","count=1,sum=2.06473e+09ns,average=2.06473e+09ns,min=2.06473e+09ns,max=2.06473e+09ns,variance=0ns^2,stddev=0ns")

Tested with LateNight Classic

@daschuer
Copy link
Member Author

It turns out that the Apple conditions are no longer relevant in Qt5 after we have removed the custom scaling.
Ready for review.

@JoergAtGithub
Copy link
Member

This crashes for me reproduceable, if I switch to Shade Summer Sunset or Dark. While other Skins including Shade Classic work as expected:
Unhandled exception at 0x00007FFC1D0CF6FE (ucrtbase.dll) in mixxx.exe: Fatal program exit requested.
grafik
grafik
This does not happen with 2.4 ( tested after merge of #13678 ), so I guess something with the color correction code of this PR is wrong.

@daschuer
Copy link
Member Author

Good catch. A narrowing the painter scope fixes the issue. I have no idea why it does not crash when starting with Shade Sunset or Shade Dark. Maybe by luck?

@daschuer
Copy link
Member Author

Ah got it. Without the last commit it crashes here all the time, no luck anymore ... good.

@daschuer
Copy link
Member Author

Done.

@JoergAtGithub
Copy link
Member

Works as expected now! Thank you! Code looking good to me!

@JoergAtGithub JoergAtGithub changed the title POC: Scaled svg cache Scaled svg cache Sep 22, 2024
@JoergAtGithub JoergAtGithub added this to the 2.4.2 milestone Sep 22, 2024
@daschuer
Copy link
Member Author

Thank you for review. Merge?

@JoergAtGithub
Copy link
Member

I think we need a test with a Mac and the high-DPI Retina-Display as well. @fwcd / @m0dB could you give this PR a try?
Expected is a less blurry graphic with Shade skin (please test with all three color schemes).
It would be also interesting how the SVG from pre #12407 behave now. I could imagine, that all platforms behave here identical now.

@daschuer
Copy link
Member Author

There are only a few SVG buttons in Shade.
The battery indicator and the Auto DJ buttons are SVGs and may benefit in Hidpi mode.

@m0dB
Copy link
Contributor

m0dB commented Sep 27, 2024

Not quite sure what I am looking at, but the auto-dj button does not use retina for sure.
Screenshot 2024-09-27 at 22 15 55

@m0dB
Copy link
Contributor

m0dB commented Sep 27, 2024

And in LateNight palemoon I get this:

Screenshot 2024-09-27 at 22 19 50

@daschuer
Copy link
Member Author

daschuer commented Oct 7, 2024

The LateNight scaling issue should be fixed with the last commit.
The Auto DJ buttons are looking good here:
grafik
grafik

It turns out that these buttons are rendered via qss and not via the Paintable. @m0dB: Can you confirm if you also see blurry buttons with the 2.4 branch?

@JoergAtGithub
Copy link
Member

It turns out that these buttons are rendered via qss and not via the Paintable. @m0dB: Can you confirm if you also see blurry buttons with the 2.4 branch?

There seems to be related changes in Qt 6.8:
grafik

@ronso0
Copy link
Member

ronso0 commented Oct 11, 2024

This will unfortunatly not be available in the next relevant Ubuntu version.
If we stick to the plan that the latest Ubuntu LTS defines the minimum Qt version we can rely on, we'll have
Qt 6.2.4 in Mixxx 2.5, and Qt 6.6.2 in Mixxx 2.6.
Maybe 26.04 will use Qt 6.8, this is roughly 2 years from now : \

@daschuer
Copy link
Member Author

Ok, this means we can merge this safety?

With QSS SVGs we have now discovered two issues:

  • Repeated disc access when hoovering on all targets
  • Blurry pictures on macOs

So maybe it is worth to hack around the Qt limitations

@m0dB
Copy link
Contributor

m0dB commented Oct 19, 2024

I confirm that the qss buttons look blurry with 2.4. This branch does not improve that (but IIRC that is expected?)

I confirm that the scaling issue that was introduced with Latenight is solved.

@JoergAtGithub
Copy link
Member

Great! Thanks for testing again!

@JoergAtGithub JoergAtGithub merged commit 6c97ded into mixxxdj:2.4 Oct 19, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants