feat: android transform origin #38558

intergalacticspacehighway · 2023-07-23T09:51:50Z

Summary:

This PR adds transform-origin support for android to make it easier to review. #37606 (review) by @javache. I'll answer feedback from @javache below.

Changelog:

[Android] [ADDED] - Transform origin

Test Plan:

Run iOS RNTester app in old architecture and test transform-origin example in TransformExample.js.

intergalacticspacehighway · 2023-07-23T09:54:33Z

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

@@ -40,7 +40,7 @@
 * provides support for base view properties such as backgroundColor, opacity, etc.
 */
 public abstract class BaseViewManager<T extends View, C extends LayoutShadowNode>
-    extends ViewManager<T, C> implements BaseViewManagerInterface<T> {
+    extends ViewManager<T, C> implements BaseViewManagerInterface<T>, View.OnLayoutChangeListener {


layout on a view is updated here. We can use onSizeChanged callback from ReactViewGroup instead of a listener here but we'll have to move the transform logic there and might need refactor as MatrixHelper class is in uimanager package and has some fields private. Also do some type cast here to call ReactViewGroup's set transform.

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

intergalacticspacehighway · 2023-07-23T10:09:59Z

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

+    if (offsets != null) {
+      MatrixMathHelper.resetIdentityMatrix(helperMatrix);
+      MatrixMathHelper.applyTranslate3D(helperMatrix, offsets[0], offsets[1], offsets[2]);
+      MatrixMathHelper.multiplyInto(result, result, helperMatrix);


answering #37606 (comment)
Multiply is required because the operation is order dependent. i.e. we need to multiply the translate operations or else the view is not translated to the transform-origin before applying other transforms 😅

We also reset the helperMatrix which behaves weird when there are multiple view transform animations running. I think it's reusing the instance between different animations.

analysis-bot · 2023-07-23T10:15:24Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,832,169	-130,625
android	hermes	armeabi-v7a	8142161	n/a
android	hermes	x86	9337701	n/a
android	hermes	x86_64	9180636	n/a
android	jsc	arm64-v8a	9,450,208	-103,737
android	jsc	armeabi-v7a	8632229	n/a
android	jsc	x86	9533110	n/a
android	jsc	x86_64	9776308	n/a

Base commit: 0b6e9bf
Branch: main

facebook-github-bot · 2023-08-21T18:30:57Z

@rozele has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

NickGerleman · 2023-08-23T00:13:50Z

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

@@ -45,7 +45,7 @@ private static double convertToRadians(ReadableMap transformMap, String key) {
    return inRadians ? value : MatrixMathHelper.degreesToRadians(value);
  }

-  public static void processTransform(ReadableArray transforms, double[] result) {
+  public static void processTransform(ReadableArray transforms, double[] result, float viewWidth, float viewHeight, ReadableArray transformOrigin) {


It looks like TransformHelper.processTransform is exposed publicly, and the API was previously being used by react-native-svg. So the change is breaking.

got it. Added an overloaded method to make it backward compatible.

NickGerleman · 2023-08-23T00:15:08Z

packages/react-native/Libraries/StyleSheet/processTransformOrigin.js

+      switch (valueLower) {
+        case 'left':
+        case 'right': {
+          invariant(


It's subjective, but there was some discussion within the team before on whether we should be permissive of invalid styles during development. E.g. add a warning, instead of throwing an exception. I think we changed some other to be more permissive and just warn and no-op, since unhandled exception is disruptive during development/live reload, but I could see both ways.

cc - @jacobp100 need to handle this in all PRs. wdyt? maybe use console.warn?

Yeah I can make changes to this. If you let me know what way you want to go I'll update it in my PR (#38626)

packages/react-native/Libraries/StyleSheet/splitLayoutProps.js

NickGerleman · 2023-08-23T00:15:57Z

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

@@ -129,6 +132,33 @@ protected T prepareToRecycleView(@NonNull ThemedReactContext reactContext, T vie
    return view;
  }

+  @Override
+  public void onLayoutChange(View v,


nit: It seems potentially error prone that this is only called when a transform is present. I.e. I could imagine someone adding code here, not realizing that it isn't always called.

true, just didn't want to attach a listener if it was not needed. Should we write a comment maybe?

// Currently, the onLayout listener is only attached when transform-origin prop is being used.

also removed the listener when transform-origin is null.

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

…nsform origin is not present

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

javache · 2023-08-23T10:23:02Z

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

  @ReactProp(name = ViewProps.TRANSFORM)
  public void setTransform(@NonNull T view, @Nullable ReadableArray matrix) {
+    view.setTag(R.id.transform, matrix);
    if (matrix == null) {
      resetTransformProperty(view);
    } else {
-      setTransformProperty(view, matrix);
+      ReadableArray transformOrigin = (ReadableArray) view.getTag(R.id.transform_origin);
+      setTransformProperty(view, matrix, transformOrigin);
+    }
+  }
+
+  @Override
+  @ReactProp(name = ViewProps.TRANSFORM_ORIGIN)
+  public void setTransformOrigin(@NonNull T view, @Nullable ReadableArray transformOrigin) {
+    view.setTag(R.id.transform_origin, transformOrigin);
+    ReadableArray transformMatrix = (ReadableArray) view.getTag(R.id.transform);
+    if (transformMatrix != null) {
+      setTransformProperty(view, transformMatrix, transformOrigin);
+    }
+    if (transformOrigin != null) {
+      view.addOnLayoutChangeListener(this);
+    } else {
+      view.removeOnLayoutChangeListener(this);


Not sure what the best solution is, but because setters are called in series, we will be calculating the transform twice, once after setTransform and again after setTransformOrigin.

yes, i had this concern. I think we can call in onAfterUpdateTransaction when both values are set. While testing I noticed that onAfterUpdateTransaction is not called for animating values. So I guess we still call it in setters but use onAfterUpdateTransaction for the initial mount.

@javache pushed an optimization. This works with animated values as well as fabric. onAfterUpdateTransaction is triggered reliably. I did something wrong in my earlier observations. I don't like this id approach but can't use instance variable as View manager can manage multiple views. Another approach would be to move the transform stuff in RootViewGroup, but that will require type casting the generic view in Manager and will need to move some files. Let me know wyt! Also thanks for the quick reviews and sorry to disturb during your parental leaves!

javache · 2023-08-24T11:58:59Z

This should be rebased on #38559 so we can land them in that order.

facebook-github-bot · 2023-08-29T10:08:01Z

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Summary: This PR adds transform-origin support for android to make it easier to review. facebook#37606 (review) by javache. I'll answer feedback from javache below. ## Changelog: [Android] [ADDED] - Transform origin <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: [ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: facebook#38558 Test Plan: Run iOS RNTester app in old architecture and test transform-origin example in `TransformExample.js`. Differential Revision: D48528339 Pulled By: javache fbshipit-source-id: 09e0c9ef569b7e9131da2f6efa9ba057aa98ff82

Summary: Adds transform-origin for old arch iOS See also intergalacticspacehighway's work for iOS Fabric and Android:- - facebook#38559 - facebook#38558 ## Changelog: [IOS] [ADDED] - Added support for transform-origin on old arch Pull Request resolved: facebook#38626 Test Plan: See RN tester Differential Revision: D48528353 Pulled By: javache fbshipit-source-id: 0189f374c9556b6593b3d72d7503b9cf166378c2

Summary: Adds transform-origin for old arch iOS See also intergalacticspacehighway's work for iOS Fabric and Android:- - #38559 - #38558 ## Changelog: [IOS] [ADDED] - Added support for transform-origin on old arch Pull Request resolved: #38626 Test Plan: See RN tester Reviewed By: NickGerleman Differential Revision: D48528353 Pulled By: javache fbshipit-source-id: 09592411e26484d81a999a7e601eff751ca7ae0b

facebook-github-bot · 2023-09-01T23:08:52Z

@javache merged this pull request in 9e68599.

feat: android transform origin

83719e5

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jul 23, 2023

intergalacticspacehighway commented Jul 23, 2023

View reviewed changes

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

intergalacticspacehighway commented Jul 23, 2023

View reviewed changes

intergalacticspacehighway mentioned this pull request Jul 23, 2023

feat: transform-origin #37606

Closed

4 tasks

jacobp100 mentioned this pull request Jul 25, 2023

Implement transform-origin for old arch iOS #38626

Closed

move process transform origin on to js

3ecc8ec

intergalacticspacehighway changed the title ~~feat(android): transform origin~~ feat: android transform origin Jul 27, 2023

xxrlzzz mentioned this pull request Jul 31, 2023

feat: Use animation matrix For Skew prop #38494

Closed

NickGerleman reviewed Aug 23, 2023

View reviewed changes

packages/react-native/Libraries/StyleSheet/splitLayoutProps.js Outdated Show resolved Hide resolved

NickGerleman reviewed Aug 23, 2023

View reviewed changes

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

intergalacticspacehighway added 3 commits August 23, 2023 10:52

fix: pr feedback

1f4abea

fix: pr feedback

71a7be5

fix: handle translate for matrix transform and remove listener if tra…

c494b02

…nsform origin is not present

javache reviewed Aug 23, 2023

View reviewed changes

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

javache reviewed Aug 23, 2023

View reviewed changes

intergalacticspacehighway added 2 commits August 23, 2023 21:09

fix: optimise transform

070787f

reset invalidate_transform view tag

bf53d65

javache approved these changes Aug 24, 2023

View reviewed changes

facebook-github-bot closed this in 9e68599 Sep 1, 2023

facebook-github-bot added the Merged This PR has been merged. label Sep 1, 2023

intergalacticspacehighway mentioned this pull request Mar 30, 2024

feat(android): percentage support in translate #43193

Closed

intergalacticspacehighway deleted the feat/android-transform-origin branch August 9, 2024 22:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: android transform origin #38558

feat: android transform origin #38558

intergalacticspacehighway commented Jul 23, 2023

intergalacticspacehighway Jul 23, 2023 •

edited

Loading

intergalacticspacehighway Jul 23, 2023

analysis-bot commented Jul 23, 2023 •

edited

Loading

facebook-github-bot commented Aug 21, 2023

NickGerleman Aug 23, 2023

intergalacticspacehighway Aug 23, 2023

NickGerleman Aug 23, 2023

intergalacticspacehighway Aug 23, 2023

jacobp100 Aug 23, 2023

NickGerleman Aug 23, 2023

intergalacticspacehighway Aug 23, 2023

intergalacticspacehighway Aug 23, 2023

javache Aug 23, 2023

intergalacticspacehighway Aug 23, 2023

intergalacticspacehighway Aug 23, 2023 •

edited

Loading

javache commented Aug 24, 2023

facebook-github-bot commented Aug 29, 2023

facebook-github-bot commented Sep 1, 2023

feat: android transform origin #38558

feat: android transform origin #38558

Conversation

intergalacticspacehighway commented Jul 23, 2023

Summary:

Changelog:

Test Plan:

intergalacticspacehighway Jul 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

analysis-bot commented Jul 23, 2023 • edited Loading

facebook-github-bot commented Aug 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

intergalacticspacehighway Aug 23, 2023 • edited Loading

Choose a reason for hiding this comment

javache commented Aug 24, 2023

facebook-github-bot commented Aug 29, 2023

facebook-github-bot commented Sep 1, 2023

intergalacticspacehighway Jul 23, 2023 •

edited

Loading

analysis-bot commented Jul 23, 2023 •

edited

Loading

intergalacticspacehighway Aug 23, 2023 •

edited

Loading