Skip to content

Commit

Permalink
Fix LayoutableShadowNode::computeRelativeLayoutMetrics when using sca…
Browse files Browse the repository at this point in the history
…le on a parent node (#37436)

Summary:
Pull Request resolved: #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
  • Loading branch information
rubennorte authored and facebook-github-bot committed May 26, 2023
1 parent 912bca0 commit 64416d9
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ static LayoutableSmallVector<Rect> calculateTransformedFrames(
if (i != size) {
auto parentShadowNode =
traitCast<LayoutableShadowNode const *>(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();
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 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;

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

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};
auto center = rect.getCenter();
return transform.applyWithCenter(rect, center);
}

Rect Transform::applyWithCenter(Rect const &rect, Point const &center) 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 = *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 + 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ struct Transform {
*/
Transform operator*(Transform const &rhs) const;

Rect applyWithCenter(Rect const &rect, Point const &center) const;

/**
* Convert to folly::dynamic.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 64416d9

Please sign in to comment.