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

fix gui scaling regressions for high DPI displays #2296

Merged
merged 3 commits into from Jan 4, 2016

Conversation

Projects
None yet
4 participants
@MarcSabatella
Copy link
Contributor

MarcSabatella commented Nov 21, 2015

See discussion in https://musescore.org/en/node/87861, specifically my comments at https://musescore.org/en/node/87861#comment-388486 and below.

Now that scores are being scaled to physical screen resolution, we don't need the "-x" option to perform that function, so I have removed guiScaling from the calculation of physicalDotsPerInch. And now that we have a good value for physicalDotsPerInch, I have also added code to set a reasonable default value of guiScaling (still needed for icons, palette elements, and window sizes, since they continue to be recorded in physical pixel dimensions).

The bottom line is that things now scale correctly for me on my high DPI display on Ubuntu, and I believe my changes should be safe for standard resolution displays. However, I am not sure what effect my changes will have on Macs with "retina" displays, since that OS apparently does some scaling magic that allows the hard coded pixel dimensions to work correctly in most places. If someone with a retina Mac can test my changes I would greatly appreciate it. If the result is that icons, palette elements, and example views are now too large, we can ifdef the code in musescore.cpp that sets the default value for guiScaling, so it is always set to 1.0 for Mac rather than being scaled to physicalDotsPerInch. Or maybe there is a different value thaty is more appropriate to use for Mac, like maybe screen->logicalDotsPerInch().

One other thing my code does not get right: while the scaling of example views is correct - so the note groups in Time Signature Properties are sized properly - the initial size of the widget is not. The changes I made to ExampleView::sizeHint() seem like they should be correct, but it seems sizeHint() is being ignored here unelss I change the size policy. Not sure what changed exactly here, since I think this was working earlier. But in any case, it seems we should look at this in conjunction with #2288. My changes here should be good in any case.

@lasconic

This comment has been minimized.

Copy link
Member

lasconic commented Nov 22, 2015

On a Mac with retina display with your changes
capture d ecran 2015-11-22 11 35 06

Without
capture d ecran 2015-11-22 11 29 37

In 2.0.2
capture d ecran 2015-11-22 11 30 07

@lasconic

This comment has been minimized.

Copy link
Member

lasconic commented Nov 22, 2015

I understand that now everything has the same size at scale 1.0 but unfortunately, it looks less good than before :-(

@lasconic

This comment has been minimized.

Copy link
Member

lasconic commented Nov 22, 2015

It "works" on windows too (not retina). Meaning everything is at the same size, but it doesn't look better.

left 2.0.2 - right master + this PR

_1rtq5lsu0vi3_ziohhmezfybgoofpe8iz5z_oktx7o

@MarcSabatella

This comment has been minimized.

Copy link
Contributor Author

MarcSabatella commented Nov 22, 2015

Hmm, that's not what I expected to see, but still, I consider this mostly good news. There are definitely things not quite right. Obviously, the wrong dimensions for the example view widget, which I see on my system as well and can continue to look into. But also, I think the palette is slightly too big - the clefs should be at 80% unless you have customized your palette properties. So something is going wrong there, I will see if I can reproduce.

BTW, the "-x" option still works, same semantics as before, except it no longer affects the score view - just icons, palettes, and example view. The idea being it is "GUI" scaling, not score scaling - we already provide zoom for that. So, with my changes, you'd probably get something pretty similar to 2.0.2 if you ran with "-x 1.5" or some such.

@MarcSabatella

This comment has been minimized.

Copy link
Contributor Author

MarcSabatella commented Nov 22, 2015

I can verify the discrepancy in palette sizes on my system. If a palette is set to 1.00 scale, it actually shows up somewhat bigger than the score. It seems this was actually the case for me in 2.0.2 as well, but I failed to notice it for some reason. If I use "-x 1.73" (which sets the score to be the correct scale on my system), then the palette elements are all bigger than the score unless the palette is scaled down (as the clefs palette normally is). So the problem must be in my original palette scaling code. Could be that it turns out to be off by a factor of 96/72, I guess.

I will look into this, then hopefully figure out how to get the dimensions of the example views correct.

@MarcSabatella

This comment has been minimized.

Copy link
Contributor Author

MarcSabatella commented Nov 24, 2015

Looking into this further, I'm kind of confused by the non-Retina Windows image from 2.0.2. If you look at the Key Signatures palette - which is supposed to be scaled at 100% - how does it compare to the sizes in My First Score? And do you know the actual DPI of the display on that machine?

From what I can tell, my guiScaling code is actually doing the right thing, and the size of the palette icons is actually "correct" now. The palette is explicitly in globals.h to a spatium of 1.9mm (with a built in assumption the display is around 100 DPI). So the icons set to 100% scaling - like key signatures - are supposed to be bigger than a standard score, which would have a spatium of 1.764mm.

Anyhow, with my changes, the sizes of the palette icons do seem correct for a spatium of 1.9mm. If we don't like the way the palette looks, we could reduce the palette spatium, or maybe play with with individual palette magnifications. The clefs, for instance, are already set to 80% as opposed to the key signatures which are at 100%.

I still need to figure out what the dimensions of the example view aren't correct.

@MarcSabatella MarcSabatella force-pushed the MarcSabatella:gui-scaling branch from 78b818e to b9837f7 Nov 24, 2015

@MarcSabatella

This comment has been minimized.

Copy link
Contributor Author

MarcSabatella commented Nov 24, 2015

I updated the commit to set PALETTE_SPATIUM to 1.764mm, and make the assumption of 96dpi more explicit (while continuing to correct for displays whose resolution differs from this assumption). I thnk this makes sense, personally. We could then consider updating the mag factors for individual palettes.

@Jojo-Schmitz

This comment has been minimized.

Copy link
Contributor

Jojo-Schmitz commented Nov 24, 2015

see my PR #2298 for a possible fix to that Travis error ;-)
It just got merged, so a rebase should do...

@lasconic

This comment has been minimized.

Copy link
Member

lasconic commented Nov 24, 2015

On Mac retina, the palette looks better but the example view is still less good than in 2.0.2 (right).

capture d ecran 2015-11-24 16 19 02

Looks ok on Windows
capture d ecran 2015-11-24 16 52 10

@Jojo-Schmitz

This comment has been minimized.

Copy link
Contributor

Jojo-Schmitz commented Nov 24, 2015

The score itself too looks too small

@lasconic

This comment has been minimized.

Copy link
Member

lasconic commented Nov 24, 2015

@Jojo-Schmitz In 2.0.2 (right) the score was too small indeed but it's ok in master+this PR (left)

@Jojo-Schmitz

This comment has been minimized.

Copy link
Contributor

Jojo-Schmitz commented Nov 24, 2015

Hmm, in 2.0.2 100% of an A4 score is almost the width of a real A4 (less than 1cm to narrow), and master is way smaller on my screen, with and without this PR (the palettes are much smaller with this PR).

@MarcSabatella

This comment has been minimized.

Copy link
Contributor Author

MarcSabatella commented Nov 24, 2015

The issue with the size of the score on Jojo's system (and one of cadiz1's, apparently) I don't understand. To be clear, I didn't change anything about that, except to make score size independent of use of "-x". So something about the DPI change itself must be causing that issue, "some" Windows systems only, apparently. Wish my Windows computer were still operational. guess I might have to bite the buller and get another.

