Skip to content
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

Improve pinch algorithm for wheel-based gestures #401

Open
2 tasks done
CAYdenberg opened this issue Nov 29, 2021 · 19 comments
Open
2 tasks done

Improve pinch algorithm for wheel-based gestures #401

CAYdenberg opened this issue Nov 29, 2021 · 19 comments
Assignees

Comments

@CAYdenberg
Copy link

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:
  • I've read the documentation.
  • If this is an issue with drag, I've tried setting touch-action: none to the draggable element.
@dbismut
Copy link
Collaborator

dbismut commented Nov 30, 2021

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.

@CAYdenberg
Copy link
Author

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.

@dbismut dbismut changed the title onPinch deltas on Mac trackpad are cumulative Improve pinch algorithm for wheel-based gestures Nov 30, 2021
@csenio
Copy link

csenio commented Jan 10, 2022

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

@dbismut
Copy link
Collaborator

dbismut commented Jan 10, 2022

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.

@breauxna
Copy link
Contributor

breauxna commented Jul 7, 2022

@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?

@dbismut
Copy link
Collaborator

dbismut commented Jul 7, 2022

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

@breauxna
Copy link
Contributor

breauxna commented Jul 7, 2022

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.

@dbismut
Copy link
Collaborator

dbismut commented Jul 8, 2022

@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.

@breauxna
Copy link
Contributor

breauxna commented Jul 8, 2022

@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')

@dbismut
Copy link
Collaborator

dbismut commented Jul 8, 2022

Weird @breauxna. On which device?

@breauxna
Copy link
Contributor

breauxna commented Jul 8, 2022

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')

@dbismut
Copy link
Collaborator

dbismut commented Jul 9, 2022

@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).

@tnrich
Copy link

tnrich commented Jul 22, 2022

@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

@dbismut
Copy link
Collaborator

dbismut commented Jul 22, 2022

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.

@tnrich
Copy link

tnrich commented Jul 22, 2022

@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!

@dbismut
Copy link
Collaborator

dbismut commented Jul 22, 2022

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!

@tnrich
Copy link

tnrich commented Jul 22, 2022

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

@tnrich
Copy link

tnrich commented Jul 25, 2022

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

@jmadelaine
Copy link

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
  // ...
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants