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

Fonts too large under Wayland #7178

Closed
michaelgregorius opened this issue Mar 29, 2024 · 12 comments · Fixed by #7185
Closed

Fonts too large under Wayland #7178

michaelgregorius opened this issue Mar 29, 2024 · 12 comments · Fixed by #7185

Comments

@michaelgregorius
Copy link
Contributor

michaelgregorius commented Mar 29, 2024

System Information

KDE Plasma, Wayland

LMMS Version(s)

286d157

Most Recent Working Version

a41da81

Bug Summary

Under KDE Plasma Wayland the fonts are rendered too large. This is caused by commit 286d157. With X11 everything looks fine and of course I reviewed the pull request under X11. 😅

The previous commit a41da81 also looks good under Wayland.

The following line returns different results under X11 and Wayland:

qreal devicePixelRatio = QGuiApplication::primaryScreen()->devicePixelRatio();

Under X11 it reports 1.25 whereas under Wayland it reports 2.

Screenshots / Minimum Reproducible Project

WaylandFontProblems

@michaelgregorius
Copy link
Contributor Author

Pinging @Rossmaxx, as I cannot assign him directly...

@michaelgregorius
Copy link
Contributor Author

@Rossmaxx, the underlying problem likely is that Qt already takes the DPI, scaling factors and whatever into account when a point sized font it set. This means that on a HiDPI display a 12pt font will be rendered with a higher number of vertical pixels than on a display with low DPI.

Multiplying the requested font size with the device pixel ratio interferes with Qt's mechanism and it will get worse the higher the device pixel ratio is.

@Rossmaxx
Copy link
Contributor

I don't have a Wayland system so can't test a fix. I don't really wanna revert the pr either as I'm preparing LMMS for qt6 support. I would like to hand this over to someone who has a Wayland system.

@Rossmaxx
Copy link
Contributor

Asked chatgpt, here's what it said (i know it might be inaccurate but for some clues)

Screenshot_2024_0330_095101

@Rossmaxx
Copy link
Contributor

So looks like some discrepancy between qt and wayland

@michaelgregorius michaelgregorius changed the title Too large fonts under Wayland Fonts too large under Wayland Mar 30, 2024
@michaelgregorius
Copy link
Contributor Author

Hi @Rossmaxx,

I am now starting to wonder why the scaling is done in the first place in the pointSize function? If a widget sets the point size in points then it is somewhat in "I know what I am doing" mode and it must be implemented such that it does not matter how many physical pixels the text rendering uses.

So I wonder if the scaling can be removed completely? In that case the function would only act as a delegate to setPointSizeF with some added convenience because it directly returns an adjusted QFont instance.

It's also a bit of a question which cases LMMS wants to support and how. I did some experimentation with the "Global (fractional) scaling" with a small Qt application run under Plasma. This type of scaling really scales everything:

  • Font sizes (even if specified in points)
  • Images
  • Window sizes

As the name implies everything is scaled globally according to the scale factor. So if an application opens with a window size of 100x100 with a global scaling of 1.0 it will open with a size of 150x150 with scaling set to 1.5.

I am not sure though how font rendering at point sizes is affected by monitors with different DPIs values. If both monitors are configured to a global scaling factor of 1 would a font with a point size of 18 have the same "physical" height on both screens? In that case the DPI values would affect the pixel height of both fonts which would then be scaled by the global scaling factor.

It seems that the following rules apply with regards to LMMS:

  • Widgets that set their font size in points must ensure that they do not use hard coded widths, heights and widget placements, i.e. they should for example use layouts.
  • Widgets that set their font sizes in pixels and that use hard coded widths, heights and widget placements will only scale and be discernible if users apply a global scaling.

It seems that everything that looks off in the attached screenshot uses point sized fonts with hard coded widths, heights and widget placements.

@Rossmaxx
Copy link
Contributor

I do agree that we should be using layouts instead. I refactored the function instead just to get the ball rolling and also, I'm not really familiar with the codebase and qt to know what changes to really make. At this point, half the code i write is suggested by chatgpt (which i modify to the context and my preferences) + i do look at the docs too.

Can we try removing the pointSize calls and see what happens?

@Rossmaxx
Copy link
Contributor

So I wonder if the scaling can be removed completely? In that case the function would only act as a delegate to setPointSizeF with some added convenience because it directly returns an adjusted QFont instance.

