Bug 414266 - Certain (not all) QQC2 buttons have vertically stretched and pixellated text when fractional display scaling is used
Summary: Certain (not all) QQC2 buttons have vertically stretched and pixellated text ...
Status: RESOLVED UPSTREAM
Alias: None
Product: frameworks-qqc2-desktop-style
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: 5.64.0
Platform: Arch Linux Linux
: HI normal
Target Milestone: ---
Assignee: Marco Martin
URL:
Keywords:
: 405145 414265 431955 432095 433554 434402 434484 457326 457629 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-11-18 14:16 UTC by Patrick Silva
Modified: 2023-01-10 22:13 UTC (History)
16 users (show)

See Also:
Latest Commit:
Version Fixed In: Qt 5.15.3


Attachments
screenshot (81.83 KB, image/png)
2019-11-18 14:16 UTC, Patrick Silva
Details
Button with diagonal seam due to 125% scaling (1.15 KB, image/png)
2021-01-12 10:55 UTC, nyanpasu64
Details
Video of a Qt Quick 2 button glitching when resized, with patched qqc2-desktop-style (187.11 KB, video/mp4)
2021-01-14 10:11 UTC, nyanpasu64
Details
Buttons with horizontal line offset at 150% scaling (74.67 KB, image/png)
2022-10-30 21:18 UTC, Gerhard
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Silva 2019-11-18 14:16:27 UTC
Created attachment 123989 [details]
screenshot

STEPS TO REPRODUCE
1. set display scale factor to 1.2 in system settings > display and monitor
2. restart plasma session
3. open system settings > Audio

OBSERVED RESULT
"Default device" buttons has badly rendered font, see the attached screenshot.


EXPECTED RESULT
all fonts are rendered correctly.

