feat: Use animation matrix For Skew prop #38494

xxrlzzz · 2023-07-18T12:43:48Z

Summary:

This PR intent to solve Skew property on Android: #27649.

After android Q, we can use setAnimationMatrix to implement css Transform. (This api can also be accessed before android Q)

Old method for implement Transform is maintain a local 4*4 matrix, after finish handle all properties decompose back to a 3*3 matrix, and set back to android view use api setTranslation, setRotation, setScale. There is no such a api to direct set skew property. But the native layer of android is using a 3*3 skia matrix to handle all those transform properties.
So we could handle those Transform on our side and call setAnimationMatrix directly.

There is also some shortage for this fix, a 3*3 matrix is unable to handle 3d rotation and perspective. We will fail back to old implementation when those property is used.

Note this pr may have a conflict with #37606.

Changelog:

[ANDROID] [Fixed] - Implement of css Transform property.

Test Plan:

RNTester App: Apis -> Animated -> Transform Styles.

facebook-github-bot · 2023-07-18T12:43:53Z

Hi @xxrlzzz!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

xxrlzzz · 2023-07-18T12:48:54Z

Original Effect:

original.mp4

Fixed Effect:

fixed.mp4

facebook-github-bot · 2023-07-18T13:09:47Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

analysis-bot · 2023-07-18T13:10:39Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	16,517,138	-1,364,411
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	19,887,736	-1,352,295
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: 8ff05b5
Branch: main

...es/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/TransformHelper.java

javache · 2023-07-18T13:22:57Z

...es/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/TransformHelper.java

+      }
+    }
+
+    for (int transformIdx = 0, size = transforms.size(); transformIdx < size; transformIdx++) {


You need to process all transforms in order, you can't have separate loops for translate and other properties, as it may cause the resulting matrix to be incorrect.

I don't sure disorder of properties will leds a incorrect result matrix, since every property are handled by matrix multiply rather direct set matrix element.
The reason for separate translate and other properties is we need to know the real center of the view to set origin of rotate and scale (default origin is the top-left point of the view, rather center of the view like it used to be)

xxrlzzz · 2023-07-26T12:03:47Z

I note there is already has setAnimationMatrix calls in BaseViewManager. This api has marked public after android api 21. Its normally free to call it with @SuppressLint("NewApi").

We can use implementation form AdnroidX transition library to access this api saftly.

xxrlzzz · 2023-07-26T12:06:15Z

Compatibility test (device with api before android Q)

2023-07-26.19.38.56.mov

2023-07-26.19.18.00.mov

xxrlzzz · 2023-07-31T02:36:10Z

This pr is current depend on #38558 to solve unable to get view width, height problem

DieTime · 2023-11-24T19:20:21Z

@xxrlzzz Is this PR still relevant? As I understand it, the PR on which this one depends was merged.

xxrlzzz · 2023-11-28T07:44:17Z

@xxrlzzz Is this PR still relevant? As I understand it, the PR on which this one depends was merged.

ok, I will continue this PR ASAP.

xxrlzzz · 2023-12-02T07:31:00Z

without transform-origin
https://github.com/facebook/react-native/assets/40893153/c6fc91f9-a7a2-4693-9cfe-41650f9e3696

with transform-origin
https://github.com/facebook/react-native/assets/40893153/8bbe1701-e736-4abe-b8d6-11b77785671e

javache

Nice work! Sorry for the delay on reviewing this, we should try to get this imported.

javache · 2024-01-31T17:15:43Z

...es/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/TransformHelper.java

+      float[] offsets = getTranslateForTransformOrigin(viewWidth, viewHeight, transformOrigin);
+      float originX = viewWidth / 2, originY = viewHeight / 2;
+      if (offsets != null) {
+        originX += offsets[0];
+        originY += offsets[1];
+      }


I think we should account for this by doing a transform as a first step and then the reverse transform as the last step, similar to processTransform

javache · 2024-04-15T10:54:49Z

While the implementation seems right here, I'm a bit concerned about adopting this given the Android SDK docs

This is used in animation frameworks, such as Transition. When the animation finishes, the matrix should be cleared by calling this method with null as the matrix parameter. Application developers should use transformation methods like setRotation(float), setScaleX(float), setScaleX(float), setTranslationX(float)} and setTranslationY(float) (float)}} instead.

