Skip to content

Commit

Permalink
Measure with transform bugfix (#44821)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #44821

Changelog: [Internal]

- Originally D37994809 was attempted to fix `Inverted FlatList` but was put behind Feature Toggle because it was causing problems in other scenarios.
- Later, D45866231 which was trying to fix scaling transform issue helped solve the issue attempted by the original diff.
- But after that points, Unit test around `computeRelativeLayoutMetrics` was having two variants where Feature Toggle for D37994809 was checked in with a wrong expected value.
- This diff revert D37994809 changes and clean up the unit test.

Reviewed By: NickGerleman

Differential Revision: D58197918

fbshipit-source-id: d8ae552018617e785e4010bc5805c53a875e02a3
  • Loading branch information
realsoelynn authored and facebook-github-bot committed Jun 11, 2024
1 parent 712ff8c commit 0d34569
Show file tree
Hide file tree
Showing 5 changed files with 157 additions and 186 deletions.
1 change: 0 additions & 1 deletion packages/react-native/ReactAndroid/api/ReactAndroid.api
Original file line number Diff line number Diff line change
Expand Up @@ -1953,7 +1953,6 @@ public class com/facebook/react/common/network/OkHttpCallUtil {
}

public class com/facebook/react/config/ReactFeatureFlags {
public static field calculateTransformedFramesEnabled Z
public static field dispatchPointerEvents Z
public static field enableBridgelessArchitecture Z
public static field enableCppPropsIteratorSetter Z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ public class ReactFeatureFlags {
/** Feature flag to configure eager attachment of the root view/initialisation of the JS code */
public static boolean enableEagerRootViewAttachment = false;

/** Enables or disables calculation of Transformed Frames */
public static boolean calculateTransformedFramesEnabled = false;

public static boolean dispatchPointerEvents = false;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,10 +396,6 @@ void Binding::installFabricUIManager(
// Keep reference to config object and cache some feature flags here
reactNativeConfig_ = config;

contextContainer->insert(
"CalculateTransformedFramesEnabled",
getFeatureFlagValue("calculateTransformedFramesEnabled"));

CoreFeatures::enablePropIteratorSetter =
getFeatureFlagValue("enableCppPropsIteratorSetter");
CoreFeatures::excludeYogaFromRawProps =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,57 +19,6 @@ namespace facebook::react {
template <class T>
using LayoutableSmallVector = std::vector<T>;

static LayoutableSmallVector<Rect> calculateTransformedFrames(
const LayoutableSmallVector<const ShadowNode*>& shadowNodeList,
LayoutableShadowNode::LayoutInspectingPolicy policy) {
auto size = shadowNodeList.size();
auto transformedFrames = LayoutableSmallVector<Rect>{size};
auto transformation = Transform::Identity();

for (auto i = size; i > 0; --i) {
auto currentShadowNode =
dynamic_cast<const LayoutableShadowNode*>(shadowNodeList.at(i - 1));
auto currentFrame = currentShadowNode->getLayoutMetrics().frame;

if (policy.includeTransform) {
if (Transform::isVerticalInversion(transformation)) {
auto parentShadowNode =
dynamic_cast<const LayoutableShadowNode*>(shadowNodeList.at(i));
currentFrame.origin.y =
parentShadowNode->getLayoutMetrics().frame.size.height -
currentFrame.size.height - currentFrame.origin.y;
}

if (Transform::isHorizontalInversion(transformation)) {
auto parentShadowNode =
dynamic_cast<const LayoutableShadowNode*>(shadowNodeList.at(i));
currentFrame.origin.x =
parentShadowNode->getLayoutMetrics().frame.size.width -
currentFrame.size.width - currentFrame.origin.x;
}

if (i != size) {
auto parentShadowNode =
dynamic_cast<const LayoutableShadowNode*>(shadowNodeList.at(i));
auto contentOriginOffset = parentShadowNode->getContentOriginOffset();
if (Transform::isVerticalInversion(transformation)) {
contentOriginOffset.y = -contentOriginOffset.y;
}
if (Transform::isHorizontalInversion(transformation)) {
contentOriginOffset.x = -contentOriginOffset.x;
}
currentFrame.origin += contentOriginOffset;
}

transformation = transformation * currentShadowNode->getTransform();
}

transformedFrames[i - 1] = currentFrame;
}

return transformedFrames;
}

LayoutableShadowNode::LayoutableShadowNode(
const ShadowNodeFragment& fragment,
const ShadowNodeFamily::Shared& family,
Expand Down Expand Up @@ -157,22 +106,6 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics(
return EmptyLayoutMetrics;
}

// ------------------------------
// TODO: T127619309 remove after validating that T127619309 is fixed
auto optionalCalculateTransformedFrames =
descendantNode->getContextContainer()
? descendantNode->getContextContainer()->find<bool>(
"CalculateTransformedFramesEnabled")
: std::optional<bool>(false);

bool shouldCalculateTransformedFrames =
optionalCalculateTransformedFrames.has_value()
? optionalCalculateTransformedFrames.value()
: false;

auto transformedFrames = shouldCalculateTransformedFrames
? calculateTransformedFrames(shadowNodeList, policy)
: LayoutableSmallVector<Rect>();
auto layoutMetrics = descendantLayoutableNode->getLayoutMetrics();
auto& resultFrame = layoutMetrics.frame;
resultFrame.origin = {0, 0};
Expand All @@ -194,9 +127,7 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics(
return EmptyLayoutMetrics;
}

auto currentFrame = shouldCalculateTransformedFrames
? transformedFrames[i]
: currentShadowNode->getLayoutMetrics().frame;
auto currentFrame = currentShadowNode->getLayoutMetrics().frame;
if (i == size - 1) {
// If it's the last element, its origin is irrelevant.
currentFrame.origin = {0, 0};
Expand All @@ -219,8 +150,7 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics(
resultFrame, currentFrame.getCenter());
}

if (!shouldCalculateTransformedFrames && i != 0 &&
policy.includeTransform) {
if (i != 0 && policy.includeTransform) {
resultFrame.origin += currentShadowNode->getContentOriginOffset();
}

Expand Down
Loading

0 comments on commit 0d34569

Please sign in to comment.