SOFTWARE/OS VERSIONS
Operating System: Arch Linux 
KDE Plasma Version: 5.17.3
KDE Frameworks Version: 5.64.0
Qt Version: 5.14.0 beta3
Comment 1 Nate Graham 2019-11-19 18:03:16 UTC
Seems like an issue in the QQC2 desktop style (or maybe in Qt itself, but let's start there). Very visible in the KCMs in System Settings' Appearance section.

I recall another bug you filed about this issue in some KCM. If you can find it, can you mark it as a duplicate of this one? Thanks!
Comment 2 Nate Graham 2019-11-19 18:03:33 UTC
*** Bug 414265 has been marked as a duplicate of this bug. ***
Comment 3 Nate Graham 2019-11-19 18:05:12 UTC
*** Bug 405145 has been marked as a duplicate of this bug. ***
Comment 4 Nate Graham 2019-11-19 18:05:21 UTC
Found it.
Comment 5 nyanpasu64 2021-01-12 10:55:06 UTC
Created attachment 134768 [details]
Button with diagonal seam due to 125% scaling

I'm running KDE at 125% display scaling, and I've experienced this problem in Kirigami-based KDE settings panels for months now. When writing a simple QML app of my own, I came across this interesting issue where instead of a horizontal row of duplicated or missing pixels, the image has a diagonal seam, running from the bottom-left of the text to the top-right.

For the screenshot, I produced a button using all | characters to make the seam more obvious. The seam disappears when I press Alt (also causing the left border of the button to have 1 less duplicated pixel), or if I add enough | characters to stretch the button horizontally. The border rendering is also quite bad to be honest, with duplicated pixels and fuzzy lines.

Is the diagonal seam caused by inconsistent rounding when dividing the button into triangles to be drawn? Is it a different root cause than the horizontal duplicated rows?
Comment 6 nyanpasu64 2021-01-14 10:11:39 UTC
Created attachment 134836 [details]
Video of a Qt Quick 2 button glitching when resized, with patched qqc2-desktop-style

I've done a deep-dive investigation, looking into multiple issues and Qt Quick design defects causing this bug's symptoms to occur. For reference, the C++ implementation of the theme is at https://invent.kde.org/frameworks/qqc2-desktop-style/-/blob/7cdeaab75171b4126ba9026875539108a834a577/plugin/kquickstyleitem.cpp.

## Background

Qt Quick 2 positions and sizes items in device-independent pixels. `KQuickStyleItem` (defined by qqc2-desktop-style in the above link) is a subclass of `QQuickItem` used to render widgets and output them on-screen. In simple QQC2 apps (like System Settings panels), the position and size of each item are integers in device-independent pixels. Perhaps in more complex apps, the widgets could be translated or rotated, making them no longer integers.

When `KQuickStyleItem::updatePolish()` is called, it renders the widget using `QPainter`/`QStyle` onto a member-variable `QImage m_image`. (Note that this is an abuse of updatePolish() which is intended to relayout and not redraw.) When `KQuickStyleItem::updatePaintNode()` is called, it uploads the `QImage` to a GPU texture, and outputs a scene graph item (`QSGNinePatchNode *` upcasted to `QSGNode *`) pointing to the texture. Qt Quick 2 turns it into a rectangle (two tris) and sends the mesh/texture to OpenGL.

The size and position of the `KQuickStyleItem`/`QQuickItem` is not determined by `KQuickStyleItem::updatePaintNode()` or `KQuickStyleItem::updatePolish()`, but merely consumed. `QQuickItem::boundingRect()` is an undocumented method used in `KQuickStyleItem::updatePaintNode()` (and assumed to match `width()`/`height()` which is what `updatePolish()` uses). Based on my debug prints, `boundingRect()` returns a rectangle where the top-left lies at (0, 0), even if the item does not lie at the window's origin. You need to use `mapToGlobal()` to find the true position in the window.

By running RenderDoc on QQC2 apps (kcm_fonts and a single-button test app), I found that the rectangle's coordinates are sent to OpenGL as device-independent pixels (`qt_VertexPosition`), and the vertex shader transforms them into `gl_Position` coordinates. Then it samples the texture using nearest-neighbor interpolation, even if you modify the `createTextureFromImage()` value by calling `setFiltering(QSGTexture::Linear)`. I was unable to get RenderDoc to replay the trace with linear filtering enabled.

## Bug

There are multiple layers of bugs:

1. With DPI display scaling enabled, the polygon drawn on the GPU, as well as the region used by mouse handling, no longer lies on integer physical pixels. The visual polygon's size may not match the QImage's size.

    - I "fixed" this by having `KQuickStyleItem::updatePaintNode()` return a scene-graph node which matches the QImage's size and not the `QQuickItem`'s size (a).

2. Even if the visual polygon's size is an integer number of physical pixels, the x or y coordinates can lie on a 0.5-pixel boundary. This causes rounding errors and sometimes results in the "diagonal seam" effect (and might also cause duplicated/missing rows/columns) where the two triangles of the rectangle round their boundaries and texture coordinates differently. With `QQuickWindow::TextureCanUseAtlas` in place, the diagonal seam effect is inconsistent, doesn't appear in all buttons, and can appear/disappear when you hover a button (because many `KQuickStyleItem`s are placed onto a single texture uploaded to the GPU, resulting in inconsistent rounding). With that parameter removed (b), the effect is consistent and appears in all buttons with the same x or y endpoints, in both hovered/unhovered states.

    - I recorded and attached a video demonstrating this bug occurring when I resize a window, with a patched version of qqc2-desktop-style which sets the returned node's size to the QImage's size in physical pixels (a) and also disables Qt's texture atlas (b). I saw both duplicated rows/columns of pixels and diagonal seams.

    - I don't know the best fix for this. I had a branch where `KQuickStyleItem::updatePaintNode()` used `mapToGlobal` and `mapFromGlobal` to quantize the output polygon to global physical pixels (c). However, this method is only called when a widget is hovered, not upon window resize (which also changes the widget's position and affects pixel rounding), so it's not a workable solution. Worse yet, this solution fails completely if you rotate a widget (but I suspect the current qqc2 theme had the same behavior). Also I was not able to find a way to enable linear filtering for `KQuickStyleItem`'s returned paint node, which would cause non-integer nodes/items to be blurry and ugly, but obviously stand out and not have diagonal/horizontal/vertical seams.

