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

Numbers are cut off on bottom of Spectrum Analyzer #5088

Closed
michaelgregorius opened this issue Jul 22, 2019 · 12 comments
Closed

Numbers are cut off on bottom of Spectrum Analyzer #5088

michaelgregorius opened this issue Jul 22, 2019 · 12 comments

Comments

@michaelgregorius
Copy link
Contributor

The numbers at the bottom of the spectrum analyzer are cut off if used on a display with a higher DPI:
SpectrumAnalyzerNumbers

This is caused by the hard coded width and height in SaSpectrumView::drawGrid:

float label_width = 24;
float label_height = 15;

One solution is to measure the strings you want to write using QFontMetrics and then make space and position the labels accordingly.

@he29-net
Copy link
Contributor

Hi,
this is actually caused by Qt handling font sizes in a rather inconsistent way with certain window managers. Something similar also happens to me (I'm using Xfce) -- the problem is not that the label boundary is too small, but that the font is rendered too big. You can also see it on the menu bar -- the font should be much smaller, matching the track names for example.

What happens, as far as I can tell, is that Qt first looks up the screen DPI and it is applied to correctly scale all elements (labels, buttons, windows, ...), say, by 1.5. Then Xfce tells it "hey, I've got this custom 1.5 scale for fonts, you should use it". And it does -- in my case, the extra scaling is even applied to buttons and other things, in your case the "damage" seems to be somewhat limited.

Running lmms as:
QT_FONT_DPI=96 ./lmms
solves the issue for me, but I'm a beginner with Qt so I'm not quite sure how a proper fix for the issue would look like.. #4919 may be related to this in some way as well.

@michaelgregorius
Copy link
Contributor Author

Hi @he29-net,

I still think that you can use QFontMetrics to find out how large the font would be rendered. Here's what I did to test this. I extended SaSpectrumView::drawGrid with the following code block at the beginning:

QFontMetrics fm = painter.fontMetrics();
QSize fsize = fm.size(Qt::TextSingleLine, "01234567890k");
int fheight = fsize.height();

This more or less asks the painter: "If I use your current font to render the string 01234567890k what would be its height in pixels?" I have used "01234567890k" because these seem to be the only characters that are potentially rendered at the bottom of the display.

Then I set a breakpoint after the fheight statement and started the application in "normal" mode. Here the height was reported as 23 in the debugger. After that I used the QT_FONT_DPI variable to set the DPI to 96. Now the height was reported as 18 (this is still bigger than label_height = 15 by the way).

So I still think that the value of label_height must not be hardcoded but determined dynamically. If you want to try to implement it like this and want to test it just set QT_FONT_DPI to a higher value. :) I think my display would be represented if you set the DPI to 117.

@he29-net
Copy link
Contributor

The width and height is already interpreted dynamically: when you tell Qt to draw text in a box of size 24x15 pixels, that is in the "device independent pixels". Qt then uses the devicePixelRatio to scale it up to the correct size -- the text boundary should scale along with the text. In your case, the multiplier should be 117 / 96 = 1.22, meaning that the actual pixel size of the boundary on your display will be 29x18, which would be matching the height reported by your debugger.

In this specific instance, the boundary size of the text is not the problem -- as you can see, all the other labels on Y axis are all right, despite using the same boundary size. I think the problem is that the bottom of the text is simply outside of the SaSpectrumView widget; the rest would be the QSplitter margin. So the only way how to work around this would be to to somehow detect that the double-scaling bug occurred and shrink the display area vertically so that the text fits within the displayable area.

That's not really a solution, that's just a hack that makes the code more complicated for no reason. This is how LMMS looks at my screen (~175 DPI) without setting the QT_FONT_DPI variable:
screen
So I'm quite reluctant to add unnecessary code that somehow remedies one occurrence of a problem that already makes the rest of the application a mess..

@michaelgregorius
Copy link
Contributor Author

Should be fixed with #5098. I have tested the fix with different settings for QT_FONT_DPI and QT_SCALE_FACTOR and it worked in all cases.

@he29-net
Copy link
Contributor

he29-net commented Dec 4, 2019

This should now be fixed in recent builds that use Qt 5.6, which fixes (at least some of) the scaling issues.

The fonts are still obviously fixed size, but that should not be an issue -- scaling the font size and other GUI elements with DPI is the job of Qt, it just needs to actually do it properly, which it now apparently does. :)

@JohannesLorenz
Copy link
Contributor

@he29-net Do you want to close this issue?

@he29-net
Copy link
Contributor

I think it can be closed, unless someone knows of an instance where the problem still occurs. I believe the issue was fixed by the Qt upgrade, and I would expect scaling support to only get better and more stable in future Qt versions, not worse.

There is also the WIP PR that re-writes the UI to be immune to the cut-off problems (although it won't prevent Qt from using bad font size), which can still link to this issue for reference even after it was closed.

@michaelgregorius
Copy link
Contributor Author

The problem is still present on my machine:
5088-CutOffWithNewQt

This was compiled with Qt 5.14 by the way and I still believe that it is not a problem with Qt. If you paint something in the paint methods of Qt it will paint what you tell it to.

The open pull request 5098 shows that the problem can be solved and how.

@he29-net
Copy link
Contributor

If you paint something in the paint methods of Qt it will paint what you tell it to.

Well, apparently not, which is the problem. I tell it to draw text of certain size in "Qt logical pixels", which is a valid thing to do if you want to make any sort of pixel-perfect, fixed-size interface (which is also defined in Qt logical pixels). It works as expected most of the time, but it seems that in some cases it messes up the font size vs. DPI adjustment and scales the text to about double the size that it should be. That is a Qt problem: imagine the text was drawn over a bitmap background -- this bug makes such a use-case completely unusable, since you can't dynamically adjust parts of the bitmap to fit whatever text size Qt decides to draw.

The fact that the numbers are cut off at the bottom is only a symptom; the underlying problem is that the numbers are huge, which is causing other problems (like 10k and 20k labels nearly overlapping).

I think there are at least three environment variables that affect Qt scaling (probably many more), and all the window managers and platforms could be setting them to any combination of values, which is likely causing the issues. The AppImage should be forcing them to "sane values", so LMMS releases should hopefully behave well. But I'm not sure if the underlying issue even can be fixed for source builds, as we have no control over the environment variables. So perhaps mitigating the issue by fixing some of the symptoms like in #5098 is the best we can do, I don't know...

@PhysSong
Copy link
Member

PhysSong commented Dec 9, 2020

I'm running everything with QT_AUTO_SCREEN_SCALE_FACTOR=1 and the analyzer draws fine. I'm on OpenSUSE Tumbleweed, Qt 5.15, 221DPI built-in monitor.

@PhysSong
Copy link
Member

PhysSong commented Dec 9, 2020

If I manually break LMMS scaling with QT_AUTO_SCREEN_SCALE_FACTOR=0 QT_SCREEN_SCALE_FACTORS=1.5, I can see the issue, BTW.

@michaelgregorius
Copy link
Contributor Author

Closing this issue as I cannot reproduce it anymore even when using QT_AUTO_SCREEN_SCALE_FACTOR=0 QT_SCREEN_SCALE_FACTORS=1.5 (see @PhysSong's comment above).

Here's how it looks with that configuration on my machine:
SpectrumAnalyzerLabelsOK

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

No branches or pull requests

4 participants