The window corner frame rounding amount does not scale at all.
This makes the windows more rectangular then they should be when scaling is used.
Details
- Reviewers
hpereiradacosta davidedmundson - Group Reviewers
Breeze VDG
No scaling, nice round corners:
2x scaled before, sharper corners:
2x scaled after, nice round corners:
Diff Detail
- Repository
- R31 Breeze
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 822 Build 835: arc lint + arc unit
mmm. Ok. It works. Now, why is it not handled upstream (by kwin or Qt), as it is in the widget style ?
(see how the QStyle rounding, for e.g. the inner frame of your list) is rounded properly when changing the scale factor.
And then, should it not be fixed there, that is, upstream, so that it applies to all decoration styles, rather than in every single decoration ?
I am ready to accept this patch, but am afraid it is a bit of a hack, and that it does not scale. (meaning: does not fix other decorations, such as oxygen, and will have to be copied if one implements other decoration components, such as e.g. rounded square buttons)
I'd prefer to have this handled upstream ...
Also: how does it work with multiple monitors ? (I see you get the info from ::primaryScreen, not for the screen the window is actually on ?
could you add @davidedmundson as a reviewer ? I think he knows a lot about how pixel scaling is/should be handled in kwin and plasma.
(definitly more than myself).
As far as I know KWin X11 does not allow different scaling factors per monitor, so it should be fine.
And on Wayland QGuiApplication::primaryScreen()->logicalDotsPerInch() is always 96 so this only affects X11
@davidedmundson right?
IIRC, decoration renderer sets device pixel ratio, so in theory, you should not multiply frame radius by dpr.
IIRC, decoration renderer sets device pixel ratio, so in theory, you should not multiply frame radius by dpr.
For some reason my comment went missing, but as zzag said, with high DPI here the only things we should ever change are pixmap sizes and QPixmap::setDevicePixelRatio not changing the (logical) size of what's actually being painted.
So do I interpret this right when saying that the issue (which is real, judging from the screenshots), is "elsewhere" ? meaning "upstream" ? Meaning also that fixing it here will break in the future, once the upstream issue is found and fixed ?
Most likely the actual issue is in kdecoration2 ...
I don't think so. Again, IIRC, decoration render does the following:
- create a backing store
- set device pixel ratio
- create QPainter
- pass the QPainter instance to decoration (->paint(&p, ...))
So, maybe, that's an issue in Qt or KWin.
Here is Breeze close button modified to use QPainter::drawRoundedRect() and uses the same radius (3) as the titlebar:
This scales well, so I don't think it is a bug in Qt.
As others said the dpi scaling should normally be dealt with QImage::setDevicePixelRatio()
As far as I can tell this is not working with Breeze for the following reason:
The decoration rendering starts here: https://github.com/KDE/kwin/blob/master/decorations/decorationrenderer.cpp#L66
The dpi is set here correctly with QImage::setDevicePixelRatio()
However this dpi value here is incorrect.
client()->client()->screenScale() is defined here: https://github.com/KDE/kwin/blob/master/toplevel.cpp#L316 and gets the value from here: https://github.com/KDE/kwin/blob/master/toplevel.cpp#L271
And in that function qreal newScale = screens()->scale(m_screen); gets the value from here: https://github.com/KDE/kwin/blob/master/screens.cpp#L114
You can see that this function just returns 1, and because of this the devicepixelratio of the decoration QImage will always be 1.
If I set Plasma scaling to 2 and change this value to 2 or the devicepixelratio here https://github.com/KDE/kwin/blob/master/decorations/decorationrenderer.cpp#L69 to 2 then the decoration renders very incorrectly. Not just simply double the size of everything, but invisible decoration and other artifacts.
We already have a workaround in Breeze for this in some parts. For example here https://github.com/KDE/breeze/blob/master/kdecoration/breezebutton.cpp#L166 when rendering the buttons for Breeze decoration we scale the QPainter manually.
This patch adds another workaround, but gets the job done, and fixing this whole thing would be a big effort.
And in that function qreal newScale = screens()->scale(m_screen); gets the value from here: https://github.com/KDE/kwin/blob/master/screens.cpp#L114
You can see that this function just returns 1
The method is virtual. It definitely is not always 1.
We already have a workaround in Breeze for this in some parts.
That code you link to is not related to scaling at all. Its normalising a viewport on a canvas to always be between 1 and 20.
However, there are some confusions so to back up a bit.
On X11 kwin does not use scaling at all. There is the old horrible thing where things are bigger if fonts are bigger.
On wayland we do proper scaling, which is why it works correctly
It's confusing when you say "when scaling is used" as you're actually referring to when we're not scaling - and doing the legacy thing.
I'm only the maintainer of when scaling is used. I don't like the other mode existing as it means we have two competing scaling systems at once which super complicates my stuff..but as a patch for that mode it does look kinda right.