Or would it be better to use setPointSizeF directly?

@michaelgregorius
Copy link
Contributor Author

If we removed the calls to pointSize completely then I think many components would render with the default font size of the system (or with the font size inherited by the parent widgets). So this would likely look wrong.

Perhaps we should try to remove the scaling in pointSize and see how it behaves with monitors with different DPIs.

Alternatively, if the previous solution with QDesktopWidget worked in all scenarios then we could also try to find out if it is possible to implement it in the same way for X11 and Wayland using Qt6's API.

Deviations in the new implementation

The new implementation does not behave the same way as before on my system. My monitor actually has a DPI around 117 which means that the scaling factor is around 1,25 (117/96 ~ 1,22). However, QApplication::desktop()->logicalDpiY() returns 96 under X11 for me (although the information in the EDID is correct). This means that the scaling factor is effectively 1 on my system with the old code.

With the new code QGuiApplication::primaryScreen()->devicePixelRatio() returns 1.25 on my system (under X11). This means that fonts are now scaled by 1.25 instead of 1 as before. As was already noted under Wayland that method returns 2 for my setup.

Calls to setPixelSize?

By the way, I could swear that there have been much more calls to setPixelSize in the past. Has this been removed at some point or am I just remembering wrong?

@michaelgregorius
Copy link
Contributor Author

It all seems to boil down to the question of which of these model(s) LMMS wants to support:

  • Global scaling
  • Only fonts are scaled

Assumptions

Let's assume that we have an application that displays the following elements:

  • Raster images
  • Text that's defined in pixels
  • Text that's defined in points

Global scaling

This scenario seems to be the simplest to support. Because global scaling "simply" scales all of the above users can use the global scaling to make applications look good and legible. If the application looks good with a scale factor of 1 on a 96 DPI display it will look the same with a scale factor of 2 on a 192 DPI display.

In this scenario it is even OK to use QFont::setPixelSize because fonts in pixel sizes are also scaled. The same applies for images. So if you have an icon with a text that's sized in pixels underneath it, it will not look minuscule after scaling is applied.

Open question

What about text defined in points on different DPI screens?

Only fonts are scaled

This model can be though of as: "A global scaling of 1 is set but the default font sizes are increased until they can be read". This might also be considered the mode of systems that do not support global scaling.

With this model raster images and text that's defined in pixels will become smaller and smaller the more the DPI increases. Text that's defined in points should work but needs a design that can accommodate for this (no fixed sizes, etc.).

LMMS still has quite some places where fixed sizes are assumed, e.g. when certain home-grown widgets are painted. If point sized text is used in these places then at certain DPIs it will grow too large and will be clipped (like in the screenshot above). If pixel sized fonts are used they will always fit in the widget but will get minuscule on high DPI screens.

Conclusions

The "Only fonts are scaled" model does not work for HiDPI displays because it breaks down for raster images, fixed size paint operations and pixel fonts. It should therefore not be pursued.

Global scaling on the other hand seems to work if the following rules are followed:

  • Widgets that use font sizes in points must be able to accommodate for text with different pixel heights. They must not use hard coded widths, heights and widget placements. They should use layouts instead of hard coded placements.
  • Widgets that set their font sizes in pixels can use hard coded widths, heights and widget placements because they will scale with global scaling and will be discernible.

With global scaling the way forward to fix the problems with HiDPI would be to either use flexible, scalable widgets that can take different font sizes as preferred by the user or use hard coded widgets with text sizes set in pixels.

@Rossmaxx
Copy link
Contributor

So you mean to say that we should be using setPointSize directly where possible. I'll try to get a pr rolling soon, which removes the entire gui_templates.h file and move stuff around.

@michaelgregorius
Copy link
Contributor Author

I have opened a question in #2510 (see comment) which asks if all of the targeted systems have a notion of global (fractional) scaling which scales everything as described above. If that was the case then a first fix might even be to use QFont::setPixelSize in all widgets that do fixed-size drawing and hard-coded widget moves, etc.

If people would then complain about images and texts being too small then we'd have to ask them to use a system that supports global fractional scaling, e.g. Plasma X11 or Wayland or other desktop environments and windows managers under Wayland.

I think it is not feasible to support all of the different scenarios, e.g. the "Only fonts are scaled" one.

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

Successfully merging a pull request may close this issue.

2 participants