From 7cb2b9ba552d304eb97f1eb425c36f765a240fbf Mon Sep 17 00:00:00 2001 From: Ruslan Shestopalyuk Date: Mon, 30 Dec 2024 02:51:00 -0800 Subject: [PATCH 1/2] Avoid calling fmod twice in roundLayoutResultsToPixelGrid (#1775) Summary: X-link: https://github.com/facebook/litho/pull/1036 X-link: https://github.com/facebook/react-native/pull/48404 ## Changelog: [Internal] - This popped up when profiling some heavy UI performance, calling `fmod` operation in Yoga's `roundLayoutResultsToPixelGrid` in `PixelGrid.cpp` can be expensive, furthermore it turns out that some of the calls were redundant. This replaces the duplicate calls to fmod with an equivalent single round operation, which for e.g. clang compiler on Windows brings the code in question from ~50 instructions (including 4 call instructions to the fmod function) down to ~30 instructions (without any external calls), and the layout operation being **~1% more efficient** for the particular benchmark I was looking into. Differential Revision: D67689065 --- yoga/algorithm/PixelGrid.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/yoga/algorithm/PixelGrid.cpp b/yoga/algorithm/PixelGrid.cpp index 7a694565..6083f0f2 100644 --- a/yoga/algorithm/PixelGrid.cpp +++ b/yoga/algorithm/PixelGrid.cpp @@ -66,7 +66,8 @@ void roundLayoutResultsToPixelGrid( yoga::Node* const node, const double absoluteLeft, const double absoluteTop) { - const auto pointScaleFactor = node->getConfig()->getPointScaleFactor(); + const auto pointScaleFactor = + static_cast(node->getConfig()->getPointScaleFactor()); const double nodeLeft = node->getLayout().position(PhysicalEdge::Left); const double nodeTop = node->getLayout().position(PhysicalEdge::Top); @@ -80,7 +81,7 @@ void roundLayoutResultsToPixelGrid( const double absoluteNodeRight = absoluteNodeLeft + nodeWidth; const double absoluteNodeBottom = absoluteNodeTop + nodeHeight; - if (pointScaleFactor != 0.0f) { + if (pointScaleFactor != 0.0) { // If a node has a custom measure function we never want to round down its // size as this could lead to unwanted text truncation. const bool textRounding = node->getNodeType() == NodeType::Text; @@ -96,12 +97,14 @@ void roundLayoutResultsToPixelGrid( // We multiply dimension by scale factor and if the result is close to the // whole number, we don't have any fraction To verify if the result is close // to whole number we want to check both floor and ceil numbers + + const double scaledNodeWith = nodeWidth * pointScaleFactor; const bool hasFractionalWidth = - !yoga::inexactEquals(fmod(nodeWidth * pointScaleFactor, 1.0), 0) && - !yoga::inexactEquals(fmod(nodeWidth * pointScaleFactor, 1.0), 1.0); + !yoga::inexactEquals(round(scaledNodeWith), scaledNodeWith); + + const double scaledNodeHeight = nodeHeight * pointScaleFactor; const bool hasFractionalHeight = - !yoga::inexactEquals(fmod(nodeHeight * pointScaleFactor, 1.0), 0) && - !yoga::inexactEquals(fmod(nodeHeight * pointScaleFactor, 1.0), 1.0); + !yoga::inexactEquals(round(scaledNodeHeight), scaledNodeHeight); node->setLayoutDimension( roundValueToPixelGrid( -- 2.50.1.windows.1 From 76ab9b846a8714192f4786e0ab696205156d3745 Mon Sep 17 00:00:00 2001 From: Ruslan Shestopalyuk Date: Mon, 30 Dec 2024 02:51:00 -0800 Subject: [PATCH 2/2] Add more unit tests for rounding values to pixel grid (#1776) Summary: # Changelog: [Internal] - I was looking into some profiling results and noticed that Yoga has some quirky custom logic when it comes to rounding to the pixel grid, especially for the values that are halfway through. There are already unit tests in place, but those cases weren't covered, so I am adding these extra corner cases here, to make sure they are covered in case that the actual rounding function may get modified (which it might make sense to, as it's currently can be more expensive than it could have been in some scenarios). Differential Revision: D67712673 --- tests/YGRoundingFunctionTest.cpp | 40 ++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/YGRoundingFunctionTest.cpp b/tests/YGRoundingFunctionTest.cpp index b49d0644..55b5e86d 100644 --- a/tests/YGRoundingFunctionTest.cpp +++ b/tests/YGRoundingFunctionTest.cpp @@ -8,6 +8,8 @@ #include #include +#include + TEST(YogaTest, rounding_value) { // Test that whole numbers are rounded to whole despite ceil/floor flags ASSERT_FLOAT_EQ(6.0, YGRoundValueToPixelGrid(6.000001, 2.0, false, false)); @@ -39,6 +41,44 @@ TEST(YogaTest, rounding_value) { ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-5.99, 2.0, false, false)); ASSERT_FLOAT_EQ(-5.5, YGRoundValueToPixelGrid(-5.99, 2.0, true, false)); ASSERT_FLOAT_EQ(-6.0, YGRoundValueToPixelGrid(-5.99, 2.0, false, true)); + + // Rounding up/down halfway values is as expected for both positive and + // negative numbers + ASSERT_FLOAT_EQ(-3, YGRoundValueToPixelGrid(-3.5, 1.0, false, false)); + ASSERT_FLOAT_EQ(-3, YGRoundValueToPixelGrid(-3.4, 1.0, false, false)); + ASSERT_FLOAT_EQ(-4, YGRoundValueToPixelGrid(-3.6, 1.0, false, false)); + ASSERT_FLOAT_EQ(-3, YGRoundValueToPixelGrid(-3.499999, 1.0, false, false)); + ASSERT_FLOAT_EQ(-3, YGRoundValueToPixelGrid(-3.500001, 1.0, false, false)); + ASSERT_FLOAT_EQ(-4, YGRoundValueToPixelGrid(-3.5001, 1.0, false, false)); + + ASSERT_FLOAT_EQ(-3, YGRoundValueToPixelGrid(-3.5, 1.0, true, false)); + ASSERT_FLOAT_EQ(-3, YGRoundValueToPixelGrid(-3.4, 1.0, true, false)); + ASSERT_FLOAT_EQ(-3, YGRoundValueToPixelGrid(-3.6, 1.0, true, false)); + ASSERT_FLOAT_EQ(-3, YGRoundValueToPixelGrid(-3.499999, 1.0, true, false)); + ASSERT_FLOAT_EQ(-3, YGRoundValueToPixelGrid(-3.500001, 1.0, true, false)); + ASSERT_FLOAT_EQ(-3, YGRoundValueToPixelGrid(-3.5001, 1.0, true, false)); + ASSERT_FLOAT_EQ(-3, YGRoundValueToPixelGrid(-3.00001, 1.0, true, false)); + ASSERT_FLOAT_EQ(-3, YGRoundValueToPixelGrid(-3, 1.0, true, false)); + + ASSERT_FLOAT_EQ(-4, YGRoundValueToPixelGrid(-3.5, 1.0, false, true)); + ASSERT_FLOAT_EQ(-4, YGRoundValueToPixelGrid(-3.4, 1.0, false, true)); + ASSERT_FLOAT_EQ(-4, YGRoundValueToPixelGrid(-3.6, 1.0, false, true)); + ASSERT_FLOAT_EQ(-4, YGRoundValueToPixelGrid(-3.499999, 1.0, false, true)); + ASSERT_FLOAT_EQ(-4, YGRoundValueToPixelGrid(-3.500001, 1.0, false, true)); + ASSERT_FLOAT_EQ(-4, YGRoundValueToPixelGrid(-3.5001, 1.0, false, true)); + ASSERT_FLOAT_EQ(-3, YGRoundValueToPixelGrid(-3.00001, 1.0, false, true)); + ASSERT_FLOAT_EQ(-3, YGRoundValueToPixelGrid(-3, 1.0, false, true)); + + // NAN is treated as expected: + ASSERT_TRUE(std::isnan(YGRoundValueToPixelGrid( + std::numeric_limits::quiet_NaN(), 1.5, false, false))); + ASSERT_TRUE(std::isnan(YGRoundValueToPixelGrid( + 1.5, std::numeric_limits::quiet_NaN(), false, false))); + ASSERT_TRUE(std::isnan(YGRoundValueToPixelGrid( + std::numeric_limits::quiet_NaN(), + std::numeric_limits::quiet_NaN(), + false, + false))); } static YGSize measureText( -- 2.50.1.windows.1