3. I made a test QQC2 app, and resized the window interior to 110x42 physical pixels (at 125% display scale). The vertex shader's transformation matrix (`qt_Matrix`) is used to map device-independent pixels (`qt_VertexPosition`) to OpenGL coordinates. However the matrix's vertical transform is wrong because it thinks the output texture size is 110 x 42.5 physical pixels (corresponding to 34 device-independent pixels, an integer). This causes missing and/or duplicate rows of pixels, even on my branch where I implemented a partial fix for Issue 2 (c) .

    - I think this is a Qt bug, not a KDE theme bug.

I have a private Git repo with my attempted bugfixes. Unfortunately it's a `git init` repo based off Arch's `qqc2-desktop-style-5.78.0` package, not a clone of the upstream source. I can publish my repo if anyone is interested.

I have no desire to work on a Qt fix for Bug 3 (`qt_Matrix`), since Qt 5.15 is no longer receiving public or open-source updates. I may investigate further if Kirigami or qqc2-desktop-style ports to Qt 6 (which still has source code for the time being), or if KDE/etc. forks Qt 5.15.

I don't know how to fix Bug 2 (non-integer positions and rounding errors) because I don't know how to recompute the scene node's position whenever the QML item is moved (eg. by the layout). Neither updatePolish() nor updatePaintNode() are called when I resize the window.

Additionally I don't know if there's any interest in fixing this bug. I don't know how many people use fractional scaling on KDE X11 (or Wayland if the bug occurs there as well). Those people are going to run into this bug more and more, as KDE ports system settings panels (and possibly other apps) to QML.
Comment 7 David Edmundson 2021-01-14 10:25:16 UTC
>since Qt 5.15 is no longer receiving public or open-source updates.

We will have no issues getting patches to users.
We regularly give patches to distros when upstream releases have issues that don't cover our needs. This is no different. 

But I don't really follow what you think is a Qt bug with regards to the qt_Matrix. It has to do the mapping between co-ordinates, so what would we change? 

>Also I was not able to find a way to enable linear filtering for `KQuickStyleItem`'s returned paint node, 

You can set a filtering on the QSGTexture. Nodes will follow that.

Though ideally our source textures and our output sizes should always match in device sizes, so it shouldn't make a difference.




>Additionally I don't know if there's any interest in fixing this bug

Ultimately QQC2-desktop-style is a hack. It's not where we want to be going long long term for styling, it's far too many texture uploads. It's just there as we need visual compatibility.
Comment 8 nyanpasu64 2021-01-14 10:54:21 UTC
>But I don't really follow what you think is a Qt bug with regards to the qt_Matrix. It has to do the mapping between co-ordinates, so what would we change?

qt_Matrix is constructed using the output texture's height and width. Its current function is to take a point in device-independent pixels (x-right = 0 .. (w/devicePixelRatio), y-down = 0 .. (h/devicePixelRatio)), and convert it to an OpenGL position (x-right = -1..1, y-up = -1..1) which maps to the the same point (converted to physical pixels) in the output texture. A symbolic description of the procedure is as follows:

- Divide x by (w/devicePixelRatio), then multiply by 2 and subtract 1.
- Divide y by (h/devicePixelRatio), then multiply by 2 and subtract 1, then negate.

However it's currently computed incorrectly, since it rounds w/devicePixelRatio and h/devicePixelRatio (the output texture size converted to device-independent pixels) to an integer. At least that's my understanding of what goes wrong, since I could not find the code that computes qt_Matrix.

