use-gesture icon indicating copy to clipboard operation
use-gesture copied to clipboard

Improve pinch algorithm for wheel-based gestures

Open CAYdenberg opened this issue 3 years ago • 19 comments

Describe the bug

When monitoring onPinch states, the values reported by state.delta[0] become greater and greater in magnitude on each successive event, and do not reset even when the gesture ends.

My understanding of the documentation is that the delta simply represents the total change for one event (not one gesture).

Sandbox or Video

https://codesandbox.io/s/bold-beaver-w0opf?file=/src/App.js

Keep pinching in one direction (in or out) -- eventually the delta gets very large or very small.

Information:

  • React Use Gesture version: 10.1.6
  • Device: MacBook Air
  • OS: macOS Big Sur 11.6.2
  • Browser Chrome 96
  • Checklist:
  • [x] I've read the documentation.
  • [x] If this is an issue with drag, I've tried setting touch-action: none to the draggable element.

CAYdenberg avatar Nov 29 '21 22:11 CAYdenberg

Hi! Feel this is going to be either simple or a headache 😅

First, let me just say that this lib handles three ways of pinching:

  • pinching with fingers: in that case scale is calculated as the ratio between the initial distance between finger 1 and finger 2 and the current one.
  • pinching with the Magic Trackpad on Safari: in that case the lib relies on WebkitGestureEvent which exposes the scale attribute (so Apple does the job for us, but it's likely that the calculation is based on distance between fingers).
  • wheeling with control key: this is your use case. Pinching the trackpad in browsers other than Safari on a Mac is emulated by wheeling with the control key being pressed (you can see that for yourself when logging events). It is not straightforward to translate wheeling deltas into scale values that feel natural to the user.

Anyway in either scenarios, deltas aren't actually cumulative, they're simply the difference between state.offset and the previous state.offset (in other words the current scale and the scale from the last event frame, and the way delta is calculated is the same across all gestures this lib handles). You can have see for yourself by doing something like this:

https://codesandbox.io/s/ancient-tree-z4brn

Where this is arguably a bug, is how scale grows exponentially when using the wheel.

Because of the introduction paragraph above, scaling with wheel in the lib is handled differently than regular finger pinching:

if (event.type === 'wheel') {
  // this is used in your setup, ie Mac + Chrome, pinching is emulated through wheeling
  this.state.offset = V.add(movement, lastOffset)
} else {
  // when using fingers or Safari magic trackpad
  this.state.offset = [(1 + movement[0]) * lastOffset[0], movement[1] + lastOffset[1]]
}

And here is how movement is calculated when the wheel is used (I'm making a few shortcuts to make it easier to read):

const PINCH_WHEEL_RATIO = 36 // arbitrary value that I found empirically 🤷‍♂️

state._delta = [(-wheelValues(event)[1] / PINCH_WHEEL_RATIO) * state.offset[0], 0]
state.movement = V.add(state.movement, state._delta)

(Don't focus too much on _delta vs delta, _delta is used for internal calculations first as offset can be altered by rubberbanding, therefore delta is ultimately resolved when offset reaches its final value. But if there's no transformation, _delta and delta should be roughly identical)

So as you can see, the bigger the current scale, the bigger the delta, and the bigger the delta, the bigger the scale. In other words, this grows exponentially, giving the feeling that delta is cumulative (but it's not).

Although I felt that formula worked when actually scaling an element, your issue makes me realize that values may get a bit crazy at some point, and that the formula reaches its limits.

If this makes sense to you, I'll keep the issue open but change its title.

dbismut avatar Nov 30 '21 08:11 dbismut

If this makes sense to you, I'll keep the issue open but change its title.

Please do. I'm a bit bound by this, so I'm willing to contribute a fix, but can't make any commitment on when I'll get to it.

CAYdenberg avatar Nov 30 '21 18:11 CAYdenberg

@dbismut Could we also normalize this between mac / windows somehow? Right now, the offset on Windows (chrome) is much larger than on mac (chrome).

csenio avatar Jan 10 '22 14:01 csenio

This depends on the wheel velocity, which is something that I guess could be hardware related and possibly configured by each user. This is not something we could normalize.

I might be wrong but as of now I wouldn't know how to manage this.

dbismut avatar Jan 10 '22 14:01 dbismut

@dbismut just wondering if there is any progress on this? It's a blocker for upgrading from the old react-use-gesture package for me since this is how it behaved previously. If there isn't any progress, do you have some advice on how to emulate the old pinch delta behavior? Should I use memo to store the movement and get movement - memoMovement?

breauxna avatar Jul 07 '22 00:07 breauxna

Hey @breauxna would you mind sharing a sandbox that would show the problem with the current version of use-gesture?

dbismut avatar Jul 07 '22 10:07 dbismut

Hey @dbismut thanks for the quick reply. So we are currently using the onPinch gesture to handle zooming on a canvas. We are taking the delta coming from the onPinch gesture, scaling it and then adding or subtracting it to our current zoom level. We are also clamping it within some min and max scale bounds. Since the new delta on onPinch isn't relative to the current gesture (not sure if that's right, but don't know how else to explain it) but grows exponentially, it just kind of messes up our stuff. Adding and subtracting the delta just jumps to the min and max zoom level on our canvas since it gets so big.

I think I actually found a solution and was able to upgrade though. It seems like the onWheel gesture still has the relative delta, so I am just using that instead and checking for ctrl key.

breauxna avatar Jul 07 '22 22:07 breauxna

@breauxna just to clarify, onPinch now gives you directly scale and rotation. There's no need for multiplying delta to approximate the zoom. If you look at the exemples such as https://githubbox.com/pmndrs/use-gesture/tree/main/demo/src/sandboxes/card-zoom you should have a good indication on what to do to migrate from older versions.

dbismut avatar Jul 08 '22 06:07 dbismut

@dbismut good to know, thanks again for your quick response! Unfortunately it seems like that sandbox isn't working anymore :/

Cannot read properties of null (reading 'startsWith')

breauxna avatar Jul 08 '22 15:07 breauxna

Weird @breauxna. On which device?

dbismut avatar Jul 08 '22 18:07 dbismut

I'm using Chrome (103) on an Intel Mac (OS X 10.15). Looks to be line 39 in utils.ts absoluteDependencies[name].startsWith('0.0.0')

breauxna avatar Jul 08 '22 19:07 breauxna

@breauxna there seems to be something weird with codesandbox. Can you try saving the sandbox (this will fork it) and reload the page? It should work properly afterwards (or just use that sandbox https://codesandbox.io/s/green-currying-1siy43).

dbismut avatar Jul 09 '22 07:07 dbismut

@breauxna you said:

It seems like the onWheel gesture still has the relative delta, so I am just using that instead and checking for ctrl key.

Where is the relative delta found? I'm looking for it here:

 usePinch(
    ({ delta: [d], event, ...rest }) => {
      console.log(`d:`, d);
      console.log(`rest:`, rest);
      event.stopPropagation();
      zoomHelper.current.triggerChange &&
        zoomHelper.current.triggerChange(({ changeValue, value }) => {
          changeValue(value + d);
        });
    },
    {
      target
    }
  );

Is it somewhere here: image

I am not interested in a delta that varies based on the internally kept scale calculated by usePinch and just want a relative delta based on how much a user is wheeling or pinching that does not change based on a previous state.

Definitely found the current default delta to be a bit confusing.

Thanks! Thomas

tnrich avatar Jul 22 '22 21:07 tnrich

delta is, as mentioned in the docs, the difference between the current movement and the previous movement. In other words, it's the delta of the scale (which yes, is calculated internally, except for when the lib uses WebkitGestureEvents which natively return scale).

To get the delta you want I guess you could try to use state.values and use memo to compute the delta in the handler.

Anyway, the thread is about improving the scale algorithm, which is something I haven't touched in a while, sorry about this.

As in theory you shouldn't need to be using any delta and just rely on what the handler's state.

dbismut avatar Jul 22 '22 21:07 dbismut

@dbismut I think this is probably not the right library for my use case then unfortunately as all I want is to get the simple delta of the actual wheel/pinch

My use case has various ways of controlling the "zoom" state of a custom dna viewer component - pinching as well as a draggable slider. That's why I think relying on the internally calculated scale of the usePinch helper won't work.

That's a bit unfortunate cause I like the hook interface provided by this library. You sure there's no way to just expose that non-internally scaled delta?

Thanks!

tnrich avatar Jul 22 '22 21:07 tnrich

Maybe you could jump on the Discord and we could chat about your use case. I'm kinda interested at why you wouldn't be able to use this lib, even if you have other ways of controlling the zoom!

dbismut avatar Jul 22 '22 21:07 dbismut

@dbismut sure! How do I find your channel? My handle is tmoney on discord

tnrich avatar Jul 22 '22 21:07 tnrich

Thanks @dbismut I was able to put something together using the from option since I was controlling the scale level externally. Here's the relevant code:

usePinch(
    ({ delta: [d], event }) => {
      if (d === 0) return;
      event.stopPropagation();
      zoomHelper.current.triggerChange &&
        zoomHelper.current.triggerChange(({ changeValue, value }) => {
          changeValue(value + d * 5);
        });
    },
    {
      target,
      from: [zoomLevel]
    }
  );

This can be found in the OVE repo here - https://github.com/TeselaGen/openVectorEditor

tnrich avatar Jul 25 '22 19:07 tnrich

To remove scaling from the pinch gesture's delta, you can divide it by the offset:

onPinch: ({ delta: [scaledDelta], offset: [scale] }) => {
  const delta = scaledDelta / scale
  // ...
}

jmadelaine avatar Mar 07 '23 18:03 jmadelaine