feat: ios fabric transform origin by intergalacticspacehighway · Pull Request #38559 · react/react-native

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

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!

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

 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

+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

+ 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
TransformOrigintransformOrigin;
for (size_ti=0; i<2; i++) {
const auto&origin=origins[i];
if (origin.hasType<Float>()) {
transformOrigin.xy[i] =yogaStyleValueFromFloat((Float)origin);
} elseif (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];
}
TransformOrigintransformOrigin;
for (size_ti=0; i<std::min(origins.size(), 2); i++) {
const auto&origin=origins[i];
if (origin.hasType<Float>()) {
transformOrigin.xy[i] =yogaStyleValueFromFloat((Float)origin);
} elseif (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

 };

+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

+
+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?

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-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

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

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

Comment threadpackages/react-native/ReactCommon/react/renderer/components/view/ViewProps.cpp Outdated

jacobp100 reviewed Jul 26, 2023
View reviewed changes

Comment threadpackages/react-native/ReactCommon/react/renderer/components/view/conversions.h Outdated

jacobp100 reviewed Jul 26, 2023
View reviewed changes

Comment threadpackages/react-native/ReactCommon/react/renderer/graphics/Transform.h Outdated

jacobp100 reviewed Jul 26, 2023
View reviewed changes

Comment threadpackages/react-native/ReactCommon/react/renderer/components/view/conversions.h

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 originJul 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

Comment threadpackages/react-native/ReactCommon/react/renderer/components/view/BaseViewProps.cpp Outdated
Comment threadpackages/react-native/Libraries/StyleSheet/private/_TransformStyle.js Outdated
Comment threadpackages/react-native/ReactCommon/react/renderer/components/view/conversions.h Outdated

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

intergalacticspacehighwayand 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 react/react-native-website#3842
Merged

facebook-github-bot closed this in 177ef21Sep 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

-TransformOrigintransformOrigin;
-for (size_ti=0; i<2; i++) {
-const auto&origin=origins[i];
-if (origin.hasType<Float>()) {
-transformOrigin.xy[i] =yogaStyleValueFromFloat((Float)origin);
- } elseif (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];
- }
+TransformOrigintransformOrigin;
+for (size_ti=0; i<std::min(origins.size(), 2); i++) {
+const auto&origin=origins[i];
+if (origin.hasType<Float>()) {
+transformOrigin.xy[i] =yogaStyleValueFromFloat((Float)origin);
+ } elseif (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];
+ }

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-actionsBot 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-actionsBot commented Jul 23, 2023•
edited
Loading

analysis-bot commented Jul 23, 2023•
edited
Loading