Skip to content

Commit

Permalink
Fix transform when calculate overflowInset
Browse files Browse the repository at this point in the history
Summary:
This diff fixes overflowInset calculation when a shadow node has transform matrix from a transfrom prop in JS. Specifically, this fixed the use case when transform directly used on a view component. When using Animated.View, it will create an invisible wrapper which will behave correctly with existing logic. This diff bring both use cases to work properly.

When a shadow node has transform on it, it will affect the overflowInset values for its parent nodes, but won't affect its own or any of its child nodes overflowInset values. This is obvious for translateX/Y case, but not for scale case. Take a look at the following case:

```
     ┌────────────────┐                 ┌────────────────┐                      ┌────────────────┐
     │Original Layout │                 │  Translate AB  │                      │    Scale AB    │
     └────────────────┘                 └────────────────┘                      └────────────────┘
                                                        ─────▶           ◀─────                  ─────▶
┌ ─ ─ ─ ┬──────────┐─ ─ ─ ─ ┐     ┌ ─ ─ ─ ┬──────────┐─ ─ ─ ─ ─ ┐      ┌ ─ ─ ─ ─ ─ ┬──────────┐─ ─ ─ ─ ─ ┐
        │ A        │                      │ A        │                             │ A        │
│       │          │        │     │       │          │          │      ├ ─ ─ ─ ─ ─ ┼ ─ ─┌─────┤─ ─ ─ ─ ─ ┤
 ─ ─ ─ ─│─ ─ ─┌───┐┼ ─ ─ ─ ─              │          │                   ◀─ ─ ─    │    │AB   │  ─ ─ ─▶
│       │     │AB ││        │     │ ┌ ─ ─ ┼ ─ ─ ─ ┬──┴┬ ─ ─ ─ ─ ┤      │           │    │     │          │
        └─────┤   ├┘                      └───────┤AB │                            └────┤     │
│             │┌──┴─────────┤     │ │             │   │         │      │ │              │ ┌───┴──────────┤
              ││ABC         │                     │┌──┴─────────┐                       │ │ABC           │
│             │└──┬─────────┤   │ │ │             ││ABC         │    │ │ │              │ │              │
┌───ABD───────┴─┐ │             │                 │└──┬─────────┘    │   ▼              │ └───┬──────────┘
├─────────────┬─┘ │         │   │ │ ├───ABD───────┴─┐ │         │    │ ├────────────────┴──┐  │          │
 ─ ─ ─ ─ ─ ─ ─└───┘─ ─ ─ ─ ─    ▼   └─────────────┬─┘ │              ▼ │      ABD          │  │
                                  └ ┴ ─ ─ ─ ─ ─ ─ ┴───┴ ─ ─ ─ ─ ┘      ├────────────────┬──┘  │          │
                                                                        ─ ─ ─ ─ ─ ─ ─ ─ ┴─────┴ ─ ─ ─ ─ ─
```

For the translate case, only view A has change on the overflowInset values for `right` and `bottom`. Note that the `left` and `top` are not changed as we union before and after transform is applied.

For the scale case, similar things are happening for view A, and both `left`, `right`, and `bottom` values are increased. However, for View AB or any of its children, they only *appear* to be increased, but that is purely cosmetic as it's caused by transform. The actual values are not changed, which will later be converted during render phase to actual pixels on screen.

In summary, overflowInset is affected from child nodes transform matrix to the current node (bottom up), but not from transform matrix on the current node to child nodes (top down). So the correct way to apply transform is to make it only affect calculating `contentFrame` during layout, which collects child nodes layout information and their transforms. The `contentFrame` is then used to decide the overflowInset values for the parent node. The current transform matrix on parent node is never used as it's not affecting overflowInset for the current node or its child nodes.

This diff reflects the context above with added unit test to cover the scale and translate transform matrix.

Changelog:
[Android/IOS][Fixed] - Fixed how we calculate overflowInset with transform matrix

Reviewed By: sammy-SC

Differential Revision: D34433404

fbshipit-source-id: 0e48e4af4cfd5a6dd32a30e7667686e8ef1a7004
  • Loading branch information
ryancat authored and Saadnajmi committed Jan 14, 2023
1 parent df6af1e commit 66f48af
Show file tree
Hide file tree
Showing 4 changed files with 232 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,6 @@ void YogaLayoutableShadowNode::layout(LayoutContext layoutContext) {
react_native_assert(!yogaNode_.isDirty());

auto contentFrame = Rect{};

for (auto childYogaNode : yogaNode_.getChildren()) {
auto &childNode =
*static_cast<YogaLayoutableShadowNode *>(childYogaNode->getContext());
Expand Down Expand Up @@ -521,24 +520,36 @@ void YogaLayoutableShadowNode::layout(LayoutContext layoutContext) {

auto layoutMetricsWithOverflowInset = childNode.getLayoutMetrics();
if (layoutMetricsWithOverflowInset.displayType != DisplayType::None) {
// The contentFrame should always union with existing child node layout +
// overflowInset. The transform may in a deferred animation and not
// applied yet.
contentFrame.unionInPlace(insetBy(
layoutMetricsWithOverflowInset.frame,
layoutMetricsWithOverflowInset.overflowInset));

auto childTransform = childNode.getTransform();
if (childTransform != Transform::Identity()) {
// The child node's transform matrix will affect the parent node's
// contentFrame. We need to union with child node's after transform
// layout here.
contentFrame.unionInPlace(insetBy(
layoutMetricsWithOverflowInset.frame * childTransform,
layoutMetricsWithOverflowInset.overflowInset * childTransform));
}
}
}

if (yogaNode_.getStyle().overflow() == YGOverflowVisible) {
auto transform = getTransform();
auto transformedContentFrame = contentFrame;
if (Transform::Identity() != transform) {
// When animation uses native driver, Yoga has no knowledge of the
// animation. In case the content goes out from current container, we need
// to union the content frame with its transformed frame.
transformedContentFrame = contentFrame * getTransform();
transformedContentFrame.unionInPlace(contentFrame);
}
// Note that the parent node's overflow layout is NOT affected by its
// transform matrix. That transform matrix is applied on the parent node as
// well as all of its child nodes, which won't cause changes on the
// overflowInset values. A special note on the scale transform -- the scaled
// layout may look like it's causing overflowInset changes, but it's purely
// cosmetic and will be handled by pixel density conversion logic later when
// render the view. The actual overflowInset value is not changed as if the
// transform is not happening here.
layoutMetrics_.overflowInset =
calculateOverflowInset(layoutMetrics_.frame, transformedContentFrame);
calculateOverflowInset(layoutMetrics_.frame, contentFrame);
} else {
layoutMetrics_.overflowInset = {};
}
Expand Down
215 changes: 196 additions & 19 deletions ReactCommon/react/renderer/components/view/tests/LayoutTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
namespace facebook {
namespace react {

// Note: the (x, y) origin is always relative to the parent node. You may use
// P482342650 to re-create this test case in playground.

// *******************************************************┌─ABCD:────┐****
// *******************************************************│ {70,-50} │****
// *******************************************************│ {30,60} │****
Expand All @@ -43,6 +46,8 @@ namespace react {
// ***********************│ │************************************
// ***********************└──────────┘************************************

enum TestCase { AS_IS, CLIPPING, TRANSFORM_SCALE, TRANSFORM_TRANSLATE };

class LayoutTest : public ::testing::Test {
protected:
ComponentBuilder builder_;
Expand All @@ -55,7 +60,7 @@ class LayoutTest : public ::testing::Test {

LayoutTest() : builder_(simpleComponentBuilder()) {}

void initialize(bool enforceClippingForABC) {
void initialize(TestCase testCase) {
// clang-format off
auto element =
Element<RootShadowNode>()
Expand All @@ -73,6 +78,7 @@ class LayoutTest : public ::testing::Test {
.children({
Element<ViewShadowNode>()
.reference(viewShadowNodeA_)
.tag(2)
.props([] {
auto sharedProps = std::make_shared<ViewShadowNodeProps>();
auto &props = *sharedProps;
Expand All @@ -85,7 +91,8 @@ class LayoutTest : public ::testing::Test {
.children({
Element<ViewShadowNode>()
.reference(viewShadowNodeAB_)
.props([] {
.tag(3)
.props([=] {
auto sharedProps = std::make_shared<ViewShadowNodeProps>();
auto &props = *sharedProps;
auto &yogaStyle = props.yogaStyle;
Expand All @@ -94,17 +101,26 @@ class LayoutTest : public ::testing::Test {
yogaStyle.position()[YGEdgeTop] = YGValue{10, YGUnitPoint};
yogaStyle.dimensions()[YGDimensionWidth] = YGValue{30, YGUnitPoint};
yogaStyle.dimensions()[YGDimensionHeight] = YGValue{90, YGUnitPoint};

if (testCase == TRANSFORM_SCALE) {
props.transform = props.transform * Transform::Scale(2, 2, 1);
}

if (testCase == TRANSFORM_TRANSLATE) {
props.transform = props.transform * Transform::Translate(10, 10, 0);
}
return sharedProps;
})
.children({
Element<ViewShadowNode>()
.reference(viewShadowNodeABC_)
.tag(4)
.props([=] {
auto sharedProps = std::make_shared<ViewShadowNodeProps>();
auto &props = *sharedProps;
auto &yogaStyle = props.yogaStyle;

if (enforceClippingForABC) {
if (testCase == CLIPPING) {
yogaStyle.overflow() = YGOverflowHidden;
}

Expand All @@ -118,6 +134,7 @@ class LayoutTest : public ::testing::Test {
.children({
Element<ViewShadowNode>()
.reference(viewShadowNodeABCD_)
.tag(5)
.props([] {
auto sharedProps = std::make_shared<ViewShadowNodeProps>();
auto &props = *sharedProps;
Expand All @@ -132,6 +149,7 @@ class LayoutTest : public ::testing::Test {
}),
Element<ViewShadowNode>()
.reference(viewShadowNodeABE_)
.tag(6)
.props([] {
auto sharedProps = std::make_shared<ViewShadowNodeProps>();
auto &props = *sharedProps;
Expand All @@ -154,32 +172,191 @@ class LayoutTest : public ::testing::Test {
}
};

// Test the layout as described above with no extra changes
TEST_F(LayoutTest, overflowInsetTest) {
initialize(false);
initialize(AS_IS);

auto layoutMetricsA = viewShadowNodeA_->getLayoutMetrics();

auto layoutMetrics = viewShadowNodeA_->getLayoutMetrics();
EXPECT_EQ(layoutMetricsA.frame.size.width, 50);
EXPECT_EQ(layoutMetricsA.frame.size.height, 50);

EXPECT_EQ(layoutMetrics.frame.size.width, 50);
EXPECT_EQ(layoutMetrics.frame.size.height, 50);
EXPECT_EQ(layoutMetricsA.overflowInset.left, -50);
EXPECT_EQ(layoutMetricsA.overflowInset.top, -30);
EXPECT_EQ(layoutMetricsA.overflowInset.right, -80);
EXPECT_EQ(layoutMetricsA.overflowInset.bottom, -50);

EXPECT_EQ(layoutMetrics.overflowInset.left, -50);
EXPECT_EQ(layoutMetrics.overflowInset.top, -30);
EXPECT_EQ(layoutMetrics.overflowInset.right, -80);
EXPECT_EQ(layoutMetrics.overflowInset.bottom, -50);
auto layoutMetricsABC = viewShadowNodeABC_->getLayoutMetrics();

EXPECT_EQ(layoutMetricsABC.frame.size.width, 110);
EXPECT_EQ(layoutMetricsABC.frame.size.height, 20);

EXPECT_EQ(layoutMetricsABC.overflowInset.left, 0);
EXPECT_EQ(layoutMetricsABC.overflowInset.top, -50);
EXPECT_EQ(layoutMetricsABC.overflowInset.right, 0);
EXPECT_EQ(layoutMetricsABC.overflowInset.bottom, 0);
}

// Test when box ABC has clipping (aka overflow hidden)
TEST_F(LayoutTest, overflowInsetWithClippingTest) {
initialize(true);
initialize(CLIPPING);

auto layoutMetricsA = viewShadowNodeA_->getLayoutMetrics();

EXPECT_EQ(layoutMetricsA.frame.size.width, 50);
EXPECT_EQ(layoutMetricsA.frame.size.height, 50);

EXPECT_EQ(layoutMetricsA.overflowInset.left, -50);
EXPECT_EQ(layoutMetricsA.overflowInset.top, 0);
EXPECT_EQ(layoutMetricsA.overflowInset.right, -80);
EXPECT_EQ(layoutMetricsA.overflowInset.bottom, -50);

auto layoutMetricsABC = viewShadowNodeABC_->getLayoutMetrics();

EXPECT_EQ(layoutMetricsABC.frame.size.width, 110);
EXPECT_EQ(layoutMetricsABC.frame.size.height, 20);

EXPECT_EQ(layoutMetricsABC.overflowInset.left, 0);
EXPECT_EQ(layoutMetricsABC.overflowInset.top, 0);
EXPECT_EQ(layoutMetricsABC.overflowInset.right, 0);
EXPECT_EQ(layoutMetricsABC.overflowInset.bottom, 0);
}

// Test when box AB translate (10, 10, 0) in transform. The parent node's
// overflowInset will be affected, but the transformed node and its child nodes
// are not affected. Here is an example:
//
// ┌────────────────┐ ┌────────────────┐
// │Original Layout │ │ Translate AB │
// └────────────────┘ └────────────────┘
// ─────▶
// ┌ ─ ─ ─ ┬──────────┐─ ─ ─ ─ ┐ ┌ ─ ─ ─ ┬──────────┐─ ─ ─ ─ ─ ┐
// │ A │ │ A │
// │ │ │ │ │ │ │ │
// ─ ─ ─ ─│─ ─ ─┌───┐┼ ─ ─ ─ ─ │ │
// │ │ │AB ││ │ │ ┌ ─ ─ ┼ ─ ─ ─ ┬──┴┬ ─ ─ ─ ─ ┤
// └─────┤ ├┘ └───────┤AB │
// │ │┌──┴─────────┤ │ │ │ │ │
// ││ABC │ │┌──┴─────────┐
// │ │└──┬─────────┤ │ │ │ ││ABC │
// ┌───ABD───────┴─┐ │ │ │└──┬─────────┘
// ├─────────────┬─┘ │ │ │ │ ├───ABD───────┴─┐ │ │
// ─ ─ ─ ─ ─ ─ ─└───┘─ ─ ─ ─ ─ ▼ └─────────────┬─┘ │
// └ ┴ ─ ─ ─ ─ ─ ─ ┴───┴ ─ ─ ─ ─ ┘

TEST_F(LayoutTest, overflowInsetTransformTranslateTest) {
initialize(TRANSFORM_TRANSLATE);

auto layoutMetricsA = viewShadowNodeA_->getLayoutMetrics();

EXPECT_EQ(layoutMetricsA.frame.size.width, 50);
EXPECT_EQ(layoutMetricsA.frame.size.height, 50);

// Change on parent node
// The top/left values are NOT changing as overflowInset is union of before
// and after transform layout. In this case, we move to the right and bottom,
// so the left and top is not changing, while right and bottom values are
// increased.
EXPECT_EQ(layoutMetricsA.overflowInset.left, -50);
EXPECT_EQ(layoutMetricsA.overflowInset.top, -30);
EXPECT_EQ(layoutMetricsA.overflowInset.right, -90);
EXPECT_EQ(layoutMetricsA.overflowInset.bottom, -60);

auto layoutMetricsAB = viewShadowNodeAB_->getLayoutMetrics();

EXPECT_EQ(layoutMetricsAB.frame.size.width, 30);
EXPECT_EQ(layoutMetricsAB.frame.size.height, 90);

// No change on self node with translate transform
EXPECT_EQ(layoutMetricsAB.overflowInset.left, -60);
EXPECT_EQ(layoutMetricsAB.overflowInset.top, -40);
EXPECT_EQ(layoutMetricsAB.overflowInset.right, -90);
EXPECT_EQ(layoutMetricsAB.overflowInset.bottom, 0);

auto layoutMetricsABC = viewShadowNodeABC_->getLayoutMetrics();

EXPECT_EQ(layoutMetricsABC.frame.size.width, 110);
EXPECT_EQ(layoutMetricsABC.frame.size.height, 20);

// No change on child node
EXPECT_EQ(layoutMetricsABC.overflowInset.left, 0);
EXPECT_EQ(layoutMetricsABC.overflowInset.top, -50);
EXPECT_EQ(layoutMetricsABC.overflowInset.right, 0);
EXPECT_EQ(layoutMetricsABC.overflowInset.bottom, 0);
}

// Test when box AB scaled 2X in transform. The parent node's overflowInset will
// be affected. However, the transformed node and its child nodes only appears
// to be affected (dashed arrow). Since all transform is cosmetic only, the
// actual values are NOT changed. It will be converted later when mapping the
// values to pixels during rendering. Here is an example:
//
// ┌────────────────┐ ┌────────────────┐
// │Original Layout │ │ Scale AB │
// └────────────────┘ └────────────────┘
// ─────▶
// ┌ ─ ─ ─ ┬──────────┐─ ─ ─ ─ ┐ ┌ ─ ─ ─ ─ ─ ┬──────────┐─ ─ ─ ─ ─ ┐
// │ A │ │ A │
// │ │ │ │ ├ ─ ─ ─ ─ ─ ┼ ─ ─┌─────┤─ ─ ─ ─ ─ ┤
// ─ ─ ─ ─│─ ─ ─┌───┐┼ ─ ─ ─ ─ │ │AB │ ─ ─ ─▶
// │ │ │AB ││ │ │ │ │ │ │
// └─────┤ ├┘ └────┤ │
// │ │┌──┴─────────┤ │ │ ┌───┴──────────┤
// ││ABC │ │ │ABC │
// │ │└──┬─────────┤ │ │ │ │ │
// ┌───ABD───────┴─┐ │ │ │ └───┬──────────┘
// ├─────────────┬─┘ │ │ │ ├────────────────┴──┐ │ │
// ─ ─ ─ ─ ─ ─ ─└───┘─ ─ ─ ─ ─ ▼ │ ABD │ │
// ├────────────────┬──┘ │ │
// ─ ─ ─ ─ ─ ─ ─ ─ ┴─────┴ ─ ─ ─ ─ ─

TEST_F(LayoutTest, overflowInsetTransformScaleTest) {
initialize(TRANSFORM_SCALE);

auto layoutMetricsA = viewShadowNodeA_->getLayoutMetrics();

EXPECT_EQ(layoutMetricsA.frame.size.width, 50);
EXPECT_EQ(layoutMetricsA.frame.size.height, 50);

// Change on parent node when a child view scale up
// Note that AB scale up from its center point. The numbers are calculated
// assuming AB's center point is not moving.
EXPECT_EQ(layoutMetricsA.overflowInset.left, -125);
EXPECT_EQ(layoutMetricsA.overflowInset.top, -115);
EXPECT_EQ(layoutMetricsA.overflowInset.right, -185);
EXPECT_EQ(layoutMetricsA.overflowInset.bottom, -95);

auto layoutMetricsAB = viewShadowNodeAB_->getLayoutMetrics();

// The frame of box AB won't actually scale up. The transform matrix is
// purely cosmetic and should apply later in mounting phase.
EXPECT_EQ(layoutMetricsAB.frame.size.width, 30);
EXPECT_EQ(layoutMetricsAB.frame.size.height, 90);

// No change on self node with scale transform. This may sound a bit
// surprising, but the overflowInset values will be scaled up via pixel
// density ratio along with width/height of the view. When we do hit-testing,
// the overflowInset value will appears to be doubled as expected.
EXPECT_EQ(layoutMetricsAB.overflowInset.left, -60);
EXPECT_EQ(layoutMetricsAB.overflowInset.top, -40);
EXPECT_EQ(layoutMetricsAB.overflowInset.right, -90);
EXPECT_EQ(layoutMetricsAB.overflowInset.bottom, 0);

auto layoutMetrics = viewShadowNodeA_->getLayoutMetrics();
auto layoutMetricsABC = viewShadowNodeABC_->getLayoutMetrics();

EXPECT_EQ(layoutMetrics.frame.size.width, 50);
EXPECT_EQ(layoutMetrics.frame.size.height, 50);
// The frame of box ABC won't actually scale up. The transform matrix is
// purely cosmatic and should apply later in mounting phase.
EXPECT_EQ(layoutMetricsABC.frame.size.width, 110);
EXPECT_EQ(layoutMetricsABC.frame.size.height, 20);

EXPECT_EQ(layoutMetrics.overflowInset.left, -50);
EXPECT_EQ(layoutMetrics.overflowInset.top, 0);
EXPECT_EQ(layoutMetrics.overflowInset.right, -80);
EXPECT_EQ(layoutMetrics.overflowInset.bottom, -50);
// The overflowInset of ABC won't change either. This may sound a bit
// surprising, but the overflowInset values will be scaled up via pixel
// density ratio along with width/height of the view. When we do hit-testing,
// the overflowInset value will appears to be doubled as expected.
EXPECT_EQ(layoutMetricsABC.overflowInset.left, 0);
EXPECT_EQ(layoutMetricsABC.overflowInset.top, -50);
EXPECT_EQ(layoutMetricsABC.overflowInset.right, 0);
EXPECT_EQ(layoutMetricsABC.overflowInset.bottom, 0);
}

} // namespace react
Expand Down
8 changes: 8 additions & 0 deletions ReactCommon/react/renderer/graphics/Transform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,14 @@ Rect operator*(Rect const &rect, Transform const &transform) {
transformedA, transformedB, transformedC, transformedD);
}

EdgeInsets operator*(EdgeInsets const &edgeInsets, Transform const &transform) {
return EdgeInsets{
edgeInsets.left * transform.matrix[0],
edgeInsets.top * transform.matrix[5],
edgeInsets.right * transform.matrix[0],
edgeInsets.bottom * transform.matrix[5]};
}

Vector operator*(Transform const &transform, Vector const &vector) {
return {
vector.x * transform.at(0, 0) + vector.y * transform.at(1, 0) +
Expand Down
6 changes: 6 additions & 0 deletions ReactCommon/react/renderer/graphics/Transform.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,12 @@ Size operator*(Size const &size, Transform const &transform);
*/
Rect operator*(Rect const &rect, Transform const &transform);

/*
* Applies tranformation to the given EdgeInsets.
* ONLY SUPPORTS scale transformation.
*/
EdgeInsets operator*(EdgeInsets const &edgeInsets, Transform const &transform);

Vector operator*(Transform const &transform, Vector const &vector);

} // namespace react
Expand Down

0 comments on commit 66f48af

Please sign in to comment.