[KCM] Scale more coarsely with the slider, but more finely with a spinbox
ClosedPublic

Authored by ngraham on Sep 30 2019, 8:22 PM.

Details

Summary

Right now we have a problem in that the important scale factors of 1.25 and 1.75 are
not reachable using the UI. However just reducing the slider increment to 0.05 would
result in way too many slider values. Instead, this patch implements the following:

  • The slider goes by increments of 0.25
  • There's a spinbox beside the slider that allows the user to set the scale factor by 0.05 increments
  • On X11 with global scaling, the user is shown a warning message when using scale factors that are not a multiple of 0.25

This way the commonly-used scale factors are more accessible, but finer values are
made available to people who really need them and are likely to go poking around.

BUG: 412447
FIXED-IN: 5.18.0

Test Plan

Diff Detail

Repository
R104 KScreen
Branch
advanced-scaling-control (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17228
Build 17246: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added a project: Plasma. · View Herald TranscriptSep 30 2019, 8:22 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Sep 30 2019, 8:22 PM

0.05 is no good idea either ;)
Just like 0.1 that is no 1/2, 1/4, 1/8, ...
If we really need something finer than 0.25, could we stick with 1/8 or 1/16?

0.05 is no good idea either ;)
Just like 0.1 that is no 1/2, 1/4, 1/8, ...
If we really need something finer than 0.25, could we stick with 1/8 or 1/16?

I don't want to lock people out of the finer scale factor numbers. Depending on your screen DPI and size, you may want to set and be quite happy with a weird scale factor like 1.65x. The idea here is to expose the more common and bug-free scale factors in the main UI, while letting experts and risk-takers set whatever they want. Maybe I should add a warning in the dialog that people who set a weird value there are on their own?

ndavis added a subscriber: ndavis.Sep 30 2019, 8:30 PM

what would be wrong with replacing the label with the spinbox? For normal control, use the slider. For fine control, use the spinbox. Then we wouldn't need an extra dialog.

what would be wrong with replacing the label with the spinbox? For normal control, use the slider. For fine control, use the spinbox. Then we wouldn't need an extra dialog.

That isn't a bad idea, but then it feels a bit redundant since you'd have two adjacent controls that set the same value.

GB_2 added a subscriber: GB_2.Sep 30 2019, 8:34 PM

The KDE HIG suggests something like this: https://hig.kde.org/_images/Slider.Speed.qml.png

0.05 is no good idea either ;)
Just like 0.1 that is no 1/2, 1/4, 1/8, ...
If we really need something finer than 0.25, could we stick with 1/8 or 1/16?

I don't want to lock people out of the finer scale factor numbers. Depending on your screen DPI and size, you may want to set and be quite happy with a weird scale factor like 1.65x. The idea here is to expose the more common and bug-free scale factors in the main UI, while letting experts and risk-takers set whatever they want. Maybe I should add a warning in the dialog that people who set a weird value there are on their own?

That is fine, but please use some step width that is a proper float.
Just use 1/16, that is 0,0625.
0.05 will just lead to the same issues as 0.1, it looks nice in the input field but is no floating pointer number you can calculate sanely with.

This comment was removed by ghost34.
This comment was removed by ndavis.

And the custom scale factor dialog really should have some warning text like on Windows ;=)

see e.g.

https://www.pcworld.com/article/2953978/use-windows-10s-individual-display-scaling-to-perfect-your-multi-monitor-setup.html

(custom scale factor not recommended)

And the custom scale factor dialog really should have some warning text like on Windows ;=)

see e.g.

https://www.pcworld.com/article/2953978/use-windows-10s-individual-display-scaling-to-perfect-your-multi-monitor-setup.html

(custom scale factor not recommended)

Ah, if it's not recommended, that might be a good reason to keep it in a separate dialog.

Btw., I think GNOME is more clever than us, too, they allow just sane scalings in the UI:

https://www.omgubuntu.co.uk/2019/06/enable-fractional-scaling-ubuntu-19-04

In D24321#540026, @GB_2 wrote:

The KDE HIG suggests something like this: https://hig.kde.org/_images/Slider.Speed.qml.png

I kind of like this idea. Then if you set a weird value, an InlineMessage at the top of the screen would appear warning you that it's not a recommended value.

Also without any string changes, it would go into Plasma 5.17. :)

ouwerkerk added a subscriber: ouwerkerk.EditedSep 30 2019, 9:01 PM

