From 64416d9503f9c17ab5ceec2a1506a97a2049f879 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Fri, 26 May 2023 09:06:00 -0700 Subject: [PATCH] Fix LayoutableShadowNode::computeRelativeLayoutMetrics when using scale on a parent node (#37436) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/37436 When testing the reported intersections from `IntersectionObserver` I realized that the reported rectangles were incorrect when using `scale` in a parent. This is because `scale` transforms use the center of the node as the origin, and when applying the transform on its children we were using the center of the children instead of the center of the same node. This fixes that issue by using the center of the parent when applying the transform on the children. Changelog: [General][Fixed] - Fixed computation of layout via `ref.measureRelative` and `ref.measureInWindow` for nodes with scale/rotate transforms in their parents. Reviewed By: sammy-SC Differential Revision: D45866231 fbshipit-source-id: 68928cbaca28c21155657d276cf8ddd2129d9668 --- .../react/renderer/core/LayoutMetrics.h | 14 ++++++--- .../renderer/core/LayoutableShadowNode.cpp | 19 +++++++----- .../core/tests/LayoutableShadowNodeTest.cpp | 4 +-- .../react/renderer/graphics/Transform.cpp | 29 ++++++++++--------- .../react/renderer/graphics/Transform.h | 2 ++ .../renderer/graphics/tests/TransformTest.cpp | 16 ++++++++++ 6 files changed, 58 insertions(+), 26 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/core/LayoutMetrics.h b/packages/react-native/ReactCommon/react/renderer/core/LayoutMetrics.h index f28ba3322fb..16faa8bc78c 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/LayoutMetrics.h +++ b/packages/react-native/ReactCommon/react/renderer/core/LayoutMetrics.h @@ -20,17 +20,23 @@ namespace facebook::react { * Describes results of layout process for particular shadow node. */ struct LayoutMetrics { - // Origin: relative to its parent content frame (unless using a method that - // computes it relative to other parent or the viewport) + // Origin: relative to the outer border of its parent. // Size: includes border, padding and content. Rect frame; - // Width of the border + padding in all directions. + // Width of the border + padding in each direction. EdgeInsets contentInsets{0}; - // Width of the border in all directions. + // Width of the border in each direction. EdgeInsets borderWidth{0}; + // See `DisplayType` for all possible options. DisplayType displayType{DisplayType::Flex}; + // See `LayoutDirection` for all possible options. LayoutDirection layoutDirection{LayoutDirection::Undefined}; + // Pixel density. Number of device pixels per density-independent pixel. Float pointScaleFactor{1.0}; + // How much the children of the node actually overflow in each direction. + // Positive values indicate that children are overflowing outside of the node. + // Negative values indicate that children are clipped inside the node + // (like when using `overflow: clip` on Web). EdgeInsets overflowInset{}; // Origin: the outer border of the node. diff --git a/packages/react-native/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp index 18374f9a286..9ab11bb4174 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp @@ -52,14 +52,14 @@ static LayoutableSmallVector calculateTransformedFrames( if (i != size) { auto parentShadowNode = traitCast(shadowNodeList.at(i)); - auto contentOritinOffset = parentShadowNode->getContentOriginOffset(); + auto contentOriginOffset = parentShadowNode->getContentOriginOffset(); if (Transform::isVerticalInversion(transformation)) { - contentOritinOffset.y = -contentOritinOffset.y; + contentOriginOffset.y = -contentOriginOffset.y; } if (Transform::isHorizontalInversion(transformation)) { - contentOritinOffset.x = -contentOritinOffset.x; + contentOriginOffset.x = -contentOriginOffset.x; } - currentFrame.origin += contentOritinOffset; + currentFrame.origin += contentOriginOffset; } transformation = transformation * currentShadowNode->getTransform(); @@ -208,12 +208,17 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics( auto shouldApplyTransformation = (policy.includeTransform && !isRootNode) || (policy.includeViewportOffset && isRootNode); + // Move frame to the coordinate space of the current node. + resultFrame.origin += currentFrame.origin; + if (shouldApplyTransformation) { - resultFrame.size = resultFrame.size * currentShadowNode->getTransform(); - currentFrame = currentFrame * currentShadowNode->getTransform(); + // If a node has a transform, we need to use the center of that node as + // the origin of the transform when transforming its children (which + // affects the result of transforms like `scale` and `rotate`). + resultFrame = currentShadowNode->getTransform().applyWithCenter( + resultFrame, currentFrame.getCenter()); } - resultFrame.origin += currentFrame.origin; if (!shouldCalculateTransformedFrames && i != 0 && policy.includeTransform) { resultFrame.origin += currentShadowNode->getContentOriginOffset(); diff --git a/packages/react-native/ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp b/packages/react-native/ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp index 9be2fae835c..46ae7c61b1e 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp @@ -377,8 +377,8 @@ TEST(LayoutableShadowNodeTest, relativeLayoutMetricsOnTransformedParent) { LayoutableShadowNode::computeRelativeLayoutMetrics( childShadowNode->getFamily(), *parentShadowNode, {}); - EXPECT_EQ(relativeLayoutMetrics.frame.origin.x, 45); - EXPECT_EQ(relativeLayoutMetrics.frame.origin.y, 45); + EXPECT_EQ(relativeLayoutMetrics.frame.origin.x, 40); + EXPECT_EQ(relativeLayoutMetrics.frame.origin.y, 40); EXPECT_EQ(relativeLayoutMetrics.frame.size.width, 25); EXPECT_EQ(relativeLayoutMetrics.frame.size.height, 25); diff --git a/packages/react-native/ReactCommon/react/renderer/graphics/Transform.cpp b/packages/react-native/ReactCommon/react/renderer/graphics/Transform.cpp index d9669ea3497..56fb6431a0a 100644 --- a/packages/react-native/ReactCommon/react/renderer/graphics/Transform.cpp +++ b/packages/react-native/ReactCommon/react/renderer/graphics/Transform.cpp @@ -366,22 +366,25 @@ Point operator*(Point const &point, Transform const &transform) { } Rect operator*(Rect const &rect, Transform const &transform) { - auto centre = rect.getCenter(); + auto center = rect.getCenter(); + return transform.applyWithCenter(rect, center); +} - auto a = Point{rect.origin.x, rect.origin.y} - centre; - auto b = Point{rect.getMaxX(), rect.origin.y} - centre; - auto c = Point{rect.getMaxX(), rect.getMaxY()} - centre; - auto d = Point{rect.origin.x, rect.getMaxY()} - centre; +Rect Transform::applyWithCenter(Rect const &rect, Point const ¢er) const { + auto a = Point{rect.origin.x, rect.origin.y} - center; + auto b = Point{rect.getMaxX(), rect.origin.y} - center; + auto c = Point{rect.getMaxX(), rect.getMaxY()} - center; + auto d = Point{rect.origin.x, rect.getMaxY()} - center; - auto vectorA = transform * Vector{a.x, a.y, 0, 1}; - auto vectorB = transform * Vector{b.x, b.y, 0, 1}; - auto vectorC = transform * Vector{c.x, c.y, 0, 1}; - auto vectorD = transform * Vector{d.x, d.y, 0, 1}; + auto vectorA = *this * Vector{a.x, a.y, 0, 1}; + auto vectorB = *this * Vector{b.x, b.y, 0, 1}; + auto vectorC = *this * Vector{c.x, c.y, 0, 1}; + auto vectorD = *this * Vector{d.x, d.y, 0, 1}; - Point transformedA{vectorA.x + centre.x, vectorA.y + centre.y}; - Point transformedB{vectorB.x + centre.x, vectorB.y + centre.y}; - Point transformedC{vectorC.x + centre.x, vectorC.y + centre.y}; - Point transformedD{vectorD.x + centre.x, vectorD.y + centre.y}; + Point transformedA{vectorA.x + center.x, vectorA.y + center.y}; + Point transformedB{vectorB.x + center.x, vectorB.y + center.y}; + Point transformedC{vectorC.x + center.x, vectorC.y + center.y}; + Point transformedD{vectorD.x + center.x, vectorD.y + center.y}; return Rect::boundingRect( transformedA, transformedB, transformedC, transformedD); diff --git a/packages/react-native/ReactCommon/react/renderer/graphics/Transform.h b/packages/react-native/ReactCommon/react/renderer/graphics/Transform.h index c8c849c6677..e0f29508f65 100644 --- a/packages/react-native/ReactCommon/react/renderer/graphics/Transform.h +++ b/packages/react-native/ReactCommon/react/renderer/graphics/Transform.h @@ -155,6 +155,8 @@ struct Transform { */ Transform operator*(Transform const &rhs) const; + Rect applyWithCenter(Rect const &rect, Point const ¢er) const; + /** * Convert to folly::dynamic. */ diff --git a/packages/react-native/ReactCommon/react/renderer/graphics/tests/TransformTest.cpp b/packages/react-native/ReactCommon/react/renderer/graphics/tests/TransformTest.cpp index 70c232ecd01..dcbb3052e68 100644 --- a/packages/react-native/ReactCommon/react/renderer/graphics/tests/TransformTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/graphics/tests/TransformTest.cpp @@ -41,6 +41,22 @@ TEST(TransformTest, scalingRect) { EXPECT_EQ(transformedRect.size.height, 200); } +TEST(TransformTest, scalingRectWithDifferentCenter) { + auto point = facebook::react::Point{100, 200}; + auto size = facebook::react::Size{300, 400}; + auto rect = facebook::react::Rect{point, size}; + + auto center = facebook::react::Point{0, 0}; + + auto transformedRect = + Transform::Scale(0.5, 0.5, 1).applyWithCenter(rect, center); + + EXPECT_EQ(transformedRect.origin.x, 50); + EXPECT_EQ(transformedRect.origin.y, 100); + EXPECT_EQ(transformedRect.size.width, 150); + EXPECT_EQ(transformedRect.size.height, 200); +} + TEST(TransformTest, invertingSize) { auto size = facebook::react::Size{300, 400}; auto transformedSize = size * Transform::VerticalInversion();