Also I think this code will break down even worse if I test on Wayland with per-monitor scaling factors (I haven't tested the current code or any of my proposed bugfixes on it, and the current code has some iffy branching off that).

>You can set a filtering on the QSGTexture. Nodes will follow that.

I tried the following in KQuickStyleItem::updatePaintNode(), and it had no effect (still nearest-neighbor):

```
    auto texture = window()->createTextureFromImage(m_image, {});
    texture->setFiltering(QSGTexture::Linear);
    styleNode->setTexture(texture);
```

>Ultimately QQC2-desktop-style is a hack. It's not where we want to be going long long term for styling, it's far too many texture uploads. It's just there as we need visual compatibility.

Is it worth fixing the bug or not, by quantizing each scene node to physical pixels whenever it moves? (I suspect the current approach will never support rendering scaled/rotated/etc. widgets cleanly. So I have to somehow detect non-translation transformation, turn off quantization, and turn on linear interpolation... which doesn't work right now.)

Is https://invent.kde.org/plasma/qqc2-breeze-style mature yet? I don't see any Arch package supplying it.

----

Additionally I think the texture width/height override code is unused. KQuickStyleItem::setTextureWidth() sets m_textureWidth, which gets multiplied by the screen DPI scaling factor when picking a texture width (this feels like a mistake). Moreover, KQuickStyleItem is a library-private class, only used internally as the QML type "org.kde.qqc2desktopstyle.private.StyleItem". And none of the QML files set the textureWidth property, so I think m_textureWidth is always 0 and this part of the code is dead.

(Code search: https://invent.kde.org/frameworks/qqc2-desktop-style)
Comment 9 Nate Graham 2021-01-26 00:59:40 UTC
*** Bug 432095 has been marked as a duplicate of this bug. ***
Comment 10 Nate Graham 2021-01-26 15:49:18 UTC
*** Bug 432114 has been marked as a duplicate of this bug. ***
Comment 11 Nate Graham 2021-01-26 21:28:06 UTC
*** Bug 431955 has been marked as a duplicate of this bug. ***
Comment 12 Nate Graham 2021-03-12 14:52:06 UTC
*** Bug 433554 has been marked as a duplicate of this bug. ***
Comment 13 Nate Graham 2021-03-15 23:12:00 UTC
*** Bug 434402 has been marked as a duplicate of this bug. ***
Comment 14 Nate Graham 2021-03-18 17:09:53 UTC
*** Bug 434484 has been marked as a duplicate of this bug. ***
Comment 15 Nate Graham 2021-03-31 17:36:36 UTC
*** Bug 435156 has been marked as a duplicate of this bug. ***
Comment 16 Nate Graham 2021-04-07 15:35:30 UTC
*** Bug 435456 has been marked as a duplicate of this bug. ***
Comment 17 Peter Shkenev 2021-04-07 15:49:29 UTC
(In reply to Nate Graham from comment #16)
> *** Bug 435456 has been marked as a duplicate of this bug. ***

That bug was at default scale
Comment 18 Nate Graham 2021-10-15 15:26:17 UTC
Should be fixed by the fixes for upstream bugs https://bugreports.qt.io/browse/QTBUG-96112 and https://bugreports.qt.io/browse/QTBUG-83626, which is being backported to the KDE patch collection.
Comment 19 Nate Graham 2022-08-01 17:29:55 UTC
*** Bug 457326 has been marked as a duplicate of this bug. ***
Comment 20 Nate Graham 2022-08-10 15:01:08 UTC
*** Bug 457629 has been marked as a duplicate of this bug. ***
Comment 21 Nate Graham 2022-09-09 17:38:55 UTC
Bug 432264 isn't related.
Comment 22 Gerhard 2022-10-30 21:15:14 UTC
Seems that this bugs has returned with Plasma 5.26.0 with Qt 5.15.6.
One example is the mouse/keyboard battery in KInfoCenter (see screenshot). This was definitely fixed before. Reopen?
Comment 23 Gerhard 2022-10-30 21:18:19 UTC
Created attachment 153341 [details]
Buttons with horizontal line offset at 150% scaling
Comment 24 Nate Graham 2022-10-31 18:13:20 UTC
This bug was about text; that's something different.
Comment 25 Nate Graham 2023-01-10 22:13:52 UTC
Something similar is happening again; see Bug 464069. :(