Ideally you would also warn if the reciprocal times the horizontal, vertical resolution yields a non-integer output, not just if some value is chosen which cannot be represented in floating point exactly.

E.g. on a 4K (3840 × 2160) a scaling factor of 1.5 is fine because the horizontal resolution scales down to effectively 3840 × 2 ÷ 3 = 2560 and the vertical resolution scales down to 2160 × 2 ÷ 3 = 1440. This means that the scaled renders will fit the physical display exactly.

Also side note, as a non-native English speaker I find "grossly" to be a bit of an odd adjective to use here -- "coarsely" would be more familiar: as in fine/coarse grained. Perhaps something to keep in mind when this lands and you start blogging about it :)

it would go into Plasma 5.17. :)

There's still a feature freeze.

No you're right, "grossly" is a poor choice of wording.

ngraham planned changes to this revision.Sep 30 2019, 9:54 PM
ngraham retitled this revision from [KCM] Scale more grossly with the slider, but more finely with a semi-hidden spinbox to [KCM] Scale more coarsely with the slider, but more finely with a semi-hidden spinbox.
ngraham edited the summary of this revision. (Show Details)

Anyway I'm going to see if the textfield-next-to-the-slider approach works better (in conjunction with a warning message).

Hi, I thought about this a bit more ;)

I really think we shall limit the minimal increase to 0.25, without any advanced field. Perhaps the range should be larger, like 1-4, if some people get "really" high DPI screens.

Rational:

  1. For small 0.1 changes: Just change your font size, we did that always for the old non-hidpi world were displays did differ a bit in the DPI, that works without any glitches
  1. Anything below 0.25 will just break half of our applications. For KTextEditor I did some hack in master that might avoid artifacts if you only scale with stuff like .25 or .5, it will still break with small stuff and most of our applications look horrible with small scalings, look at attached screenshot fo 1.1, do we really want to advertise that?

Qt 5.14 will even have "opt-out" of fractional scaling and perhaps we even need to do that for some applications...

it would go into Plasma 5.17. :)

There's still a feature freeze.

At least moving away from 0.1 increases is actually a bugfix. Look at my konsole screenshot, similar artifacts can be created with most of our applications.
(sadly still with 0.25/0.5, too, but at least not that often and you then can try to fix that by choosing some better font...)

For small 0.1 changes: Just change your font size, we did that always for the old non-hidpi world were displays did differ a bit in the DPI, that works without any glitches

I even used to do this in the new high DPI world. Before Qt had fractional scaling we had the slider it would adjust the font size until you reached 1.75 then it would change the Qt scale too. It worked fine.

For small 0.1 changes: Just change your font size, we did that always for the old non-hidpi world were displays did differ a bit in the DPI, that works without any glitches

I even used to do this in the new high DPI world. Before Qt had fractional scaling we had the slider it would adjust the font size until you reached 1.75 then it would change the Qt scale too. It worked fine.

Perhaps such a mixed approach might even make more sense.

As said, any scaling like 1.2 will not work with most of our applications, see above Konsole.

With "a lot" of work, stuff like 1.25 or 1.5 will work like in KTextEditor by adjusting the fonts internally a bit to avoid sub-pixel jitter per line.

I think we shall really avoid to expose bad scale factors to users.

For small 0.1 changes: Just change your font size, we did that always for the old non-hidpi world were displays did differ a bit in the DPI, that works without any glitches

I even used to do this in the new high DPI world. Before Qt had fractional scaling we had the slider it would adjust the font size until you reached 1.75 then it would change the Qt scale too. It worked fine.

Is there any technical barrier to returning to this? What was the UI like before? Was there a visual relationship between the scaling UI and the Fonts KCM, or did the former simply update the latter?

romangg requested changes to this revision.Oct 1 2019, 2:33 PM

First the situation is different on X11 and Wayland:

  • On X11 KScreen sets Qt scale factors and some fonts dpi value through Xft. You can read it up here what was taken pretty much as it was from the old code: https://cgit.kde.org/kscreen.git/tree/kcm/kcm.cpp#n317
  • On Wayland KWin scales in OpenGl the whole image it receives from a client. There should be no artifacts visible with any scale factor, be it 1.1 or π.

Looking at the provided screenshot with artifacts: This must be on X11 and I recognize these artifacts from when I tested different fractional scaling values on X11 months and years ago. Insofar this is no regression and is independent of the KCM rewrite. Which makes sense since the previous ui also offered 0.1 step sizes: https://www.dedoimedo.com/computers/plasma-hd-scaling.html