There's also significant complexity being introduced here in terms of what code paths can potentially triggered. Adding a different transform property to the array can significantly change how the transform is set.

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 18, 2023

facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Jul 18, 2023

javache reviewed Jul 18, 2023

View reviewed changes

...es/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/TransformHelper.java Outdated Show resolved Hide resolved

javache reviewed Jul 18, 2023

View reviewed changes

xxrlzzz force-pushed the feat/android_skew branch from e2eeaaf to 14e6815 Compare July 23, 2023 15:05

xxrlzzz requested a review from javache July 25, 2023 11:01

xxrlzzz force-pushed the feat/android_skew branch from 14e6815 to 87482db Compare July 25, 2023 11:42

xxrlzzz changed the title ~~feat: Use animation matrix after android Q For Skew prop~~ feat: Use animation matrix For Skew prop Jul 26, 2023

xxrlzzz force-pushed the feat/android_skew branch from 87482db to 5ba54ff Compare July 26, 2023 12:12

xxrlzzz marked this pull request as draft July 31, 2023 02:34

xxrlzzz added 2 commits November 28, 2023 16:07

Use animation matrix after android Q

8475e22

Merge branch 'main' into feat/android_skew

faa408e

xxrlzzz force-pushed the feat/android_skew branch from 5ba54ff to faa408e Compare December 2, 2023 07:27

xxrlzzz marked this pull request as ready for review December 2, 2023 07:29

javache reviewed Jan 31, 2024

View reviewed changes

Merge branch 'main' into feat/android_skew

74cc237

xxrlzzz force-pushed the feat/android_skew branch from fe24a84 to 74cc237 Compare February 17, 2024 04:15

javache mentioned this pull request Mar 14, 2024

Simulate skew on Android #42676

Open

xxrlzzz closed this Apr 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Use animation matrix For Skew prop #38494

feat: Use animation matrix For Skew prop #38494

xxrlzzz commented Jul 18, 2023 •

edited

Loading

facebook-github-bot commented Jul 18, 2023

xxrlzzz commented Jul 18, 2023

facebook-github-bot commented Jul 18, 2023

analysis-bot commented Jul 18, 2023 •

edited

Loading

javache Jul 18, 2023

xxrlzzz Jul 19, 2023

xxrlzzz commented Jul 26, 2023

xxrlzzz commented Jul 26, 2023

xxrlzzz commented Jul 31, 2023

DieTime commented Nov 24, 2023

xxrlzzz commented Nov 28, 2023

xxrlzzz commented Dec 2, 2023

javache left a comment

javache Jan 31, 2024

xxrlzzz Feb 17, 2024

javache commented Apr 15, 2024

feat: Use animation matrix For Skew prop #38494

feat: Use animation matrix For Skew prop #38494

Conversation

xxrlzzz commented Jul 18, 2023 • edited Loading

Summary:

Changelog:

Test Plan:

facebook-github-bot commented Jul 18, 2023

Action Required

Process

xxrlzzz commented Jul 18, 2023

facebook-github-bot commented Jul 18, 2023

analysis-bot commented Jul 18, 2023 • edited Loading

javache Jul 18, 2023

Choose a reason for hiding this comment

xxrlzzz Jul 19, 2023

Choose a reason for hiding this comment

xxrlzzz commented Jul 26, 2023

xxrlzzz commented Jul 26, 2023

xxrlzzz commented Jul 31, 2023

DieTime commented Nov 24, 2023

xxrlzzz commented Nov 28, 2023

xxrlzzz commented Dec 2, 2023

javache left a comment

Choose a reason for hiding this comment

javache Jan 31, 2024

Choose a reason for hiding this comment

xxrlzzz Feb 17, 2024

Choose a reason for hiding this comment

javache commented Apr 15, 2024

xxrlzzz commented Jul 18, 2023 •

edited

Loading

analysis-bot commented Jul 18, 2023 •

edited

Loading