-
Notifications
You must be signed in to change notification settings - Fork 548
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Properly handle smooth scrolling #1270
Conversation
197fae6
to
eb13391
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sometimes using this feature (zooming past FFT_MIN_DB) to examine audio filter/resampler stopband attenuation and ripple. So it may be better to leave it as is or make FFT_MIN_DB configurable at runtime...
I've tested this PR. It works well on my laptop. |
Thanks for testing. Zooming/scrolling below FFT_MIN_DB causes the plotter's scroll logic to become glitchy, so if you need to go lower, a better solution would be reducing FFT_MIN_DB or making it configurable. As is, being able zoom below FFT_MIN_DB is a bug. |
Great point on CFreqCtrl, I actually forgot it supported frequency change with the scroll wheel till you mentioned it. I'll add an extra commit to this PR to fix that shortly. |
I now implemented a similar accumulator of scroll events for CFreqCtrl, so it's fixed there too. |
Most modern trackpads and some wired mice implement smooth scrolling, where frequent mouse events are sent with small deltas. The old calculation logic did not handle this properly, as it zoomed the same amount regardless of the size (delta) of the scroll event, resulting in excessively fast zoom and jitteriness. The new logic zooms proportional to the delta, allowing smooth and precise zoom on mice with smooth scrolling, while still also handling the large infrequent scroll events of old fashioned mice properly.
On devices with smooth scrolling (like most modern laptops), scroll events are frequent and have small deltas that are usually less than one step unless one is scrolling very quickly. Due to the tuning logic adjusting the frequency one "click" for every mouse "step", and rounding to the nearest click, slow scrolling for precise frequency adjustment on computers with smooth scrolling just gets lost to the rounding logic. This new logic accumulates scroll event deltas for frequency adjustment, making precise and comfortable tuning possible using smooth scrolling mice/trackpads.
In some situations, it was possible to zoom in a way that causes the minimum dB value in the plotter to become too low.
Depending on the vertical position and zoom/scale, the top most signal strength label was sometimes not being drawn despite the associated line being drawn and there being plenty of space to draw the label. This was due to a mismatch of <= being used for counting the lines, but < being used for counting the labels. The new logic makes the label indexing consistent with the line drawing indexing, while adding a check to make sure the label is not cut off vertically.
For computers with smooth scrolling that issue frequent scroll wheel events with small deltas.
Thanks for this, and sorry for the long delay in reviewing it. So far it looks like touchpad scrolling is working much better on Mac. I'll do a bit more testing and code review before getting this merged in. |
Touchpad scrolling also appears to work well on Ubuntu. |
On computers with smooth scrolling that issue frequent scroll events with small deltas (such as Mac laptops), the current scrolling logic behaves very poorly, as each event zooms the same amount regardless of its size (delta), resulting in overly fast and jittery zoom, and the cumulative effects of non-zooming small scroll events are lost due to rounding and integer discretization. This new logic handles smooth scrolling trackpads/mice properly for zooming, filter adjustment, and frequency adjustment, while also handling old fashioned mice with infrequent large scroll events correctly too.