Skip to content

Slider & ProgressBar: Rewrite for the greater good

Time for another Super-Duper-Mega-MR.

Let's talk about Sliders

sliders

yes, those ones

In QtQuick bridge they didn't get to have any sane implicit size — unless — they had tick marks due to stepSize property being set to non-zero. In Widgets world sizing is managed by two complementing forces:

  • sizeHint() implementations of individual widgets classes,
  • sizeFromContents() implementations in styles, called from a widget whenever it feels like it.

Looking at existing styles and at upstream widgets internals, it seems like sizeFromContents() was never meant to report full implicit size of a styled item. Thus, when our bridge code only called sizeFromContents() for sliders it never got a full implicit size in return. The fix is to copy-paste few lines of code from QSlider::sizeHint(). Code duplication is a sad thing, but there's really no other way around it, aside from creating and synchronizing a shadow QSlider object.

Next, the bridge class has a pair of contentWidth/contentHeight properties. They are only used by one other component (ComboBox's combined max width), and by reading the C++ code it's not really clear to me what they were even intended for. And the way they are used interchangeably with arguments in KQuickStyleItem::sizeFromContents(int width, int height) makes me think that this all is very much in need for a nice little refactoring.

Anyhow, implementing proper implicit size for sliders allowed to get rid of contentWidth/Height bindings in the styling code. As a side-effect, this also meant dropping 2px thickness bump in Tablet Mode; but AFAIK no styles ever used the added screen estate to render a larger image, and besides Oxygen/Breeze styles are painting at the top, not centered. Which was also another reason to drop anchors.verticalCenter positioning: if we want that, we need to fix it in our style; Fusion renders centered, and MS Windows 9x straight up fills up available space for a full-height handle (knob).

And since I just mentioned slider's handle, our component now correctly reports its size, and it allows QtQuick.Templates/Slider use that metrics to correctly calculate value from a mouse position. This is especially noticeable around the edges, where previously you had to drag the handle all the way to the edge slider's texture boundaries to set an extreme value. But with a properly reported size, extreme points are now located where you would expect them to be: handle.width/2 away from the bounds, right above the points where the groove ends. You might say that it's not a big deal. But once you know it, it's impossible to "unsee" this papercut.

Last but not least, there are sliders which do not report their value as they go — only once released. Such sliders are technically called "non-live", and their value is separate from their position. QStyle has access to both numbers, but unfortunately in QtQuick.Controls world there is only final value in client-specified range [from..to] and current position & visualPosition duo ranging from 0.0 to 1.0. As you may have guess by now since I'm writing about this, our bridge was only receiving a final value, and that didn't quite work for "non-live" sliders. The fix is to shift toward position property, and I used this opportunity to drop that over-engineered factor multiplier (which has also caused breakaged for too large ranges in past). Now the range is fixed 0..100000 (because we are working with integer precision), and only step binding needed some not-completely-trivial expression to scale it to 0..1 range.

BUG: 473400

Did we make any Progress?

progress bar

Trowing few cents about ProgressBar as well, it was also ported away from factor sorcery, and is now as simple and robust as it could ever get. There is an accompanying MR in Breeze RTL rendering and its interaction with invertedAppearance feature.

Apart from that, ProgressBar has got its hover tracking feature disabled by default — there was nothing reacting to hover anyway.

Its expression for implicitHeight was ported from a hardcoded Breeze-specific magic number to a standard QQC2 formula, and extra size converted to insets — which also helps preserve some memory by rendering on a smaller texture. Unlike sliders, this didn't require any changes in native code, as its sizeFromContents is good enough in all shipped styles, and anyhow QProgressBar::sizeHint() doesn't do anything particularly interesting for QQC2 version of ProgressBar.


And that's about it for now. Feel free to give it spin with an updated manual test testBars.qml!

Edited by ivan tkachenko

Merge request reports