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

Smooth pinch with ResumableZoom #10

Closed
rrebase opened this issue Mar 26, 2024 · 2 comments · Fixed by #11
Closed

Smooth pinch with ResumableZoom #10

rrebase opened this issue Mar 26, 2024 · 2 comments · Fixed by #11
Assignees
Labels
question Further information is requested

Comments

@rrebase
Copy link

rrebase commented Mar 26, 2024

Description

Hey, first of all wanted to say that you've done a fantastic job with this library 🙇‍♂️

Tried the ResumableZoom component and it works well, there's only two places where it "jitters" on iOS device. Did some investigation and here's what I found:

  • When pinch is active and panning just a tiny bit then it "stutters". Traced it down to inaccurate focalX & focalY values on RNGH side Pinch gesture focalX & focalY truncated on iOS software-mansion/react-native-gesture-handler#2833
  • When releasing a pinch, then it often jumps. Apparently there's a few updates that happen with one touch / finger instead of two. This leads to a significant change in focal point. Possible solution is to add a check here to only handle the update if e.numberOfPointers === 2
@rrebase rrebase added the question Further information is requested label Mar 26, 2024
@Glazzes Glazzes self-assigned this Mar 26, 2024
@Glazzes
Copy link
Owner

Glazzes commented Mar 26, 2024

Hello there 👋, thank you for your kind comments and this awesome contribution, here's what I come up with about the issues you've described.

The Jittering

I tried simulating the problem with the focal point lack of decimal places by using Math.round, floor and ceil and they are responsible for the jittering as you mentioned, for this issue I'm tied to whatever Software Mansion decides to do about it, there's nothing I can do about it, however I can probably come up with a workaround using the pan gesture in the meantime.

Could you please create a pan gesture which uses minPointers property set to two, then print x and y event properties to see if they have decimal places? I currently don't have access to a mac and Iphone, so let me know if they have them.

const pan = Gesture.Pan()
  .minPointers(2)
  .maxPointer(2)
  .onPanUpdate(e => {
    console.log(e.x, e.y);
  });

Jump at the end of the pinch gesture

Observations

You're right about assuming its caused by a single pointer triggering undesired updates, however the gesture triggering such updates is actually the pan gesture, however this activation's behaviour is abnormal because of the conditions that trigger it, from my perspective a couple of onPanChange callback updates appear to outrun onPanEnd callback, I actually fixed a way more severe error related to this one, by mixing some code that should be in onPanEnd to onPanchange, this line right here, I assumed it was because I do not have a decent phone.

What is the root cause of this issue?

All gestures are disabled automatically at the very beginning of onPinchEnd callback, it seems the pan gesture manages to activate as it's getting disabled and at the middle of the snapback animation of ScaleMode.BOUNCE, then it gets clamped right away by the onPanChange.

Solution

Disabling pan gesture at the beginning of pinch gesture so they can not collide with each other when pinch gesture ends, disabling pan gesture makes no effect to "pan with pinch feature" as pan gesture is not responsible for it. following this action seems to fix the jump issue as I have not been able to replicate it anymore.

I'm not able to obtain a single pointer with the pinch gesture, however I will add the pointer check to prevent any strange and undesired issues.

I'll work on it tomorrow morning to ensure it works properly this time.

Glazzes added a commit that referenced this issue Mar 27, 2024
Disable pan gesture at the beginning of pinch gesture instead of the end
this change has been applied to both ResumableZoom and CropZoom

refs #10
@Glazzes Glazzes linked a pull request Mar 28, 2024 that will close this issue
@Glazzes
Copy link
Owner

Glazzes commented Mar 28, 2024

After many unsuccessful attempts to implement a workaround for this feature I could not make it work.

  • Using a pan based approach with two pointers outputs the same focal point a pinch gesture does, however its behaviour is not reliable at all, produces sudden jumps and jittering.
  • Using pan gesture's translationX and translationY value result in a displacement of the image that prevents it from being aligned with the user's interaction.
  • Manual gestures can probably be an option, I managed to simulate the focal points position, however for the scale value the end result is an image that looks to be a two different places at the same time.

Best I can do at this point is to make panWithPinch an opt in feature for iOS users and warn them in the documentation until a release of Gesture Handler with your fix comes out.

You can test again whether the jump issue has gone away with the new version 1.0.2.

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

Successfully merging a pull request may close this issue.

2 participants