In regards to Wayland: I don't agree that the 1.25 and 1.75 values are more important than 1.2, 1.3 and respectively 1.7, 1.8 values. That's just because you are used to it from other platforms. And that's a normal first reaction but you won't see a significant size difference between 1.2 and 1.25. Try it with 1.2 and 1.3: even here the difference is minimal.

What I definitely don't want for Wayland: recommending to users to set the size of their UI through the overall scaling value on the one side and on the other side through font size. I know some proficient members of our community know how to do that and think it's ok because it was always like that and it comes naturally to them. But to other users it's difficult or cumbersome to do.

Conclusion: Leave it as it is on Wayland. It works fine there and gives user a way more fine-grained level of control than with .25 steps only. On X11 a separate isolated bug fix to this old known problem with artifacts must be proposed.

;=) I talked only about the Qt HiDPI scaling code paths.

For any other scaling I don't care, as that should be application transparent, like you say.
But if Wayland really just scales up the stuff via GL fonts will look terrible.

And no, its non-trivial to fix there any artifacts for strange scales and it makes there computation wise a very large different if the scale factor is 1.2 or 1.25, as the later is something you can sanely compute things with ;)
Therefore I really would like to have only some larger scaling steps set for the QT_SCALE_FACTOR... stuff and for the smaller things just some font adjustments.

Hmm, I just tried here a Wayland session, and yes, the KCM has there just some 1x/2x combobox and my fonts are maximal ugly compared to some export QT_SCALE_FACTOR=2 + starting a Qt application (on Wayland).
But I must try that at home on real HiDPI screens, perhaps my test is suboptimal here.

;=) I talked only about the Qt HiDPI scaling code paths.

In this case I recommend studying the bug report I linked. There is also a patch attempt in Qt but it got closed for some reason.

For any other scaling I don't care, as that should be application transparent, like you say.
But if Wayland really just scales up the stuff via GL fonts will look terrible.

They are scaled down via Gl to match the fractional part of the scaling value. Applications provide pre-scaled buffers of integer factors 2, 3, 4 and so on.

And no, its non-trivial to fix there any artifacts for strange scales and it makes there computation wise a very large different if the scale factor is 1.2 or 1.25, as the later is something you can sanely compute things with ;)
Therefore I really would like to have only some larger scaling steps set for the QT_SCALE_FACTOR... stuff and for the smaller things just some font adjustments.

Don't see a reason for that. Never heard about necessity in Gl to only calculate with certain floating values for best image quality. But if you can provide some source for that I would be interested in studying that. Can't say I see problems here in a Wayland session though, currently using a scaling factor of 1.8.

;=) I talked only about the Qt HiDPI scaling code paths.

In this case I recommend studying the bug report I linked. There is also a patch attempt in Qt but it got closed for some reason.

You can't patch that issues in Qt if e.g. your application relies on facts like you paint 10 lines and you don't get some rounding errors all the time ;)

For any other scaling I don't care, as that should be application transparent, like you say.
But if Wayland really just scales up the stuff via GL fonts will look terrible.

They are scaled down via Gl to match the fractional part of the scaling value. Applications provide pre-scaled buffers of integer factors 2, 3, 4 and so on.

Hmm, but that means you still set QT_SCALE_.... to some non-fractional thing like 2, 3, 4 and the only fine adjust, that makes sense.
Thought for me it did look like the small scale window was up-scaled in my try with the 2x combobox field.

And no, its non-trivial to fix there any artifacts for strange scales and it makes there computation wise a very large different if the scale factor is 1.2 or 1.25, as the later is something you can sanely compute things with ;)
Therefore I really would like to have only some larger scaling steps set for the QT_SCALE_FACTOR... stuff and for the smaller things just some font adjustments.

Don't see a reason for that. Never heard about necessity in Gl to only calculate with certain floating values for best image quality. But if you can provide some source for that I would be interested in studying that. Can't say I see problems here in a Wayland session though, currently using a scaling factor of 1.8.

The issue is that you transform individual elements.

e.g. if you have some list of X graphical elements with height "virtual" pixel 10. If you scale them by 1.1, you get not "real pixel" 11, you get some 11.00100101010 something. If you later compute things like "10 * element size", you not get 110 pixel but again some weired stuff ;=)

https://www.exploringbinary.com/why-0-point-1-does-not-exist-in-floating-point/

If you apply that as transform on the full texture, I can agree that it shouldn't be a real issue.
But if it is applied as QT_SCALE... you will have the issue of rounding artifacts for the individual widgets/layout members that then sum up in weired ways.
You get already some artifacts with 1/2 or 1/4, but at least you can multiply that sanely (as long as your floating point numbers don't get enourmous).

Ok, tried Wayland now on my HiDPI setup.
In Plasma's KCM I only have Scale 1x or 2x and that works ok, as one would think.
But I see no way to set any more fine-grained scale to check if the scaling is nice for other values.
The slider is for me only there for X11.

I will investigate the https://bugreports.qt.io/browse/QTBUG-66036 bug again, too :/
Even with the rounding issues one gets over larger multiplications, it shouldn't be that buggy for short ones.

I got some more insight into https://bugreports.qt.io/browse/QTBUG-66036
If one turns off anti-aliasing, in KTextEditor most of the evil artifacts disappear, but not all.

Ok, I wasted one evening on the rendering artifacts stuff.

https://bugreports.qt.io/browse/QTBUG-66036

I have now ugly workarounds in KTextEditor:

  1. no anti-aliasing, else fillRect misses to fill 1 pixel at the borders by rounding errors
  2. adjust the clip rect by one logical pixel to not miss 1 real device pixel sometimes....
dhaumann added a subscriber: dhaumann.EditedOct 1 2019, 8:52 PM

As far as I understand, the reasoning of @cullmann is that 0.1 cannot be accurately be represented by a computer. Following this discussion, the number 0.1 will turn into either 0.09999999999999999167332731531132594682276248931884765625 or 0.1000000000000000055511151231257827021181583404541015625.

And @cullmann's point here is: Do we really want to have calls of QPainter::scale() with these above numbers? It is quite obvious that you will never get any full pixels with such scaling factors.

That's why i) proposing a coarser step width, and ii) numbers that CAN be represented lead to the proposed patch of 0.25, 0.5, 0.75, ... These numbers are perfectly representable by a computer without rounding artifacts, and multiplying with ints you may even have a chance of getting a nice int again in many cases, which is exactly what you want, since the chance of getting full pixels is much higher.

So +1 for this patch, it does make a lot of sense (in fact, based on the above reasoning, I'd even drop the option of setting 0.05, 0.1, ... via the advanced button). Please reconsider this patch.

Even with my hacks for https://bugreports.qt.io/browse/QTBUG-66036 in current KTextEditor master (see https://bugs.kde.org/show_bug.cgi?id=390451) you will still get after every xx lines some stray artifact in some cases during selection if you have some 1.1 scaling.
That still seems to be a remaining issue with selection painting. Not happening with proper scaling of 1.25 or 1.5 as nothing adds up multiplication.

ngraham updated this revision to Diff 67156.Oct 1 2019, 9:26 PM

Take two on the UI

ngraham retitled this revision from [KCM] Scale more coarsely with the slider, but more finely with a semi-hidden spinbox to [KCM] Scale more coarsely with the slider, but more finely with a spinbox.Oct 1 2019, 9:27 PM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
ndavis accepted this revision.Oct 2 2019, 2:22 AM

Visually, this looks fine. I'll leave the technical review to those who know more about that.

dhaumann added a comment.EditedOct 2 2019, 5:33 AM

Thanks Nate for the update, I think this is very transparent to users and makes using bad values by default a bit harder.

+1 for the patch.

I like that UI, too.
For Wayland: Can somebody tell me how I can try there some fractional scaling?

I like that UI, too.
For Wayland: Can somebody tell me how I can try there some fractional scaling?

You are on KScreen master or beta channel? Because the new UI allowing fractional scaling is only available there. If you are check that there is only one kcm_kscreen.so in
/usr/lib/x86_64-linux-gnu/qt5/plugins/kcms but not in /usr/lib/x86_64-linux-gnu/qt5/plugins.

I like the new approach. On Wayland the value range should start at 0.5 and go till 3.

ngraham updated this revision to Diff 67191.Oct 2 2019, 2:11 PM

Allow values as low as 0.5 in the spinbox for the Wayland case

romangg requested changes to this revision.Oct 2 2019, 2:36 PM

Last issue: we need to change in kded/output.cpp line 363 where it rounds on config write to one digit to round to two digits instead:

info[QStringLiteral("scale")] = int(output->scale() * 100 + 0.5) / 100.;

kcm/package/contents/ui/OutputPanel.qml
90

whitespace around multiplication, next line as well

This revision now requires changes to proceed.Oct 2 2019, 2:36 PM
ngraham updated this revision to Diff 67195.Oct 2 2019, 2:45 PM
ngraham marked an inline comment as done.

Address final review comments

Hmm, could we perhaps adjust the minimal step to 0,0625.
That is 1/16 and with that, I never get any artifacts with KWrite in current master, as it nicely sums up.
Then you don't need to round at all for output.

To illustrate that, just try, with master of KTextEditor:

export QT_SCALE_FACTOR=1.125

kwrite with some file => select 100 lines, zoom font in and out => I get no artifacts

export QT_SCALE_FACTOR=1.0625

> same

export QT_SCALE_FACTOR=1.1

> artifacts each X lines

Hmm, could we perhaps adjust the minimal step to 0,0625.
That is 1/16 and with that, I never get any artifacts with KWrite in current master, as it nicely sums up.
Then you don't need to round at all for output.

We can think about that in a separate patch. But in general I'm against putting much work in the X11 case. Also I believe the issue should be dealt with in Qt X11 scaling implementation instead of trying to fix it downstream through involved workarounds. The current design by Nate I like in particular because it solves the issue I had with the 0.1 steps slider of being too fine grained to control nicely. That's the main reason for me supporting the change not so much that it circumvents Qt scaling issues on X11.

Having 0.25 steps only on X11 by default is good enough for my liking.

romangg accepted this revision.Oct 2 2019, 2:55 PM

master

This revision is now accepted and ready to land.Oct 2 2019, 2:55 PM
This revision was automatically updated to reflect the committed changes.

I don't get why we want to introduce artifacts we can very simple avoid by using a different base for the steps.
Is 0.0625 really that much worse than 0.05?

I don't get why we want to introduce artifacts we can very simple avoid by using a different base for the steps.
Is 0.0625 really that much worse than 0.05?

I'll investigate this in a follow-up patch. My only reservation is that 0.05 is a much more "round" value to show in the user interface compared to 0.0625. I'll see what I can do with it.

cullmann reopened this revision.Oct 2 2019, 3:14 PM

Besides, I think you missed half of the code, as for me, I still get a different widget here, as for my setup the OutputPanel.qml is used, not Panel.qml.

(as I have two displays that can be configured separately)

Could you fix that, independent of the 0.05.. issue?
I agree that we can do an extra patch for that.

This revision is now accepted and ready to land.Oct 2 2019, 3:14 PM

Or I am maximal confused ;=)

If I compile a master kscreen, have sourced the prefix.sh, shall I see the correct stuff in e.g. the kcm_testapp in bin of the kscreen build dir?

If not, I am sorry, could somebody help me out how to test this properly?

ngraham closed this revision.Oct 2 2019, 3:46 PM

I think the issue you're seeing is related to not having the new KCM installed properly, not anything this patch introduced. It's a bit tricky. You need to delete the old kscreen.so file or else the new one won't show up properly.

OutputPanel.qml is for the per-display scaling UI that's only used on Wayland. Panel.qml is the X11-only global scaling UI.

Ok, but given I have a system wide installed kcm anyways, from the distro, is there a way to use the own I have in my user local prefix?
That explains why I am too dumb to try the fractional scaling on Wayland with my installed master version, too.

ngraham added a comment.EditedOct 2 2019, 3:52 PM

Ok, but given I have a system wide installed kcm anyways, from the distro, is there a way to use the own I have in my user local prefix?

I could not find a way. I moved aside the .so installed by my distro and then I could use the locally-compiled one. I think it's because of the prefix paths; kscreen.so takes precedence over kcms/kscreen.so. So hopefully this should be fixed once your distro ships Plasma 5.17 and the distro-provided version lives at kcms/kscreen.so.

Ok, that explains why all my tries fail...
I thought the hint above about looking for kcm_screen in the system was just if I have mixed up some stuff, but I always thought if the QT_PLUGIN_PATH is properly set like with prefix.sh the local plugins will be prefered.
Will try at home.

Ok ,works for me, too. Sorry for the noise! And thanks for the explanation!
I think this is already much better than before, the details of the last finesse with the minimal step can be discussed elsewhere.

Here's a patch that changes the increment (along with a few dependencies that improve the UI in preparation for it): D24373