-
-
Notifications
You must be signed in to change notification settings - Fork 955
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
focalX and focalY are wrong on android only #2138
Comments
Hey! 👋 It looks like you've omitted a few important sections from the issue template. Please complete Snack or minimal code example section. |
Hi! I tried to reproduce it on a simple view but it seems to be working fine in that case. Would you mind sharing the repository with the application you used to make a recordings? |
In the repro step, I have included my sample code. |
Thanks @j-piasecki Here are my update
I tried to use Not only this, I had a deep dive into this file ScaleGestureDetector.java. As you copy this from AOSP and it does not have a complex calculation for the focal point, it is hard for me to believe it is the bug on android side. But I have to say it is because motionEvent getX and getY return this value. Does it make sense to you? |
I found the problem: Lines 248 to 254 in 3ff7674
Only the first point will have correct position, while others will still be scaled with respect to the upper-left corner of the view. I'll try to figure out a way to fix it without making breaking changes. |
@wood1986 As a workaround for now you can wrap the transformed <GestureDetector gesture={Gesture.Race(tap, pinch)}>
<Animated.View>
<Animated.View collapsable={false} style={animatedStyle}>
...
</Animated.View>
</Animated.View>
</GestureDetector> This way the coordinates will be consistent between tap and pinch. |
Thanks @j-piasecki. Your workaround does not work because the coordinate of that workaround is not based on scaled view space. it is based on the parent view space. If I tap on the 4 corners on a scaled view, I expect it should not be [0 - width] or [0 - height] |
I have just checked the history. This issue has been there for more than 3 years. Not sure if you can fix it quickly |
Yeah, unfortunately this is bigger than it looked like 😞. Fixing this properly would mean that all gestures would be calculated in the transformed coordinate space of the view, this would mean the speed of pan would depend on the scale, pan translation and the tap/long press coordinates affected by the rotation etc. That would also mean a reimplementing how the events are processed by the Gesture Handler. As to the workaround, you could make use of touch events the fact that the coordinates of the first pointer are correct - scaling the rest of the pointers with respect to the first one should allow you to calculate the correct focal point. |
Thanks. BTW how come iOS does not this issue? |
Gesture Handler on iOS uses UIGestureRecognizers under the hood, they allow us to calculate the location with respect to any view thanks to |
Hey @j-piasecki, as I really need this bug to be fixed otherwise android will never be able to have a perfect pinch to zoom for all react-native app, my question is if I can get the exact same I also try to scaling the rest of pointer and it does not work. I took the transformation matrix and then apply it to second point. the value is the not correct. maybe my math was wrong. |
It's hard to say right now, I'll look more into this and come back to you. Meanwhile, if you want to try one more thing, you could use the additional |
Thanks @j-piasecki, just let you know I want to help and fix it fundamentally. I have been looking at the place setLocation and trying to apply matrix transformation to the second point to see if I can get the right coordinates. But no progress. |
Lines 248 to 254 in 3ff7674
Based on the comment, I changed to this // TODO: we may consider scaling events if necessary using MotionEvent.transform
// for now the events are only offset to the top left corner of the view but if
// view or any ot the parents is scaled the other pointers position will not reflect
// their actual place in the view. On the other hand not scaling seems like a better
// approach when we want to use pointer coordinates to calculate velocity or distance
// for pinch so I don't know yet if we should transform or not...
handler.view?.let {
it.matrix.invert(inverseMatrix)
event.transform(inverseMatrix)
} It can return the touch position correctly. But it is flicking Do you know why? |
Unfortunately not, I've also tried it (and got the same result) but I haven't investigated it yet. |
Thanks @j-piasecki With your fix, the pinch to zoom behaviour is definitely better than the current. I found an issue when zoom out I have a few questions.
|
FYI: I do not know if you are using my repo for testing the fix. I did a clean up with a force push. you may need to clone again |
The flickering was caused because the scale factor was being calculated in the coordinate space of the view being scaled. Because of that, when the pointers moved the scale of the view would change, which in turn would change the position of the pointers relative to the view. I worked around it by calculating the scale factor in the coordinate view of the GestureHandlerRootView. I will continue to work on the draft, trying to figure out a way to solve the issues and, hopefully, find a way to integrate it without (or as little as possible) breaking changes. As it's a relatively large change to the core of the library I cannot guarantee when it will be merged. As for the help, pointing out all the issues you find would be greatly appreciated 🙂. |
Also, the same thing happens on iOS: Simulator.Screen.Recording.-.iPhone.13.-.2022-08-09.at.11.17.21.movSince, we're not using any custom logic for scaling and transforming events there, this suggests that something may be wrong in your code. |
Hey @j-piasecki, your PR fixes the major issue. But it still have some issue. Let me give you some context. I am trying to have the same pinch-to-zoom experience as the iOS built-in Photos app. And I am not sure if it is achievable When I try to pan with using 2 fingers and the existing scale, the image does not move. in iOS Photos, it moves When I pinch and then pan using 2 fingers, the image moves but I move a opposite direction. in iOS Photos, it moves the right direction As the event in Here is the https://github.com/wood1986/pinch-bug/tree/p2z |
This should do the trick: import React from 'react';
import { StyleSheet, SafeAreaView, View, Button } from 'react-native';
import {
Gesture,
GestureDetector,
GestureHandlerRootView,
} from 'react-native-gesture-handler';
import Animated, {
useAnimatedStyle,
useSharedValue,
useAnimatedRef,
measure,
} from 'react-native-reanimated';
import { identity3, Matrix3, multiply3 } from 'react-native-redash';
function translateMatrix(matrix: Matrix3, x: number, y: number) {
'worklet';
return multiply3(matrix, [1, 0, x, 0, 1, y, 0, 0, 1]);
}
function scaleMatrix(matrox: Matrix3, value: number) {
'worklet';
return multiply3(matrox, [value, 0, 0, 0, value, 0, 0, 0, 1]);
}
const ImageViewer = () => {
const ref = useAnimatedRef();
const origin = useSharedValue({ x: 0, y: 0 });
const transform = useSharedValue(identity3);
const scale = useSharedValue(1);
const translation = useSharedValue({ x: 0, y: 0 });
const pinch = Gesture.Pinch()
.onStart((event) => {
const measured = measure(ref);
origin.value = {
x: event.focalX - measured.width / 2,
y: event.focalY - measured.height / 2,
};
})
.onChange((event) => {
scale.value = event.scale;
})
.onEnd(() => {
let matrix = identity3;
matrix = translateMatrix(matrix, origin.value.x, origin.value.y);
matrix = scaleMatrix(matrix, scale.value);
matrix = translateMatrix(matrix, -origin.value.x, -origin.value.y);
transform.value = multiply3(matrix, transform.value);
scale.value = 1;
});
const pan = Gesture.Pan()
.averageTouches(true)
.onChange((event) => {
translation.value = {
x: event.translationX,
y: event.translationY,
};
})
.onEnd(() => {
let matrix = identity3;
matrix = translateMatrix(
matrix,
translation.value.x,
translation.value.y
);
transform.value = multiply3(matrix, transform.value);
translation.value = { x: 0, y: 0 };
});
const animatedStyle = useAnimatedStyle(() => {
let matrix = identity3;
if (translation.value.x !== 0 || translation.value.y !== 0) {
matrix = translateMatrix(
matrix,
translation.value.x,
translation.value.y
);
}
if (scale.value !== 1) {
matrix = translateMatrix(matrix, origin.value.x, origin.value.y);
matrix = scaleMatrix(matrix, scale.value);
matrix = translateMatrix(matrix, -origin.value.x, -origin.value.y);
}
matrix = multiply3(matrix, transform.value);
return {
transform: [
{ translateX: matrix[2] },
{ translateY: matrix[5] },
{ scaleX: matrix[0] },
{ scaleY: matrix[4] },
],
};
});
return (
<>
<GestureDetector gesture={Gesture.Simultaneous(pinch, pan)}>
<Animated.View
ref={ref}
collapsable={false}
style={[styles.fullscreen]}>
<Animated.Image
source={require('./1.png')}
resizeMode={'contain'}
style={[styles.fullscreen, animatedStyle]}
/>
</Animated.View>
</GestureDetector>
<View style={{ position: 'absolute', end: 0, backgroundColor: 'black' }}>
<Button
title="RESET"
onPress={() => {
transform.value = identity3;
}}
/>
</View>
</>
);
};
const styles = StyleSheet.create({
fullscreen: {
...StyleSheet.absoluteFillObject,
flex: 1,
width: '100%',
height: '100%',
resizeMode: 'contain',
},
pointer: {
width: 60,
height: 60,
borderRadius: 30,
backgroundColor: 'red',
position: 'absolute',
marginStart: -30,
marginTop: -30,
},
});
const App = () => {
return (
<GestureHandlerRootView style={{ flex: 1 }}>
<SafeAreaView style={{ flex: 1, backgroundColor: 'black' }}>
<ImageViewer />
</SafeAreaView>
</GestureHandlerRootView>
);
};
export default App; |
Your code is perfect!!!!! Thank you so much |
Originally, I thought 2 fingers pan were done in the pinch gesture. In fact, it is handled by pan gesture. I was wrong at the beginning. Without you I will never be able to implement that. Once you merge this code, I plan to add this example to this repo. Maybe make a component for people to use |
## Description Gesture Handler on Android intercepts events via the `GestureHandlerRootView`, which means that we need to transform the touch coordinated by ourselves to check if it's inside the view. Similarly, we need to transform the event when dispatching it to one of the gesture handlers in order to correctly calculate the changes and send touch events. The current method works well until there is more than one pointer at once on the screen. When there is more than one pointer, only the first one will be correctly transformed, while others will simply be moved by the offset calculated when transforming the first pointer. This PR changes the logic when delivering events to gesture handlers - the whole event is transformed, and handlers receive both events - transformed and untransformed one. This way things like touch events and `NativeViewGestureHandler` which rely on the location in the coordinate space of the view keep working, and other gestures get to work in the coordinate space of the root view. Fixes #2138. ## Test plan Tested on the Example app.
Thanks a ton for #2138 (comment), this was the only example I could find that handles focal pinching/panning correctly. For future readers who also want to have pan constrained by the bounds (like in stock Android apps), I've extended this code to do that in bluesky-social/social-app#1563 — feel free to have a look. |
Description
I am trying to implement pinch to zoom and I pretty sure my math is correct. But the focalX and focalY are wrong on android only
Android
iOS
On android, the first pinch-to-zoom behave "correct". The last pinch-to-zoom must be wrong. I tap two points and the coordinates are
x = 194.86 y = 351.61
andx = 194.32 y = 396.36
. Then I pinch and the focal point isx = 195.93 y = 464.81
. I expect the y-coord should be between two taps point. Now it is outside.351.61 <= 464.81 <= 396.36 ❌ ❌ ❌ ❌
. x-coord has the same issue.When you take a look iOS, it behaves correctly
313 <= 351 <= 374 ✅✅✅✅
I think the bug is on either react-native or react-native-gesture-handler. I do not think it is react or react-native-reanimated bug
Platforms
Screenshots
Steps To Reproduce
git clone https://github.com/wood1986/pinch-bug.git
yarn android
Expected behavior
tap[0].x <= focalX <= tap[1].x
tap[0].y <= focalY <= tap[1].y
Actual behavior
focalX <= tap[0].x or tap[1].x <= focalX
focalY <= tap[0].y or tap[1].y <= focalY
Snack or minimal code example
Package versions
The text was updated successfully, but these errors were encountered: