feat: ios fabric transform origin #38559

intergalacticspacehighway · 2023-07-23T11:56:56Z

Summary:

This PR adds transform-origin support for iOS fabric. This PR also incorporates feedback/changes suggested by @javache in the original PR.

Changelog:

[IOS] [ADDED] - Fabric Transform origin

Test Plan:

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

github-actions · 2023-07-23T11:57:56Z

	Warnings
⚠️	packages/react-native/Libraries/Components/View/ReactNativeStyleAttributes.js#L11 - packages/react-native/Libraries/Components/View/ReactNativeStyleAttributes.js line 11 – Requires should be sorted alphabetically (lint/sort-imports)

Generated by 🚫 dangerJS against a30ed3d

analysis-bot · 2023-07-23T12:21:00Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,962,403	+2,327
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	9,556,421	+2,331
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: a0a544f
Branch: main

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

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

packages/react-native/ReactCommon/react/renderer/components/view/conversions.h

packages/react-native/ReactCommon/react/renderer/graphics/Transform.h

packages/react-native/ReactCommon/react/renderer/components/view/conversions.h

intergalacticspacehighway · 2023-07-27T07:11:06Z

Getting this error on test_android CI 🤔

C/C++: /root/react-native/packages/react-native/ReactCommon/react/renderer/graphics/Transform.h:19:10: fatal error: 'yoga/YGValue.h' file not found

facebook-github-bot · 2023-08-21T18:31:19Z

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

javache

Thanks for splitting these up into multiple PR's!

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

packages/react-native/Libraries/StyleSheet/private/_TransformStyle.js

packages/react-native/ReactCommon/react/renderer/components/view/conversions.h

javache · 2023-08-24T12:00:54Z

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

  opacity?: AnimatableNumericValue,
  elevation?: number,
  pointerEvents?: 'auto' | 'none' | 'box-none' | 'box-only',
+  transformOrigin?: string,


Why is this required? transformOrigin should be included via ____TransformStyle_Internal already.

My bad. Updated!

javache · 2023-08-24T12:01:37Z

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

+namespace {
+
+std::array<float, 3> getTranslateForTransformOrigin(float viewWidth, float viewHeight, facebook::react::TransformOrigin transformOrigin)  {
+  float viewCenterX = viewWidth / 2;
+  float viewCenterY = viewHeight / 2;
+
+  std::array<float, 3> origin = {viewCenterX, viewCenterY, transformOrigin.z};
+
+  for (size_t i = 0; i < 2; ++i) {
+    auto& currentOrigin = transformOrigin.xy[i];
+    if (currentOrigin.unit == YGUnitPoint) {
+      origin[i] = currentOrigin.value;
+    } else if (currentOrigin.unit == YGUnitPercent) {
+        origin[i] = ((i == 0) ? viewWidth : viewHeight) * currentOrigin.value / 100.0f;
+      }
+    }
+
+
+  float newTranslateX = -viewCenterX + origin[0];
+  float newTranslateY = -viewCenterY + origin[1];
+  float newTranslateZ = origin[2];
+
+  return std::array{newTranslateX, newTranslateY, newTranslateZ};
+}
+
+}


nit: move inside the namespace facebook::react below (and remove the extra facebook::react prefixes)

javache · 2023-08-24T12:03:18Z

packages/react-native/ReactCommon/react/renderer/components/view/conversions.h

+  TransformOrigin transformOrigin;
+
+   for (size_t i = 0; i < 2; i++) {
+     const auto& origin = origins[i];
+     if (origin.hasType<Float>()) {
+       transformOrigin.xy[i] = yogaStyleValueFromFloat((Float)origin);
+     } else if (origin.hasType<std::string>()) {
+       const auto stringValue = (std::string)origin;
+
+       if (stringValue.back() == '%') {
+         auto tryValue = folly::tryTo<float>(
+                                             std::string_view(stringValue).substr(0, stringValue.length() - 1));
+         if (tryValue.hasValue()) {
+           transformOrigin.xy[i] = YGValue{tryValue.value(), YGUnitPercent};
+         }
+       }
+     }
+   }
+
+    if (origins[2].hasType<Float>()) {
+      transformOrigin.z = (Float)origins[2];
+    }


You do need to check now that the indices you're accessing are not exceeding the vector bounds.