Meanwhile, I will be looking at the example view issue today.

What seems to be the case is that the sizeHint is being ignored, and the calculation Qt makes as to how big it needs to be is not working correctly either. That is, I can do anything I like in sizeHint and nothing changes. But if I run with "-x", which scales the content of the example view, then the widgets does change size - it's just never the right size.

I also notice that in the windows screenshot, it looks "OK" but not "correct" - the notes in the example view should be displaying at the same scale as notes in the score. At least that was my intent, and it works that way on Linux and seems to be on Mac.

@MarcSabatella MarcSabatella force-pushed the MarcSabatella:gui-scaling branch from b9837f7 to bc53dfa Nov 24, 2015

@MarcSabatella

This comment has been minimized.

Copy link
Contributor Author

MarcSabatella commented Nov 24, 2015

I rebased then added a commit implementing something for the scaling on Windows 7 where QScreen::physicalDotsPerInch() apparently does not return anything useful. I don't know that this is really the right thing to do, but I figure it doesn't hurt to put it out there and let people try it out.

@Jojo-Schmitz

This comment has been minimized.

Copy link
Contributor

Jojo-Schmitz commented Nov 25, 2015

Looks good to me, esp. the Win7 fix, results in the same sizes as in 2.0.2 and before the DPI changes.

Well, only the Win7 fix, the other causes a crash, division by 0 in palette.h line 189, hgrid being 0

Edit:
Sorry, my fault, I manually applied the patch and failed...

@MarcSabatella MarcSabatella force-pushed the MarcSabatella:gui-scaling branch from bc53dfa to 3957c59 Dec 3, 2015

@MarcSabatella

This comment has been minimized.

Copy link
Contributor Author

MarcSabatella commented Dec 3, 2015

I can't say I understand why it doesn't work to set the sizePolicy for ExampleView to "Preferred", but "Minimum" seems to work well on my system. "MinimumExpanding" also works, but on my system anyhow, Qt seems to give it extra space unnecessarily by default. The advantage of using "MinimumExpanding" would be that the example would grow if you resize the dialog, but in theory, this is not necessary, since this should be getting the dimensions big enough now.

wschweer added a commit that referenced this pull request Jan 4, 2016

Merge pull request #2296 from MarcSabatella/gui-scaling
fix gui scaling regressions for high DPI displays

@wschweer wschweer merged commit e842ec0 into musescore:master Jan 4, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.