From 735e9b6a42cfbf0d3b0f109491d7b83ec0f5459f Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Mon, 6 May 2024 15:54:29 -0700 Subject: [PATCH] Fix duplicate rotation operations (#44429) Summary: `Transform::Rotation(x, y, z)` creates an empty transform with the associated operation, then [multiplies by xyz rotation vectors](https://en.wikipedia.org/wiki/Rotation_matrix#General_3D_rotations). Multiplication chains each transform operation, so afterward, we end up with correct transform matrix, but duplicate operations. This removes the first transform operation, and lets the per-axis multiplications set them. Changelog: [General][Fixed] - Fix duplicate rotation operations Differential Revision: D57025602 --- .../react/renderer/graphics/Transform.cpp | 2 -- .../renderer/graphics/tests/TransformTest.cpp | 20 +++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/graphics/Transform.cpp b/packages/react-native/ReactCommon/react/renderer/graphics/Transform.cpp index 3cd227e0292df3..3fd74379cdaae0 100644 --- a/packages/react-native/ReactCommon/react/renderer/graphics/Transform.cpp +++ b/packages/react-native/ReactCommon/react/renderer/graphics/Transform.cpp @@ -129,8 +129,6 @@ Transform Transform::RotateZ(Float radians) { Transform Transform::Rotate(Float x, Float y, Float z) { auto transform = Transform{}; - transform.operations.push_back( - TransformOperation{TransformOperationType::Rotate, x, y, z}); if (!isZero(x)) { transform = transform * Transform::RotateX(x); } 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 dcbb3052e68b10..3fcd9d259ed51e 100644 --- a/packages/react-native/ReactCommon/react/renderer/graphics/tests/TransformTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/graphics/tests/TransformTest.cpp @@ -77,6 +77,26 @@ TEST(TransformTest, rotatingRect) { ASSERT_NEAR(transformedRect.size.height, 14.1421, 0.0001); } +TEST(TransformTest, rotate3dOverload) { + auto point = facebook::react::Point{10, 10}; + auto size = facebook::react::Size{10, 10}; + auto rect = facebook::react::Rect{point, size}; + + auto transform = Transform::Rotate(0, 0, M_PI_4); + EXPECT_EQ(transform.operations.size(), 1); + EXPECT_EQ(transform.operations[0].type, TransformOperationType::Rotate); + EXPECT_EQ(transform.operations[0].x, 0); + EXPECT_EQ(transform.operations[0].y, 0); + ASSERT_NEAR(transform.operations[0].z, M_PI_4 - 0.0001, M_PI_4 + 0.0001); + + auto transformedRect = rect * Transform::Rotate(0, 0, M_PI_4); + + ASSERT_NEAR(transformedRect.origin.x, 7.9289, 0.0001); + ASSERT_NEAR(transformedRect.origin.y, 7.9289, 0.0001); + ASSERT_NEAR(transformedRect.size.width, 14.1421, 0.0001); + ASSERT_NEAR(transformedRect.size.height, 14.1421, 0.0001); +} + TEST(TransformTest, scalingAndTranslatingRect) { auto point = facebook::react::Point{100, 200}; auto size = facebook::react::Size{300, 400};