Zoom plugin improvements #3388
Zoom plugin improvements #3388
Conversation
src/plugins/zoom.ts
Outdated
| } else { | ||
| this.wavesurfer.zoom(newMinPxPerSec) | ||
| this.container.scrollLeft = (pointerTime - newLeftSec) * newMinPxPerSec |
There was a problem hiding this comment.
Does it still keep the scroll position at the exact same time position? I think that was @HoodyHuo's original intention.
There was a problem hiding this comment.
I'd actually like some confirmation on that. In v7.3, this code was necessary to accomplish that. I noticed that for me in v7.5.0, the scroll position stayed the same even when I removed this code. I don't know if that's true universally, or just for my environment or configuration.
There was a problem hiding this comment.
You probably know that but just to clarify:
The default zoom behavior is to preserve the scroll position at the playhead position. E.g. you click on a 5-second mark, playhead goes there, then you zoom and your scroll position remains on the 5-second mark. It can be seen here: https://wavesurfer.xyz/examples/?timeline.js
With this plugin, the playhead position is ignored and it instead tries to stay at the same time position where you last scrolled. E.g. your playhead is at the initial 0 position, you scroll to a 10-second mark, zoom, and it stays at 10 seconds.
So I'm wondering if this change alters this behavior.
There was a problem hiding this comment.
Based on the use of e.clientX, I actually think it is supposed to zoom in/out from the cursor position. So if the cursor is at the right side of the waveform, zooming in will create the appearance that the waveforms are stretching out to the left. If the cursor is in the middle, the stretch effect will occur evenly from both sides of the cursor. This is regardless of playhead and even scroll position. @HoodyHuo could confirm
If that's the case, this code is still relevant regardless of changes in the latest version. I added the code back in, just to be safe anyway.
There was a problem hiding this comment.
True, you’re right. We should indeed keep that code then. The PR looks good to me now. Thank you!
|
Great work on this! Quick question: I noticed (using a mouse wheel), that holding shift doesn't have the expected result of scrolling/panning through the waveform horizontally. Would you want to include that improvement in this PR, or open a new one? |
|
@ffxsam that's a good idea! Please feel free to create a separate issue for it. |
|
@ffxsam I've added your request in #3389. 👍 @cmorbitzer thanks again for the amazing contribution! It's now released in 7.5.1. 🚀 |
|
@cmorbitzer @katspaugh I took this for a spin on my MacBook Air, and it works great! It's also pretty standard when compared against pro audio tools (Audition, iZotope RX, etc), in that down = zoom in, up = zoom out (or the opposite, depending on your Mac trackpad settings). 😆 The scrolling and panning action is perfect. Love this! |
|
Hey guys, do you suppose there ought to be some sort of debouncing to save CPU cycles? This is pretty intense. 🤯 Not sure what the answer here is. If you debounce, then the waveform simply doesn't redraw while scrolling, which wouldn't look right. My other idea is for a more "notched" zoom, a la Adobe Audition. Zoom.CPU.mp4 |
|
Adobe Audition's zoom, for reference: Audition.mp4 |
|
I suggest we continue this discussion in the forum. |
This makes changes to
ZoomPlugin, which was added in #3276. Each change is on its own commit on this branch, to make reversions/modifications easier if requested before approval.Fix scroll when using trackpad
The zoom is currently triggered by the wheel event. On a trackpad, horizontal scrolling fires a wheel event with both an X- and Y- delta, unless the user scrolls perfectly horizontally on the trackpad, which is not typical. This creates a poor UX when the user is attempting to scroll because the waveform errantly zooms simultaneously. This PR changes the event listener to perform a zoom only when the Y-delta is greater than the X-delta, which is the case for the pinch gesture.
Account for wheel speed when zooming (breaking change)
Currently the zoom level is adjusted by the same amount for each wheel event that is fired. This PR changes the event listener to adjust the zoom level in proportion to the scroll/pinch distance. This offers a more consistent UX than relying on the quantity of event instances. As a result,
scalechanges from a magnification factor to a coefficient. A scale of.2felt too unresponsive with the new formula so this changes the default to.5.Allow using zoom plugin when
minPxPerSecis 0 or defaultThe zoom formula that this change introduces swapped
scaleforminPxPerSecas the coefficient. Therefore, it is now possible to use this plugin without first having setminPxPerSecto a non-zero value.Add
maxZoomoptionOptional. Specifies a maximum
minPxPerSecvalue while zooming.Checklist