QtCollider: AbstractStepValue widgets: improve wheelEvent scroll behavior + fix regression#7030
Conversation
elgiano
added
qt6
elgiano
changed the title
|
Here's are my tests on M1 macbook pro with internal touchpad: Scroll without inertia (sliding across full vertical space of the touchpad): scroll.-.no.inertia.movScroll with inertia (trying to use similar speed on the touchpad on each of the sliders, but that's of course subjective) scroll.-.with.inertia.mov |
|
BTW I looked at Qt documentation for this, as I thought there should be a more direct translation from a distance on the touchpad to what's on the screen to get a natural feel. Qt docs suggest to use
|
|
Thanks so much for testing! It makes sense to me. My guess is that trying to find the right scaling for pixelDelta() for every system might be difficult...
I'll make another commit to only use angleDelta(), so that it should behave exactly like 3.13. Then if we want to change scroll behavior we can think about it separately.
…-------- Original Message --------
On 7/3/25 20:41, Marcin Pączkowski wrote:
dyfer left a comment [(supercollider/supercollider#7030)](#7030 (comment))
Here's are my tests on M1 macbook pro with internal touchpad:
In comparison to SC 3.13, this PR makes the scroll more sensitive.
Note that current develop has the scroll less sensitive than 3.13.
I think that with inertia (having the scroll continue after a quicker "flick" of the touchpad) this PR might be too sensitive? However, scrolling without inertia seems good to me.
Scroll without inertia (sliding across full vertical space of the touchpad):
https://github.com/user-attachments/assets/38f42a71-76ab-4ec1-abb2-816c2fcdc5a4
Scroll with inertia (trying to use similar speed on the touchpad on each of the sliders, but that's of course subjective)
https://github.com/user-attachments/assets/5a8b1a50-fc8c-467c-9df2-cbb06505294f
—
Reply to this email directly, [view it on GitHub](#7030 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AACNMWWT2VZWAMLB2CU2Z2D3GV2OZAVCNFSM6AAAAACAWZTEI2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTAMZTGIYDSMZYGA).
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
After looking again at our code and this PR, I guess we are using pixelData only when available:
However, for something like a slider, we should probably use pixelDelta and use it to change the value, but taking the slider dimensions into account, so that it feels natural... EDIT On one hand I agree that this could be for a separate PR. However, using pixelData is preferable as it provides a smoother experience... I'm not sure what's better in the end. |
|
More thoughts on this: For something like a slider, it makes sense to use |
|
Sounds reasonable, but I'm not sure I would want to go that path, because QSlider has steps. It seems consequential to me to only use numSteps and thus angleDelta. Also note that we only support vertical scrolling so far, even if the slider is horizontal. It seems like scrolling is intended as a way to step the value up and down (and you can also shift- or ctrl-scroll to use different step sizes). If we wanted to go the
and this would make scrolling react differently to widgets of different sizes, which makes sense to me intuitively, but not so much if we think about it in terms of an interface for steps |
|
Also note that there is already another interface for moving a slider an exact number of pixels: dragging it. |
Let's do that later maybe. There are a few more things to consider, like handling inverted scrolling (in my tests I had to swipe down to make the slider go up, as I use the "natural scrolling" direction on my system).
I'm not sure if the steps are really in the way here. I believe that visual slider should move according to physical movement? As we have Also, for non-slider objects, I think it makes sense to stick to angle data, I think this should also be smooth if the hardware/OS supports it. |
|
Another point of information: I was trying to see how scroll to change value is implemented in other macOS audio apps... and it seems that "scroll to change value" is typically not implemented. I'm not saying we should do it one way or the other, but just to raise awareness of what seems to be the "typical" macOS look&feel. |
|
Thanks, I haven't noticed double step = qMax(_step, pixelStep());
modifyStep(&step);
const double scrollSteps = e->pixelDelta().isNull() ? e->angleDelta().y() / 120.0 : e->pixelDelta().y();
double dval = scrollSteps * step;
setValue(_value + dval);That is, dval is
What do you think? |
Ugh, in that case maybe an ifdef for X11? Or is there a Qt flag that tells us if we're in X11 at runtime? |
|
It should be possible to check |
|
I tried the build 50ff352 I see two issues:
|
|
Well, looking at the code I guess the question is whether we set the scroll wheel sensitivity based on gui size, or not. Intuitively I'd think it makes sense to set it based on the number of revolutions of the scroll wheel, i.e. one full revolution = full range (or whatever?) of the slider, but maybe that's not a good look and feel? In that case we shouldn't use pixelStep from scroll wheel. EDIT: part of the issue is that the range of scrolling on the touchpad is different base on speed. For slow movements, I get a "movement too big" situation. For faster movements, it's closer to what I'd expect, which is that the scale on the touchpad reflects the size on screen. For the mouse wheel, the jump size increases with speed. At the slow speed it seems fine, but at medium speed (what I'd consider a "regular" scroll speed) it moves too much. |
yeah, this code is agin' :D I don't have a mouse wheel around to try, so I don't know if pixelDelta is available or not in that case. But theoretically: if I tried another way to calculate the increment from pixelDelta: Then I modify step (so that alt_scale and ctrl_scale still work), and get the increment as deltaSteps * modifiedStep. This for me gives a scroll speed comparable with dragging, on wayland. It is faster than it was in 3.13, but feels more intuitive. On X11 on the other hand, is way too fast. I'll sleep on it. We can also just revert everything to angleDelta so to reproduce the 3.13 behavior, and take this later, if it takes too long. |
elgiano
added
comp: Qt GUI
|
Touchpad scroll feels pretty good now!
It seems too fast with the mousewheel on macOS as well. I posted some print statements and curiously mousewheel events also come through as pixelData on my macOS. This is touchpad slow scrollThis is touchpad faster scrollThis is mousewheel slow scrollThis is mousewheel faster scroll (a single gesture of the mousewheel, moved the slider all the way up)printing codevoid QcSlider::wheelEvent(QWheelEvent* e) {
double step = qMax(_step, pixelStep());
double scrollRatio;
// Qt advises against using pixelDelta on X11 (xcb)
if (e->pixelDelta().isNull() || QGuiApplication::platformName() == "xcb") {
std::cout << "QcSlider::wheelEvent: using angleDelta: " << e->angleDelta().y() << std::endl;
scrollRatio = e->angleDelta().y() / 8. / 360. / 16.;
} else {
std::cout << "QcSlider::wheelEvent: using pixelDelta: " << e->pixelDelta().y() << std::endl;
scrollRatio = e->pixelDelta().y() * pixelStep();
}
const double numSteps = scrollRatio / step; // totSteps = 1.0 / step
modifyStep(&step);
setValue(_value + step * numSteps);
Q_EMIT(action());
} |
|
ah interesting thanks! EDIT: and while you're at it, could you also print e->deviceType()? void QcSlider::wheelEvent(QWheelEvent* e) {
double step = qMax(_step, pixelStep());
double scrollRatio;
// Qt advises against using pixelDelta on X11 (xcb)
if (e->pixelDelta().isNull() || QGuiApplication::platformName() == "xcb" || true) {
std::cout << "QcSlider::wheelEvent: device " << (int) e->deviceType() << " using angleDelta: " << e->angleDelta().y() << std::endl;
scrollRatio = e->angleDelta().y() / 8. / 360. / 16.;
} else {
std::cout << "QcSlider::wheelEvent: using pixelDelta: " << e->pixelDelta().y() << std::endl;
scrollRatio = e->pixelDelta().y() * pixelStep();
}
const double numSteps = scrollRatio / step; // totSteps = 1.0 / step
modifyStep(&step);
setValue(_value + step * numSteps);
Q_EMIT(action());
} |
|
I pushed a new version, that uses angleDelta for mouse devices (with same scaling as we had in 3.13), X11 (with a faster scaling to match pixelDelta), and otherwise uses pixelDelta. It works fine for me on both X and Wayland, and the behavior makes sense to me at different widget sizes, with different steps, and scales. Shift scale is way too fast when widget is small, but I think it makes sense like this. @dyfer (and everyone interested, maybe @bgola?) can you give it a try? |
elgiano
force-pushed
the
topic/wayland-scroll
branch
2 times, most recently
from
4350d97 to
4d7f769
Compare
|
@elgiano I will try but at a later time. Sorry for the delay. I'll report back once I try. Thanks for working on this! |
|
In 4d7f769 the touchpad feels exactly right for the slider 👍. For the mouse, I have two comments:
|
|
All right, after more testing, here are my thoughts on macOS:
void QcSlider::wheelEvent(QWheelEvent* e) {
double step;
double numSteps;
const bool isX11 = QGuiApplication::platformName() == "xcb";
// use angleDelta for X11 (advice from Qt docs)
if (e->pixelDelta().isNull() || isX11) {
step = qMax(_step, 1.0 / 360.0);
numSteps = e->angleDelta().y() / 8; // I think between "/ 8" and "/ 16" works well with my mouse, taking 5-10 finger gestures for the full range
} else {
step = qMax(_step, pixelStep());
// for pixelDelta, map full range to widget dimension (= 1/pixelStep)
// scrollPx / totPx = scrollSteps / totSteps (totSteps = 1.0 / step)
numSteps = e->pixelDelta().y() * pixelStep() / step;
}
modifyStep(&step);
setValue(_value + step * numSteps);
Q_EMIT(action());
} |
It does in other apps, where the window can be scrolled both horizontally and vertically, but only for the mousewheel , i.e. mousewheel scrolls window up and down shift+mousewheel scrolls left/right. So all in all, we shouldn't have the alt fixes active on macOS. I'd need to check on Windows though. BTW I think I know what's happening with shift on macOS: So for Slider2D, when I tested before the alt changes were pushed, the behavior with the mousewheel was: when pressing shift, the axis switched to horizontal; but we also increased the step, so the value jumped all the way to left/right (step too large to be useful). |
|
Ok, great, it's starting to become clearer now. Now Alt doesn't affect macos anymore, we need to test it on Windows though... and I've abstracted a function to get scrollRatio, now it's all in one place. As for the scales: I think it would make sense to make the default scale like 10 times smaller, so that shift and ctrl scales can be used for bigger movements and alt for very fine tunings, and it also would address @bgola's comment. On the other hand I think the pixelDelta() scaling is a quite intuitive criterion (1px scroll -> 1px step). What do you think? |
|
Thanks! Notes for 105f650 on macOS: Slider, Knob, behave fine with touchpad and mousewheel. Behavior with shift is still noop (more below). Slider2D: with natural scrolling on, the horizontal axis on touchpad is now inverted! I think this should only happen when the controls are not inverted. BTW the Volume control still scrolls only up, the Regarding shift: One more note: it looks like the UI design on macOS these days generally discourages using scroll for changing values. None of the Apple apps I tested (Logic Pro etc) allows changing values with a scroll. I'm not saying we should disable it on macOS, but I think we only need to make it work "well enough" i.e. not be counterintuitive, not necessarily have all the same functionality as other platforms. In my mind that means that we can forego "shift" on macOS, for example. But that's just my personal opinion. |
|
I have a stronger and stronger feeling that we're over-engineering a seldom used feature... and also that we shouldn't change the behavior when all we wanted to do was to fix a regression. The "original" behavior is to use angleDelta, assume 15deg/step and apply the number of steps. When no step is defined, it defaults to pixelStep, and it feels very slow because the step is very small. I would accept this because it fits with the metaphor and also because it would be the same behavior as in 3.13. I would restore this behavior and accept that it feels slow when no step is provided. Alt and Ctrl work on all platforms to use a bigger or smaller step (and I would accept that shift doesn't work on macos, since it seems the axes swap is done by system and not by Qt). So I would also keep the "Alt" and "inverted" countermeasures put so far. |
Yeah, I think that too... But also this seems to be really close to be done right, at least on some systems. How do you want to proceed? |
|
I agree it feels close to right, but I see three problems:
So, I would like to go back to the previous paradigm, and to use only angleDelta. After all these are not scrolling contents, but adjustable stepping controls. I also thought it would make sense to have a visual paradigm when the step is not defined, but then I also feel it should be configurable somehow, and such a big change of the default feeling could disrupt some user interfaces. We could also scale down pixelDelta to be comparable more or less with angleDelta (say, I hope I was clear, sorry for the long hairy discussion, I can also push a version soon if you want to see how it would feel |
Yeah, the non-configurable part is unfortunate.
For the slider, when steps were not defined, weren't we effectively using the visual proportion though?
Yeah, the platform-specific differences are fragile.
Okay. This will possibly degrade experience on macOS (angleDelta doesn't have the speed-based scaling and inertia IIUC), but OTOH on macOS scrolling with touchpad seems not really present in the UI paradigm so maybe we don't need to care about it.
Again, isn't this what we had before?
If we use pixelDelta, the current scaling feels good, so I wouldn't change that... after all, pixelDelta should refer to the visual paradigm. But maybe it's okay to revert to angleDelta More comments:
|
Almost: we were mapping 15 degrees from angleDelta to a pixelStep. That is: a fixed amount of scroll to an increment that depends on widget size: If we find a stable way to convert pixelDelta to angleDelta, we could still use pixelDelta, to keep the macos extra features like inertia. On my system is a more or less 10px/step, I suspect it will be different on other platforms, but if 10px/step turns out to be a good-enough average, and we don't get extreme differences, I'm ok to keep it!
I'm afraid it's quite a drastic change from 3.13, as @bgola said, around 20x, so I would still scale it down to match the old angleDelta scaling, at least at this stage.
I would keep all the collateral improvements: both the inverted and the Alt key things, and also the fractional step accumulation.
I copied this branch to a new branch on main repo: https://github.com/supercollider/supercollider/tree/topic/qt-scroll-widgets-visual For this PR, as a last attempt to keep pixelDelta, we can try to map 10px to a step and see if it's acceptable on all OSes. |
|
I applied the changes to restore 3.13 behavior, plus the pixelDelta scaling attempt. |
|
Hi @elgiano
Hm really? I thought that this line used
Great, thanks!
Here's the output on macOS. However, it shows up inconsistently in the post window (doesn't post for a while, then posts a bunch of lines at once) so I'm not sure what movement it actually corresponded to. DetailsI think the slower scroll on macOS with touchpad is fine, with inertia etc it's easy enough to change the value I think, even if it doesn't correspond to the physical distance on screen. Also, we were hoping with @capital-G to release 3.14.0-rc2 in about 24h... (after 18:00 UTC on Monday), do you think this is good in the current form (minus the printing of course)? Do we need more testing on Linux - X11 or Waylwand? Also, I tested this build on Windows 10. Scrolling is slow, but I think it's okay. Curiously, inverted scroll setting is not detected (i.e. when turned on, the widgets scroll in opposite direction). Nothing we can do about it, just FYI. |
|
Ok, thanks! I didn't expect such a difference in px/step, but since you say it feels ok I would leave it as it is.
This line was changed in 9d10e19, which is the commit that replaced the deprecated |
I think this happens when you "fling" fingers over touchpad to quickly scroll. |
I meant the difference in px/step = 60 (which never changes in your example), compared to the values I get on Wayland (~10) and X (120, but anyway we are not using pxDelta on X). |
Ah, got it. |
dyfer
left a comment
dyfer
left a comment
There was a problem hiding this comment.
Thanks!
I think this is a net improvement and can go in as is.
One point I want to clarify: sliders with larger steps are now scrolled "by step", i.e. a fixed amount of movement of the touchpad/wheel will always advance by one step. (Do we still use the fractional step accumulation?). This feels "too large" for large steps with the touchpad, but I think it's reasonable for mouse scroll wheel movement. The behavior is I think similar to 3.13, but scroll was so much slower in 3.13 that it's hard to tell.
|
I had a typo in the last commit (checking "x11" instead of "xcb"). EDIT: we still use fractional step accumulation, which makes a big difference with how 3.13 worked. On 3.13 in wayland, I can't scroll a slider with step=0.5 at all, while with this PR i can I think in the future if we want to use pixelDelta, we have to to switch to the visual paradigm, which also makes sense with high-res scrolling to make small or big adjustments. But I also don't want to keep going in circles :D so if this behavior is good enough for you, let's go with it and eventually change it in a later release. Alternatively, we can still go back to using only angleDelta to match perfectly the 3.13 behavior. |
|
@elgiano this is good for me as is, if you can vouch for behavior on Linux. I don't want to keep going in circles either. |
|
What a ride - thanks gianluca and marcin for tackling this <3 |
4 participants
Purpose and Motivation
Fixes #7008
When migrating from deprecated .delta() to .angleDelta()/.pixelDelta() (9d10e19)
there were some errors in scaling the delta values, and some probably typos turning vertical scaling to horizontal.
angleDelta()is a direct translation of the old deprecateddelta(), so I think we could skip checking forpixelDelta()in widgets like QcSlider and just useangleDelta(), to get the old behavior back. I'm not sure if using pixelDelta gives any better precision/smoothness in this case. However, for now I've kept alsopixelDelta, and changed it's scaling to a value that gives me the same feel as angleDelta (and as in 3.13).Can someone on a mac try it and report how does scrolling feel between this PR, 3.13 and 3.14-rc1? For me on linux (X11/Wayland) it's now the same across all versions.
While at it, I also added support for wheelEvent in other widgets. Full list of affected widgets:
Note: this is not about Qt6, as .angleDelta and .pixelDelta were introduced in Qt5, and .delta was deprecated already then
Types of changes
To-do list