-
Notifications
You must be signed in to change notification settings - Fork 301
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
Comments
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:
Anyway in either scenarios, deltas aren't actually cumulative, they're simply the difference between 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 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 So as you can see, the bigger the current scale, the bigger the 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. |
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 Could we also normalize this between mac / windows somehow? Right now, the offset on Windows (chrome) is much larger than on mac (chrome). |
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 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? |
Hey @breauxna would you mind sharing a sandbox that would show the problem with the current version of use-gesture? |
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 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 good to know, thanks again for your quick response! Unfortunately it seems like that sandbox isn't working anymore :/
|
Weird @breauxna. On which device? |
I'm using Chrome (103) on an Intel Mac (OS X 10.15). Looks to be line 39 in utils.ts |
@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). |
@breauxna you said:
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
}
); 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! |
To get the delta you want I guess you could try to use 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 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! |
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 sure! How do I find your channel? My handle is |
Thanks @dbismut I was able to put something together using the
This can be found in the OVE repo here - https://github.com/TeselaGen/openVectorEditor |
To remove scaling from the pinch gesture's onPinch: ({ delta: [scaledDelta], offset: [scale] }) => {
const delta = scaledDelta / scale
// ...
} |
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:
touch-action: none
to the draggable element.The text was updated successfully, but these errors were encountered: