-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
[Android] Reset sMatrixDecompositionContext before applying transformations #25438
Conversation
Prior to this commit React Native on Android was not properly setting the transform's scale properly correctly when setting it to 0. In order to properly set one would need to use values close to 0, like 0.0001. This was happing due to BaseViewManager sharing sMatrixDecompositionContext across all views in a React Native app. In some cases the decomposeMatrix() method from the MatrixMathHelper would return early and not set any new values in sMatrixDecompositionContext (this is, the new transform values) and since this is a shared object the BaseViewManager would set the transform values from the a previous transform. In order to prevent this issue, before setting the new transform values always reset the sMatrixDecompositionContext values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work figuring out this issue! LGTM
cc @cpojer
Thank you very much, @janicduplessis ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😮
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
This pull request was successfully merged by @cabelitos in ab80001. When will my fix make it into a release? | Upcoming Releases |
@cpojer thank you very much |
Summary: Recover accidentally removed call to `sMatrixDecompositionContext.reset()` added in #25438. Reviewed By: mdvacca Differential Revision: D17136023 fbshipit-source-id: 01d351729b427a01a04e3a260f72c12da7b8d03a
…cebook#25438) Summary: Prior to this commit React Native on Android was not properly setting the transform's scale properly correctly when setting it to 0. In order to properly set one would need to use values close to 0, like 0.0001. This was happing due to BaseViewManager sharing sMatrixDecompositionContext across all views in a React Native app. In some cases the decomposeMatrix() method from the MatrixMathHelper would return early and not set any new values in sMatrixDecompositionContext (this is, the new transform values) and since this is a shared object the BaseViewManager would set the transform values from the a previous transform. In order to prevent this issue, before setting the new transform values always reset the sMatrixDecompositionContext values. ## Changelog [Android] [Fixed] - Reset sMatrixDecompositionContext before applying transformations Pull Request resolved: facebook#25438 Test Plan: Run the code below on an Android device/emulator and you should see the following results: ### Android without the patch - current implementation Notice that the scale 0 is not properly applied to the last rectangle and the second rectangle does not animate properly. ![android-not-working](https://user-images.githubusercontent.com/984610/60400418-d29e0500-9b49-11e9-8006-63d6956d3a44.gif) ### Android with the patch Everything works fine ![android-working](https://user-images.githubusercontent.com/984610/60400420-d5005f00-9b49-11e9-9025-f11a9ee62414.gif) ### iOS - current implementation Everything works fine ![ios-working](https://user-images.githubusercontent.com/984610/60400421-d6ca2280-9b49-11e9-9d81-608780c69936.gif) ```javascript /** * Sample React Native App * https://github.com/facebook/react-native * * format * flow */ import React from 'react'; import { Animated, Button, StyleSheet, View, Text, SafeAreaView } from 'react-native'; const HorizontalContainer = ({ children, title }) => ( <View style={styles.horizontalContainer}> {children} <Text style={styles.text}>{title}</Text> </View> ); const App = () => { const animValue1 = React.useRef(new Animated.Value(1)).current; const animValue2 = React.useRef(new Animated.Value(1)).current; const onStartAnim = React.useCallback(() => { animValue1.setValue(0); animValue2.setValue(0); Animated.sequence([ Animated.timing(animValue1, { toValue: 1, duration: 300, useNativeDriver: true, }), Animated.timing(animValue2, { toValue: 1, delay: 700, duration: 300, useNativeDriver: true, }) ]).start(); }, [animValue1, animValue2]); return ( <SafeAreaView style={styles.container}> <Button title="Start Animation" onPress={onStartAnim} /> <HorizontalContainer title="Animated scale from 0 to 1"> <Animated.View style={[styles.view, { transform: [{ scaleX: animValue1 }] }]} /> </HorizontalContainer> <HorizontalContainer title="Animated scale from 0 to 1 - delayed"> <Animated.View style={[styles.view, { transform: [{ scaleX: animValue2 }] }]} /> </HorizontalContainer> <HorizontalContainer title="Scale 0.4"> <View style={[styles.view, { transform: [{ scaleX: 0.4 }] }]} /> </HorizontalContainer> <HorizontalContainer title="Scale 0.2"> <View style={[styles.view, { transform: [{ scaleX: 0.2 }] }]} /> </HorizontalContainer> <HorizontalContainer title="Scale 0"> <View style={[styles.view, { transform: [{ scaleX: 0 }, { translateY: 100 }] }]} /> </HorizontalContainer> </SafeAreaView> ); }; export default App; const styles = StyleSheet.create({ text: { fontSize: 10, color: 'black', marginLeft: 10, }, horizontalContainer: { justifyContent: 'center', alignItems: 'center', flex: 1, flexDirection: 'row', }, container: { flex: 1, justifyContent: 'center', alignItems: 'center', backgroundColor: '#F5FCFF', }, view: { width: 100, height: 100, backgroundColor: 'indigo', marginVertical: 5, } }); ``` Closes facebook#25205 Closes facebook#6278 Closes facebook#6278 Differential Revision: D16071126 Pulled By: cpojer fbshipit-source-id: 50820229db2e3c22cf6296831413d26b42f57070
Summary
Prior to this commit React Native on Android was not properly setting
the transform's scale properly correctly when setting it to 0. In order
to properly set one would need to use values close to 0, like 0.0001.
This was happing due to BaseViewManager sharing sMatrixDecompositionContext
across all views in a React Native app.
In some cases the decomposeMatrix() method from the MatrixMathHelper would
return early and not set any new values in sMatrixDecompositionContext
(this is, the new transform values) and
since this is a shared object the BaseViewManager would set the transform values
from the a previous transform.
In order to prevent this issue, before setting the new transform values
always reset the sMatrixDecompositionContext values.
Changelog
[Android] [Fixed] - Reset sMatrixDecompositionContext before applying transformations
Test Plan
Run the code below on an Android device/emulator and you should see the following results:
Android without the patch - current implementation
Notice that the scale 0 is not properly applied to the last rectangle and the second rectangle does not animate properly.
Android with the patch
Everything works fine
iOS - current implementation
Everything works fine
Closes #25205
Closes #6278
Closes #6278