Suggested change

TransformOrigin transformOrigin;

for (size_t i = 0; i < 2; i++) {

const auto& origin = origins[i];

if (origin.hasType<Float>()) {

transformOrigin.xy[i] = yogaStyleValueFromFloat((Float)origin);

} else if (origin.hasType<std::string>()) {

const auto stringValue = (std::string)origin;

if (stringValue.back() == '%') {

auto tryValue = folly::tryTo<float>(

std::string_view(stringValue).substr(0, stringValue.length() - 1));

if (tryValue.hasValue()) {

transformOrigin.xy[i] = YGValue{tryValue.value(), YGUnitPercent};

}

}

}

}

if (origins[2].hasType<Float>()) {

transformOrigin.z = (Float)origins[2];

}

TransformOrigin transformOrigin;

for (size_t i = 0; i < std::min(origins.size(), 2); i++) {

const auto& origin = origins[i];

if (origin.hasType<Float>()) {

transformOrigin.xy[i] = yogaStyleValueFromFloat((Float)origin);

} else if (origin.hasType<std::string>()) {

const auto stringValue = (std::string)origin;

if (stringValue.back() == '%') {

auto tryValue = folly::tryTo<float>(

std::string_view(stringValue).substr(0, stringValue.length() - 1));

if (tryValue.hasValue()) {

transformOrigin.xy[i] = YGValue{tryValue.value(), YGUnitPercent};

}

}

}

}

if (origin.size() >= 3 && origins[2].hasType<Float>()) {

transformOrigin.z = (Float)origins[2];

}

javache · 2023-08-24T12:04:27Z

packages/react-native/ReactCommon/react/renderer/graphics/Transform.h

 };

