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

svg viewbox fix #12407

Merged
merged 5 commits into from
Dec 9, 2023
Merged

svg viewbox fix #12407

merged 5 commits into from
Dec 9, 2023

Conversation

m0dB
Copy link
Contributor

@m0dB m0dB commented Dec 7, 2023

Adjusted the Latenight palemoon and classic btn__*.svg so they render correctly on retina + some changes by @ronso0 to the qss files.

Almost all files were done automatically (with a perl script), multiplying the width and height by two and inserting viewBox with the non-multiplied width and height. The btn__fx_selector_down.svg and btn__xfader_deck_*.svg and btn__xfader_aux_*.svg were done manually.

This replaces #12364 and fixes #12361

@m0dB m0dB changed the base branch from main to 2.4 December 7, 2023 23:23
@m0dB m0dB requested a review from ronso0 December 7, 2023 23:30
@m0dB m0dB requested a review from ywwg December 7, 2023 23:32
@ronso0
Copy link
Member

ronso0 commented Dec 7, 2023

@Swiftb0y Do you have any idea why eslint kicked in?

It seems github-labeler also broke...

@JoergAtGithub
Copy link
Member

Something really strange happend to my decks with this PR:

Aufzeichnung.2023-12-08.010501.mp4

@ronso0
Copy link
Member

ronso0 commented Dec 8, 2023

Oh oh, didn't remove the debug commit before merging #4710
I'll fix it.

@ronso0
Copy link
Member

ronso0 commented Dec 8, 2023

Reverted in 2.4, no merging 2.4 to main (still building).

@ronso0
Copy link
Member

ronso0 commented Dec 8, 2023

@m0dB Please rebase onto 2.4
(which should also fix the countless skin warnings)

@ronso0
Copy link
Member

ronso0 commented Dec 8, 2023

and please pick c83b125 to fix the library radio buttons in Classic theme

@ronso0
Copy link
Member

ronso0 commented Dec 8, 2023

LGTM @100% on Ubuntu with Qt 5.12.8

@m0dB
Copy link
Contributor Author

m0dB commented Dec 8, 2023

rebased & cherry-picked

@ronso0
Copy link
Member

ronso0 commented Dec 8, 2023

@m0dB Btw, how do Deere and Tango look like?

@m0dB
Copy link
Contributor Author

m0dB commented Dec 8, 2023

Deere has only the sync master icon at non-retina resolution:

Screenshot 2023-12-08 at 16 57 35

Tango has more issues:

Screenshot 2023-12-08 at 16 56 21 Screenshot 2023-12-08 at 16 56 17 Screenshot 2023-12-08 at 16 56 50 Screenshot 2023-12-08 at 16 56 32 Screenshot 2023-12-08 at 16 56 28

If you can give me a list with the corresponding svgs files, I can run my script on them to see if it solves the issue without further tweaking.

@m0dB
Copy link
Contributor Author

m0dB commented Dec 8, 2023

But I would prefer to merge this first. Deere and Tango can be done in a follow-up PR.

@ronso0
Copy link
Member

ronso0 commented Dec 8, 2023

Sure, I was just curious because IIRC in Deere most icons were 48px or 32px squares. And in Tango they're all 1:1 (SVG:screen pixels).

@ronso0
Copy link
Member

ronso0 commented Dec 8, 2023

@JoergAtGithub This is looking good on Windows with both 100% app scaling and higher, and with Mixxx' scaling?

@JoergAtGithub
Copy link
Member

There are scaling issues with CPU-Load-Meter (here 125% Windows Zoom):
grafik
With 175% Windows zoom this white bar on the right appears and the text looks strange:
grafik
150% Windows Zoom
grafik
100% Windows zoom + 400% Mixxx zoom looks fine:
grafik

Inverse Loop-In/Loop-Out marker icons in Palemoon seam to be the wrong ones
https://github.com/mixxxdj/mixxx/assets/64457745/7457a2e6-6789-4aee-a5e5-9d4f88a461d5

Otherwise all looks fine on Windows11.

@m0dB
Copy link
Contributor Author

m0dB commented Dec 8, 2023

There are scaling issues with CPU-Load-Meter (here 125% Windows Zoom):

I don't think that is related to this PR.

@ronso0
Copy link
Member

ronso0 commented Dec 8, 2023

Inverse Loop-In/Loop-Out marker icons in Palemoon seam to be the wrong ones
https://github.com/mixxxdj/mixxx/assets/64457745/7457a2e6-6789-4aee-a5e5-9d4f88a461d5

Oh! I'll send a fix.

@m0dB
Copy link
Contributor Author

m0dB commented Dec 8, 2023

Inverse Loop-In/Loop-Out marker icons in Palemoon seam to be the wrong ones

Indeed, but also not due to this PR. But I will fix it here.

@m0dB
Copy link
Contributor Author

m0dB commented Dec 8, 2023

Oh! I'll send a fix.

Thanks, @ronso0 , I'll leave it to you!

@m0dB
Copy link
Contributor Author

m0dB commented Dec 8, 2023

The issue with the OpenGL widget leaving this white border was already reported for the spinnies btw. Maybe this can be avoided by using only sizes that are multiple of 4? Or maybe we can set a different colour than white so it is not as noticable?

@ronso0
Copy link
Member

ronso0 commented Dec 8, 2023

Here it is: b83eb57
I noticed that some files contain a few invisible objects. and many unused attributes.

@ronso0
Copy link
Member

ronso0 commented Dec 8, 2023

@m0dB when yo have picked that feel free to press merge yourself

@daschuer
Copy link
Member

daschuer commented Dec 9, 2023

Did you have an eye on these issues?
grafik

The issue has changed compared to the #12364 version where an other icon was effected:
grafik

Can we use the #12364 version for the two?

Did you onside the dark lower part of the numbers? This is no mayor issue to me ..

@m0dB
Copy link
Contributor Author

m0dB commented Dec 9, 2023

Not sure what goes wrong with the intro/outro icons, but I have recreated the end icons from the start icons. I can't check this myself, on retina they look fine.

@m0dB m0dB merged commit 0c55229 into mixxxdj:2.4 Dec 9, 2023
10 checks passed
@ronso0
Copy link
Member

ronso0 commented Dec 9, 2023

Thank you.
I think the intro/outro ramps are rendered like that because the angles are not 45° (the cathete ratio is 9/10), so it is somewhat random how the rounding is applied with fractional scaling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants