feat: transform-origin #37606

intergalacticspacehighway · 2023-05-27T12:11:40Z

Checklist

iOS new arch
Android new arch
iOS/Android old arch
Add examples

Summary

This PR adds transform-origin support, similar to web. Spec

Why?

By default, rotate/scale/skew transforms occur around the center of the View. If we want to perform them around the top-left of the view, we need to first translate to the top/left point, perform the transform, and then translate back to the center. This can be achieved with the following steps:

 transform: [
   { translateX: -viewWidth / 2 }, 
   { translateY: -viewHeight / 2 }, 
   { scale: animatedV.value }, 
   { translateX: viewWidth / 2 }, 
   { translateY: viewHeight / 2 }
]

The above approach requires View dimensions, that need to be calculated using onLayout, measure, or using constants. Transform origin can simplify it and also bring it closer to the web feature parity.

Changelog:

[GENERAL] [ADDED] - transform-origin.

Test Plan:

Run pod install/rebuild android RN tester app and test transform origin example in Transform examples.
Test on fabric/paper architecture.

packages/react-native/React/Fabric/Mounting/RCTMountingManager.mm

analysis-bot · 2023-05-27T12:36:11Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	9,043,848	-1,379
android	hermes	armeabi-v7a	8,292,445	-2,054
android	hermes	x86	9,561,193	-227
android	hermes	x86_64	9,401,751	-2,002
android	jsc	arm64-v8a	9,603,220	-1,199
android	jsc	armeabi-v7a	8,729,244	-1,890
android	jsc	x86	9,691,421	-44
android	jsc	x86_64	9,935,977	-1,831

Base commit: c803a5b
Branch: main

packages/react-native/React/Views/RCTViewUtils.m

javache

Thanks for contributing this! I have some concerns about how this works when you just change the transformOrigin prop and the transform remains stable.

packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm

packages/react-native/React/Fabric/Mounting/RCTMountingManager.mm

packages/react-native/React/Views/RCTViewManager.m

packages/react-native/React/Views/RCTViewUtils.m

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

packages/rn-tester/js/examples/Transform/TransformExample.js

packages/react-native/React/Views/RCTViewUtils.h

javache · 2023-05-31T08:39:26Z

High-level thought on the approach here: Could we applytransformOrigin as we calculate the transform matrix on the ShadowNode/ShadowView instead, and avoid having to mutate the transform in the mounting layer?

An example of what's going to be incorrect here now is any of the measure callbacks which respect transforms:

react-native/packages/react-native/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp

Line 101 in 0e83c53

layoutMetrics.frame = layoutMetrics.frame * ancestorNode.getTransform();

intergalacticspacehighway · 2023-06-04T12:25:55Z

High-level thought on the approach here: Could we applytransformOrigin as we calculate the transform matrix on the ShadowNode/ShadowView instead, and avoid having to mutate the transform in the mounting layer?

i am currently mutating it from the mounting layer because we need the View dimensions for transform origin. From high level i can think of two approaches, either get the transform in shadow node calculation from the native view or create a state for transforms in shadow node and reset it from the mounting layer. Does this make any sense?

javache · 2023-06-05T09:21:12Z

i am currently mutating it from the mounting layer because we need the View dimensions for transform origin.

ShadowNode / ShadowView have the full layout information available, so ideally getTransform correctly represents the transform, accounting for the transformOrigin. I'd recommend making the change at that layer.

Please also add a test that shows this works correctly when animations of the transform property are used.

intergalacticspacehighway · 2023-06-09T07:09:34Z

@javache can you check this approach? https://github.com/facebook/react-native/compare/main...intergalacticspacehighway:react-native:f/transform-origin-resolve?expand=1

It adds a resolveTransform function that returns a new transform based on the transform-origin and layoutMetrics. This makes sure we get the updated transform in shadow node's getTransform. This won't require us to mutate the transform prop. (only tested on fabric iOS. If this looks good, I can try to add it for Android/old Arch)

javache · 2023-06-13T22:47:13Z

@intergalacticspacehighway That looks very promising! Can you add it here, and I'll add some comments?

intergalacticspacehighway · 2023-06-18T06:03:54Z

@javache sorry for the delay, been out for a trip. I just merged the new approach here so you can review. I'll add the support for old arch and android.

javache · 2023-06-19T10:02:02Z

packages/react-native/ReactCommon/react/renderer/components/view/ViewProps.cpp

+  std::istringstream iss(transformOrigin);
+  std::string part;
+  for (int i = 0; std::getline(iss, part, ' ') && i < 3; i++) {
+    auto percentPos = part.find('%');
+    bool isPercent = percentPos != std::string::npos;


istringstream and getline are pretty inefficient string parsing methods.

I'd use std::find instead to find the next space (or end), then use string_view (like here) to parse it.

Instead of parsing this on every transform calculation, consider parsing it as and storing it as a YGValue, using YGUnitPercent, YGUnitPoint and YGResolveValue

TODO: this still needs to move away from getline, and look at adopting YGValue to avoid parsing this on every layout change

javache · 2023-06-19T10:05:34Z

packages/react-native/ReactCommon/react/renderer/components/view/ViewProps.cpp

+    } else if (part == "top") {
+       origin[1] = 0.0f;
+    } else if (part == "bottom") {
+       origin[1] = static_cast<float>(viewHeight);
+    } else if (part == "left") {
+       origin[0] = 0.0f;
+    } else if (part == "right") {
+       origin[0] = static_cast<float>(viewWidth);


This will currently accept the input "top left", which is not spec complaint. Do browsers accept?

yeah, the top left should work. It'll result in -viewWidth/2 , -viewHeight/2 as the offsets. It also works in any order in browser i.e. top left, left top. Invalid inputs could be left left, that results in -viewWidth/2 0 offsets so it also works same as browsers. left, right affects horizontal offset and top ,bottom vertical and center is the initial value - Spec. So, right always take priority over center even if comes later in order e.g. right center, center right results in the same origin. I can surely add test cases!
I am out this week as well. I'll pick this up as soon as I am back home! 😅

intergalacticspacehighway · 2023-06-28T10:17:46Z

@javache I've updated the code so it works on old/new arch + android/ios. However, I think there is a fundamental issue with the approach I have taken to make it work on Android and iOS old arch. Ideally, I want to implement a solution where it triggers transform updates from cpp/shadow node realm, so I don't have to repeat things for respective platforms.
i.e. once we have the layout info in shadow nodes we can trigger an imperative update call to native side to update the view transforms and not use the existing setter prop approach. wdyt? I'll have to dig more for this. Let me know what you think! The current approach works with native driver as well but has some redundancy/not very clean.

javache

The duplication for the old architecture is fine. No way to avoid that right now.

Do you want to split this in a separate diff for iOS and Android (and potentially even old and new architecture) to make this more reviewable?

javache · 2023-07-04T15:08:27Z

packages/react-native/React/Views/RCTConvert+Transform.h

@@ -9,6 +9,6 @@

 @interface RCTConvert (Transform)

-+ (CATransform3D)CATransform3D:(id)json;
+ (CATransform3D)CATransform3D:(id)json viewWidth: (CGFloat) viewWidth viewHeight: (CGFloat) viewHeight transformOrigin: (NSString*) transformOrigin;


For backwords compatibility we should keep the old method signature around too (but have it just call the new method with 0 widht/height and nil origin)

javache · 2023-07-04T15:09:25Z

packages/react-native/React/Views/RCTConvert+Transform.m

  return transform;
 }

+ (NSArray *)getTranslateForTransformOrigin:(CGFloat)viewWidth viewHeight:(CGFloat)viewHeight transformOrigin


Put this in a static (local) method, no need to have it live on RCTConvert

javache · 2023-07-04T15:09:44Z

packages/react-native/React/Views/RCTConvert+Transform.m

+ (NSArray *)getTranslateForTransformOrigin:(CGFloat)viewWidth viewHeight:(CGFloat)viewHeight transformOrigin
+:(NSString*)transformOrigin {
+  if (transformOrigin.length == 0 || (viewWidth == 0 && viewHeight == 0)) {
+    return @[@(0.0), @(0.0), @(0.0)];


Can we return nil in this case, and use that to avoid a call to CATransform3DTranslate?

javache · 2023-07-04T15:10:20Z

packages/react-native/React/Views/RCTConvert+Transform.m

+  CGFloat translateX = [offsets[0] floatValue];
+  CGFloat translateY = [offsets[1] floatValue];
+  CGFloat translateZ = [offsets[2] floatValue];
+  transform = CATransform3DTranslate(transform, translateX, translateY, translateZ);


Suggested change

CGFloat translateX = [offsets[0] floatValue];

CGFloat translateY = [offsets[1] floatValue];

CGFloat translateZ = [offsets[2] floatValue];

transform = CATransform3DTranslate(transform, translateX, translateY, translateZ);

if (offset) {

transform = CATransform3DTranslate(transform, [offsets[0] floatValue], [offsets[1] floatValue], [offsets[2] floatValue]);

}

javache · 2023-07-04T15:10:49Z

packages/react-native/React/Views/RCTConvert+Transform.m

@@ -141,7 +148,50 @@ + (CATransform3D)CATransform3D:(id)json
      RCTLogInfo(@"Unsupported transform type for a CATransform3D: %@.", property);
    }
  }
+
+  transform = CATransform3DTranslate(transform, -translateX, -translateY, -translateZ);


Suggested change

transform = CATransform3DTranslate(transform, -translateX, -translateY, -translateZ);

if (offset) {

transform = CATransform3DTranslate(transform, -translateX, -translateY, -translateZ);

}

javache · 2023-07-04T15:21:18Z

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

+    MatrixMathHelper.multiplyInto(result, result, helperMatrix);
+  }
+
+  public static float[] getTranslateForTransformOrigin(float viewWidth, float viewHeight, String transformOrigin) {


javache · 2023-07-04T15:22:29Z

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

@@ -60,6 +60,10 @@ public static void processTransform(ReadableArray transforms, double[] result) {
      return;


Is the transformOrigin already applied in the codepath above?

javache · 2023-07-04T15:23:01Z

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

+    MatrixMathHelper.applyTranslate3D(helperMatrix, offsets[0], offsets[1], offsets[2]);
+    MatrixMathHelper.multiplyInto(result, result, helperMatrix);


result is the identity matrix at this point, no multiplication should be required to do this.

javache · 2023-07-04T15:24:23Z

packages/react-native/ReactCommon/react/renderer/components/view/ViewProps.cpp

+  auto newTransform = Transform{};
+  std::array<float, 3> translateOffsets = getTranslateForTransformOrigin(viewWidth, viewHeight);
+  newTransform = newTransform * Transform::Translate(translateOffsets[0], translateOffsets[1], translateOffsets[2]);
+  newTransform = newTransform * transform;
+  newTransform = newTransform * Transform::Translate(-translateOffsets[0], -translateOffsets[1], -translateOffsets[2]);
+  return newTransform;


Suggested change

auto newTransform = Transform{};

std::array<float, 3> translateOffsets = getTranslateForTransformOrigin(viewWidth, viewHeight);

newTransform = newTransform * Transform::Translate(translateOffsets[0], translateOffsets[1], translateOffsets[2]);

newTransform = newTransform * transform;

newTransform = newTransform * Transform::Translate(-translateOffsets[0], -translateOffsets[1], -translateOffsets[2]);

return newTransform;

std::array<float, 3> translateOffsets = getTranslateForTransformOrigin(viewWidth, viewHeight);

auto newTransform = Transform::Translate(translateOffsets[0], translateOffsets[1], translateOffsets[2]);

newTransform = newTransform * transform;

newTransform = newTransform * Transform::Translate(-translateOffsets[0], -translateOffsets[1], -translateOffsets[2]);

return newTransform;

javache · 2023-07-04T15:25:11Z

packages/react-native/ReactCommon/react/renderer/components/view/ViewProps.cpp

+  std::istringstream iss(transformOrigin);
+  std::string part;
+  for (int i = 0; std::getline(iss, part, ' ') && i < 3; i++) {
+    auto percentPos = part.find('%');
+    bool isPercent = percentPos != std::string::npos;


TODO: this still needs to move away from getline, and look at adopting YGValue to avoid parsing this on every layout change

packages/react-native/Libraries/Components/View/ReactNativeStyleAttributes.js

intergalacticspacehighway · 2023-07-23T11:59:15Z

@javache Thank you for reviewing. I have made separate PRs and incorporated the changes you suggested. We can discuss it there.
iOS old arch - #38626
Android - #38558
iOS fabric arch - #38559

javache · 2023-08-21T16:05:34Z

Sorry for the delayed reviews, I was on parental leave. I'll get to this this week!

javache · 2023-08-23T10:29:01Z

Closing this PR as reviews are happening on the split PR's.

Summary: This PR adds transform-origin support for iOS fabric. This PR also incorporates feedback/changes suggested by javache in the original [PR.](facebook#37606) ## Changelog: [IOS] [ADDED] - Fabric 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#38559 Test Plan: Run iOS RNTester app in old architecture and test transform-origin example in `TransformExample.js`. Differential Revision: D48528363 Pulled By: javache fbshipit-source-id: 21cfe06db750c8cf55d43039f0189089d29fca6f

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: This PR adds transform-origin support for iOS fabric. This PR also incorporates feedback/changes suggested by javache in the original [PR.](#37606) ## Changelog: [IOS] [ADDED] - Fabric 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: #38559 Test Plan: Run iOS RNTester app in old architecture and test transform-origin example in `TransformExample.js`. Reviewed By: NickGerleman Differential Revision: D48528363 Pulled By: javache fbshipit-source-id: 347b7c5896ad19ad24278de81b0e055e4cb01016

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 <!-- 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: #38558 Test Plan: Run iOS RNTester app in old architecture and test transform-origin example in `TransformExample.js`. Reviewed By: NickGerleman Differential Revision: D48528339 Pulled By: javache fbshipit-source-id: e255a7f364e57396dada60b2c69c724cec406f51

intergalacticspacehighway referenced this pull request in intergalacticspacehighway/react-native May 27, 2023

transform origin android

aac94a8

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 May 27, 2023

intergalacticspacehighway commented May 27, 2023

View reviewed changes

packages/react-native/React/Fabric/Mounting/RCTMountingManager.mm Outdated Show resolved Hide resolved

nandorojo mentioned this pull request May 27, 2023

RFC: React DOM for Native (reduce API fragmentation) react-native-community/discussions-and-proposals#496

Open

intergalacticspacehighway marked this pull request as ready for review May 28, 2023 17:51

intergalacticspacehighway commented May 29, 2023

View reviewed changes

packages/react-native/React/Views/RCTViewUtils.m Outdated Show resolved Hide resolved

javache requested changes May 30, 2023

View reviewed changes

javache reviewed May 31, 2023

View reviewed changes

packages/react-native/React/Views/RCTViewUtils.h Outdated Show resolved Hide resolved

necolas mentioned this pull request Jun 1, 2023

[w3c] ☂️ Web Styles (Part 1) umbrella issue #34425

Open

74 tasks

resolve transform function

f39ba67

intergalacticspacehighway force-pushed the feat/transform-origin-1 branch from 6a43736 to f39ba67 Compare June 18, 2023 05:54

merge main

903776c

javache requested a review from sammy-SC June 19, 2023 09:54

javache reviewed Jun 19, 2023

View reviewed changes

intergalacticspacehighway added 3 commits June 28, 2023 03:46

fix: transform origin old arch ios

4dbcb66

Merge branch 'main' into feat/transform-origin-1

eda46d0

transform origin android

3be49ad

javache requested changes Jul 4, 2023

View reviewed changes

xxrlzzz mentioned this pull request Jul 18, 2023

feat: Use animation matrix For Skew prop #38494

Closed

github-actions bot added the Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. label Jul 19, 2023

intergalacticspacehighway mentioned this pull request Jul 19, 2023

feat(ios(old arch)): transform origin #38514

Closed

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 20, 2023

jacobp100 reviewed Jul 21, 2023

View reviewed changes

packages/react-native/Libraries/Components/View/ReactNativeStyleAttributes.js Show resolved Hide resolved

This was referenced Jul 23, 2023

feat: android transform origin #38558

Closed

feat: ios fabric transform origin #38559

Closed

javache closed this Aug 23, 2023

intergalacticspacehighway deleted the feat/transform-origin-1 branch August 9, 2024 22:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: transform-origin #37606

feat: transform-origin #37606

intergalacticspacehighway commented May 27, 2023 •

edited

Loading

analysis-bot commented May 27, 2023 •

edited

Loading

javache left a comment

javache commented May 31, 2023 •

edited

Loading

intergalacticspacehighway commented Jun 4, 2023 •

edited

Loading

javache commented Jun 5, 2023

intergalacticspacehighway commented Jun 9, 2023

javache commented Jun 13, 2023

intergalacticspacehighway commented Jun 18, 2023

javache Jun 19, 2023

javache Jun 19, 2023 •

edited

Loading

javache Jul 4, 2023

javache Jun 19, 2023

intergalacticspacehighway Jun 20, 2023 •

edited

Loading

intergalacticspacehighway commented Jun 28, 2023

javache left a comment

javache Jul 4, 2023

javache Jul 4, 2023

javache Jul 4, 2023

javache Jul 4, 2023

javache Jul 4, 2023

javache Jul 4, 2023

javache Jul 4, 2023

javache Jul 4, 2023

javache Jul 4, 2023

javache Jul 4, 2023

intergalacticspacehighway commented Jul 23, 2023 •

edited

Loading

javache commented Aug 21, 2023

javache commented Aug 23, 2023

		@@ -60,6 +60,10 @@ public static void processTransform(ReadableArray transforms, double[] result) {
		return;

		MatrixMathHelper.applyTranslate3D(helperMatrix, offsets[0], offsets[1], offsets[2]);
		MatrixMathHelper.multiplyInto(result, result, helperMatrix);

feat: transform-origin #37606

feat: transform-origin #37606

Conversation

intergalacticspacehighway commented May 27, 2023 • edited Loading

Checklist

Summary

Why?

Changelog:

Test Plan:

analysis-bot commented May 27, 2023 • edited Loading

javache left a comment

Choose a reason for hiding this comment

javache commented May 31, 2023 • edited Loading

intergalacticspacehighway commented Jun 4, 2023 • edited Loading

javache commented Jun 5, 2023

intergalacticspacehighway commented Jun 9, 2023

javache commented Jun 13, 2023

intergalacticspacehighway commented Jun 18, 2023

Choose a reason for hiding this comment

javache Jun 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

intergalacticspacehighway Jun 20, 2023 • edited Loading

Choose a reason for hiding this comment

intergalacticspacehighway commented Jun 28, 2023

javache left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

intergalacticspacehighway commented Jul 23, 2023 • edited Loading

javache commented Aug 21, 2023

javache commented Aug 23, 2023

intergalacticspacehighway commented May 27, 2023 •

edited

Loading

analysis-bot commented May 27, 2023 •

edited

Loading

javache commented May 31, 2023 •

edited

Loading

intergalacticspacehighway commented Jun 4, 2023 •

edited

Loading

javache Jun 19, 2023 •

edited

Loading

intergalacticspacehighway Jun 20, 2023 •

edited

Loading

intergalacticspacehighway commented Jul 23, 2023 •

edited

Loading