+struct TransformOrigin {
+  std::array<YGValue, 2> xy;


Per @NickGerleman feedback on another PR, using YGValue is an undesired dependency. Let's introduce a custom struct for this in another PR.

Added a PR for struct - #39150. I have tested it replacing YGValue and it works, kept it simple. let me know!

std::array<YGValue, 2> xy; -> std::array<ValueUnit, 2> xy;

cc - @jacobp100 we can also use it here - #38626

Will try and get to it later this weekend

Co-authored-by: Nick Gerleman <nick@nickgerleman.com>

…ormorigin

jacobp100 · 2023-08-25T11:36:57Z

packages/react-native/ReactCommon/react/renderer/graphics/ValueUnit.h

+
+namespace facebook::react {
+
+enum class UnitType {


Why do we need undefined?

To represent initial/undefined value? Also useful to check the value is set. Check isSet in TransformOrigin struct.

facebook-github-bot · 2023-08-29T10:06:39Z

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

javache · 2023-09-01T13:33:39Z

This is now ready to ship internally so should be pushed soon. Please also submit a PR to https://github.com/facebook/react-native-website to add documentation for this.

jacobp100 · 2023-09-01T14:54:13Z

@javache Sorry - I never found time to do the iOS old arch - do you still need help with that?

facebook-github-bot · 2023-09-01T20:08:31Z

@javache merged this pull request in 177ef21.

javache · 2023-09-05T10:51:01Z

@javache Sorry - I never found time to do the iOS old arch - do you still need help with that?

Doesn't 5f40f08 handle the old arch? What are we missing still?

jacobp100 · 2023-09-05T10:53:43Z

@javache Sorry - I never found time to do the iOS old arch - do you still need help with that?

Doesn't 5f40f08 handle the old arch? What are we missing still?

I think the plan was to remove the dependency on Yoga in this - 5f40f08#diff-6654f56131a3cc2c701d1ca11fbdd2c4c41854136153eaceba4a103413e81809R11-R15

So make our own struct that stores the float value, and whether it was a percent or pixel value

javache · 2023-09-05T13:56:34Z

I think the plan was to remove the dependency on Yoga in this - 5f40f08#diff-6654f56131a3cc2c701d1ca11fbdd2c4c41854136153eaceba4a103413e81809R11-R15

So make our own struct that stores the float value, and whether it was a percent or pixel value

I think we're ok as-is, as we've been able to cleanup the dependency in the new arch version.

feat: transform origin ios fabric

4b13fb1

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

intergalacticspacehighway mentioned this pull request Jul 23, 2023

feat: transform-origin #37606

Closed

4 tasks

jacobp100 reviewed Jul 25, 2023

View reviewed changes

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

intergalacticspacehighway added 2 commits July 25, 2023 23:51

move transform in invalidateLayer

26b8430

fix: lint errors

e65fbf3

jacobp100 mentioned this pull request Jul 25, 2023

Implement transform-origin for old arch iOS #38626

Closed

intergalacticspacehighway added 2 commits July 26, 2023 01:34

revert needs invalidate layer

fbaf6c0

transform origin struct

35badaf

jacobp100 reviewed Jul 26, 2023

View reviewed changes

packages/react-native/ReactCommon/react/renderer/components/view/ViewProps.cpp Outdated Show resolved Hide resolved

jacobp100 reviewed Jul 26, 2023

View reviewed changes

packages/react-native/ReactCommon/react/renderer/components/view/conversions.h Outdated Show resolved Hide resolved

jacobp100 reviewed Jul 26, 2023

View reviewed changes

packages/react-native/ReactCommon/react/renderer/graphics/Transform.h Outdated Show resolved Hide resolved

jacobp100 reviewed Jul 26, 2023

View reviewed changes

packages/react-native/ReactCommon/react/renderer/components/view/conversions.h Show resolved Hide resolved

intergalacticspacehighway added 2 commits July 26, 2023 17:27

nit: pr suggestions

302a9aa

revert non fabric changes

be6b4d5

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

intergalacticspacehighway added 2 commits July 27, 2023 11:26

fix: tests

ffa6afc

nit: header fix

ab174da

intergalacticspacehighway added 2 commits August 7, 2023 11:25

merge main

a9632d6

merge main

eefec92

javache approved these changes Aug 23, 2023

View reviewed changes

intergalacticspacehighway added 2 commits August 23, 2023 23:01

fix: PR feedback

40ed8db

fix: PR feedback

8902301

javache mentioned this pull request Aug 24, 2023

feat: android transform origin #38558

Closed

javache reviewed Aug 24, 2023

View reviewed changes

intergalacticspacehighway added 5 commits August 24, 2023 17:51

PR feedback

0e91714

remove redundant type

a74fdf8

fix min error

854710a

fix min error

9a3c230

add value unit struct

55ad2c9

intergalacticspacehighway mentioned this pull request Aug 24, 2023

feat: add ValueUnit struct #39150

Closed

intergalacticspacehighway and others added 3 commits August 25, 2023 03:24

Apply suggestions from code review

696621e

Co-authored-by: Nick Gerleman <nick@nickgerleman.com>

Merge branch 'feat/add-value-unit-struct' into feat/ios-fabric-transf…

0f27a0c

…ormorigin

use ValueUnit struct

a30ed3d

jacobp100 reviewed Aug 25, 2023

View reviewed changes

intergalacticspacehighway mentioned this pull request Sep 1, 2023

docs: transform origin facebook/react-native-website#3842

Merged

facebook-github-bot closed this in 177ef21 Sep 1, 2023

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

intergalacticspacehighway deleted the feat/ios-fabric-transformorigin branch August 9, 2024 22:42

Saadnajmi mentioned this pull request Nov 21, 2024

refactor(transforms): simplify macOS transform logic on old and new architecture microsoft/react-native-macos#2289

Merged

feat: ios fabric transform origin #38559

feat: ios fabric transform origin #38559

Uh oh!

Conversation

intergalacticspacehighway commented Jul 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Changelog:

Test Plan:

Uh oh!

github-actions bot commented Jul 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

analysis-bot commented Jul 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

intergalacticspacehighway commented Jul 27, 2023

Uh oh!

facebook-github-bot commented Aug 21, 2023

Uh oh!

javache left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 29, 2023

Uh oh!

javache commented Sep 1, 2023

Uh oh!

jacobp100 commented Sep 1, 2023

Uh oh!

facebook-github-bot commented Sep 1, 2023

Uh oh!

javache commented Sep 5, 2023

Uh oh!

jacobp100 commented Sep 5, 2023

Uh oh!

javache commented Sep 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

intergalacticspacehighway commented Jul 23, 2023 •

edited

Loading

github-actions bot commented Jul 23, 2023 •

edited

Loading

analysis-bot commented Jul 23, 2023 •

edited

Loading