peaks.js icon indicating copy to clipboard operation
peaks.js copied to clipboard

Continuous & mousewheel zoom

Open Thanzex opened this issue 5 years ago • 17 comments

I'd like to use peaks but the project I'm working on but it requires a continuous zoom action using the mouse wheel. I see this has been discussed a long time ago in #91, I know it's a quite old issue but it's still open, I can't find anything in the docs and there's no update on it. Is there a way to implement mouse wheel scroll, preferably continuous?

Thanzex avatar Jul 15 '19 11:07 Thanzex

There is an old and unmaintained branch with continuous zoom based on the approach described in #91, but it would take quite a bit of work to get that working again in the latest code base. If you're interested in working on this feature, I'd be happy to talk in more detail.

Adding mousewheel scroll using the existing zoom API should be straightforward, and wouldn't require changes to Peaks.js.

chrisn avatar Jul 16 '19 19:07 chrisn

This is semi-related, I think. I've added mousewheel support to allow scrolling the zoomview left/right. I have a branch in my fork and it's working perfectly.

All it came down to was adding a wheel handler in mouse-drag-handler.js, which passes the deltaX to a new onMouseWheel callback in waveform-zoomview.js:

      onMouseWheel: function(deltaX) {
        var newFrameOffset = Utils.clamp(
          self._frameOffset + deltaX, 0, self._pixelLength - self._width
        );

        self._updateWaveform(newFrameOffset);
      }

I can open a PR if you'd like! However, there are a couple of considerations:

  • This makes click/drag to scroll obsolete IMO. It would be nice to allow click/drag be a really easy way to create a new segment
  • Obviously if we changed the click/drag behavior, it would be a breaking change, so the appropriate version bump would be required

ffxsam avatar Jan 18 '21 22:01 ffxsam

@chrisn I just noticed this:

Adding mousewheel scroll using the existing zoom API should be straightforward, and wouldn't require changes to Peaks.js.

I'm not sure how this would've been possible just using the API. Is there some way to adjust the frame offset via the API that I've missed?

ffxsam avatar Jan 18 '21 23:01 ffxsam

Current progress below. I'm happy to open a PR and have people help hack on this! I'm sure it could be improved. But it's up to Chris how much of the library he's willing to change.

Note: the segment creation is not built-in. I did add zoomview.mousedown, zoomview.drag, and zoomview.mouseup which allowed my app to easily create the segment by dragging.

https://user-images.githubusercontent.com/12532733/104978547-a9f98c80-59c7-11eb-9005-eee65260daf2.mp4

ffxsam avatar Jan 19 '21 01:01 ffxsam

I've added mousewheel support to allow scrolling the zoomview left/right. I have a branch in my fork and it's working perfectly.

This sounds good, but how have you handled default behaviour, which is to scroll the page?

I'm not sure how this would've been possible just using the API. Is there some way to adjust the frame offset via the API that I've missed?

Not sure why I mentioned the zoom API, but rather zoomView.setStartTime().

This makes click/drag to scroll obsolete IMO.

Not all users have a mousewheel (e.g., I use Peaks.js with a laptop trackpad).

It would be nice to allow click/drag be a really easy way to create a new segment.

Yes, (there's a related issue: #190) The way I'd prefer to do this is add an API that puts the mouse handler into different modes.

  • Default: click/drag to scroll the waveform
  • Segment: click/drag to add segment

Applications using Peaks.js could then build their own UI for changing the mode.

chrisn avatar Jan 19 '21 12:01 chrisn

This sounds good, but how have you handled default behaviour, which is to scroll the page?

The wheel event is on the Konva stage. So as long as your mouse isn't over the waveform, the page is still scrollable.

stage.container().addEventListener('wheel', this._mouseWheel, false);

Not sure why I mentioned the zoom API, but rather zoomView.setStartTime().

That would work, though not ideal since you're specifying a time to offset to, not pixels. Though I wonder if I took the event.deltaX and just divided it by an arbitrary number, if I could get the same level of smoothness in scrolling as you see in the video above. I'll give it a shot! If I can move some of this custom code out of Peaks.js, that's the best route.

Not all users have a mousewheel (e.g., I use Peaks.js with a laptop trackpad).

I personally haven't used a trackpad in over a decade that didn't have horizontal & vertical scrolling capabilities, though I understand the desire to support older devices. I too am on a trackpad and it works with no issues.

I like that idea to allow switching between the two click/drag modes!

I'll give the zoomview.setStartTime() trick a shot to see if that works for horizontal scrolling. I could also very easily move the zoom in/out capability (shift + scroll) outside of Peaks.js too, especially since it's just calling self.setZoom(). Nothing special there.

Thanks, Chris!

ffxsam avatar Jan 19 '21 15:01 ffxsam

@chrisn Great tip on using zoomview.setStartTime()! I was able to move all of the scroll/zoom code outside of Peaks.js.

    <div
      ref="zoomview"
      class="zoomview"
      @wheel.prevent="zoomviewWheel"
    />
    zoomviewWheel(event: WheelEvent) {
      const zoomview = this.peaks.views.getView('zoomview');

      if (!zoomview) return;

      if (event.shiftKey) {
        // @ts-ignore
        const maxScale = zoomview._getScale(this.peaks.player.getDuration());

        zoomview.setZoom({
          // @ts-ignore
          scale: clamp(zoomview._scale + event.deltaY * 16, 64, maxScale),
        });
      } else {
        zoomview.setStartTime(
          // @ts-ignore
          zoomview.pixelsToTime(zoomview._frameOffset) + event.deltaX / 100
        );
      }
    },

ffxsam avatar Jan 19 '21 15:01 ffxsam

That's interesting, although it depends on some internals (anything not documented in the README, including things with underscore prefix).

Please do send a PR for the mouse wheel scroll change. The only issue I found is that you have to be careful to place the mouse pointer over the waveform before using the scroll gesture, otherwise the page will scroll or the browser may interpret it as a forward/back navigation gesture. But, it's certainly an easier gesture on a trackpad than drag scrolling.

chrisn avatar Jan 19 '21 15:01 chrisn

We could also add shift+scroll to change zoom. My main concern with this is performance when you have very long audio files. Our use case can involve audio files several hours long, and dynamically changing the zoom as in your demo would be very slow.

chrisn avatar Jan 19 '21 15:01 chrisn

Let me know exactly which changes you want. I've made a new fork here: https://github.com/reelcrafter/peaks.js

That fork, at this point in time, incorporates the following changes:

  • Mouse wheel/trackpad scroll (scroll left/right) replaces click/drag scroll
  • Mouse wheel/trackpad zoom (hold shift + scroll up/down)
  • Added event emitters for mousedown/drag/mouseup on zoomview (so I could add my own easy segment creation ability)
  • Touch scrolling still works on mobile devices

I decided to move the scroll/zoom back into the library itself. After I moved it out, I realized it left the library with the inability to scroll at all.. so it made sense to me to just put that back in.

We could also add shift+scroll to change zoom. My main concern with this is performance when you have very long audio files. Our use case can involve audio files several hours long, and dynamically changing the zoom as in your demo would be very slow.

Yeah, zoom can be a little choppy on longer waveforms. In my use case, most files won't be more than 8-10 minutes long. I can see how a stepped zoom might be better, and I may wind up changing this later based on user feedback.

Another idea for zoom is to put drag handles on the overview, which would allow people to resize that window, and only when they let go of the drag handle would the zoomview change zoom & redraw. See example from Adobe Audition below for their overview interaction:

https://user-images.githubusercontent.com/12532733/105063119-f7661000-5a40-11eb-9eb9-f8e52ac7d497.mp4

ffxsam avatar Jan 19 '21 16:01 ffxsam

Let me know exactly which changes you want.

Thank you, here's what I'm currently thinking:

Mouse wheel/trackpad scroll (scroll left/right) replaces click/drag scroll

I'm interested adding this, but without replacing click/drag to scroll. What I'd like to have is a method to configure it, such as:

zoomView.enableWheelScroll(boolean);
zoomView.enableDragScroll(boolean);

or (see below, about creating segments)

zoomView.setDragMode('scroll' | 'segment'):

We could put more flags into Peaks.init() options but I'd prefer not to, as there are a lot of options already, and these options can be set (and also changed) after construction.

Mouse wheel/trackpad zoom (hold shift + scroll up/down)

I would like to include this. I guess it could work in two modes: step between the current fixed zoom levels, or with continuous zoom. I'd prefer continuous zoom to be opt-in, enabled by a method call (as above). We can document the performance implications of using it, and perhaps work towards improving performance in future (see discussion in #91).

Added event emitters for mousedown/drag/mouseup on zoomview (so I could add my own easy segment creation ability)

These sound good. In general, what I want to do is provide documented extensibility points so that people can customise Peaks.js to their needs without having to fork and change the code directly.

That said, a built-in mode where you can click/drag to create segments would be a welcome addition, to resolve issue #190.

Touch scrolling still works on mobile devices

Yes, this is important.

chrisn avatar Jan 20 '21 13:01 chrisn

I think we could have just one method to enable/disable wheel scrolling, instead of two.

Maybe:

zoom.setScrollMethod('drag' | 'wheel');

I'm not too keen on the word "wheel" since it can be a trackpad in many cases. Maybe 'drag' | 'scroll' or 'drag' | 'gesture'?

Totally agree with all your points. And I agree with keeping the current behavior as default, just to avoid surprising anyone using this library with unexpected changes. All of this can and should be opt-in.

My time is really limited these days, so hopefully someone else has the bandwidth to add these changes! All the code is already written, so please feel free to copy from my fork—and heck, even improve upon it. I'll be adding more features to that fork as our customers ask for them or as I think of them.

ffxsam avatar Jan 21 '21 18:01 ffxsam

@ffxsam, this is awesome!

By using your fork, I have been able to add the ability to create segments on the fly. Thank you.

dutzi avatar Feb 19 '21 01:02 dutzi

@dutzi Glad you found it helpful!

ffxsam avatar Feb 19 '21 05:02 ffxsam

@ffxsam, do you mind sharing what made you ditch scroll-to-zoom? I checked out your PR, reverted the last commit, and it seemed to work fine.

dutzi avatar Feb 19 '21 10:02 dutzi

@dutzi Yeah, I just found it a bit too choppy in some cases, especially the longer the waveform. It's not the end of the world if our customers have to click + and - to zoom in and out. I've made it so they get the same zoom steps no matter how long the audio file (so they're not clicking 200 times to zoom in on a very long file).

const ZOOM_STEP_MULTIPLIER = 3;
const MIN_SCALE = 64;

// ...

        case 'in':
          this.zoomToScale(
            Math.max(
              this.zoomScale - this.track.duration! * ZOOM_STEP_MULTIPLIER,
              MIN_SCALE
            )
          );
          break;
        case 'out':
          this.zoomToScale(
            Math.min(
              this.zoomScale + this.track.duration! * ZOOM_STEP_MULTIPLIER,
              this.maxScale
            )
          );
          break;

ffxsam avatar Feb 19 '21 14:02 ffxsam

@ffxsam, yeah, makes sense. I hope I manage to find some time improving this, not sure where the bottleneck is, but I'm thinking maybe if we cache multiple representations of the waveform in multiple zoom-levels we could interpolate between each as the user zooms in and out. Kind of like mipmapping. Work for gaming, so who knows? 🤷

dutzi avatar Feb 19 '21 15:02 dutzi