From f86c74ce7e34b2943f8f3a942aeedac43dd7dc73 Mon Sep 17 00:00:00 2001 From: David Aurelio Date: Tue, 19 Feb 2019 09:54:45 -0800 Subject: [PATCH] Call measure and baseline fns within `YGNode` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: @public Stricter encapsulation of baseline and measure callbacks withing `YGNode`. Instead of invoking these callbacks directly (`node->getBaseline()(...)`), they are invoked via methods on `YGNode` (`node->baseline(...)`). This change will allow us to add the concept of a *Layout Context,* where measure and baseline functions will be able to receive an additional `void *` argument if configured accordingly. This API will be used internally for Yoga’s JNI bindings, to avoid storing a weak JNI reference for each node, and avoid reference table overflows. Changed API: - `YGNodeGetMeasureFunc()` -> `YGNodeHasMeasureFunc()` - `YGNodeGetBaselineFunc()` -> `YGNodeHasBaselineFunc()` - `YGNode::getMeasure()` -> `YGNode::hasMeasureFunc()` + `YGNode::measure()` - `YGNpde::getBaseline()` -> `YGNode::hasBaselineFunc()` + `YGNode::baseline()` Reviewed By: SidharthGuglani Differential Revision: D14099550 fbshipit-source-id: 2653ab36acc252a9747986bc88d21dac22d8c91b --- YogaKit/Source/YGLayout.m | 2 +- tests/YGMeasureTest.cpp | 61 +++++++++++++++------------ tests/YGNodeCallbackTest.cpp | 81 ++++++++++++++++++++++++++++++++++++ yoga/YGNode.cpp | 4 +- yoga/YGNode.h | 20 +++++++-- yoga/YGNodePrint.cpp | 2 +- yoga/Yoga.cpp | 26 ++++++------ yoga/Yoga.h | 4 +- yoga/instrumentation.h | 7 +++- 9 files changed, 154 insertions(+), 53 deletions(-) create mode 100644 tests/YGNodeCallbackTest.cpp diff --git a/YogaKit/Source/YGLayout.m b/YogaKit/Source/YGLayout.m index e333765f..94a1e550 100644 --- a/YogaKit/Source/YGLayout.m +++ b/YogaKit/Source/YGLayout.m @@ -180,7 +180,7 @@ static YGConfigRef globalConfig; // the measure function. Since we already know that this is a leaf, // this *should* be fine. Forgive me Hack Gods. const YGNodeRef node = self.node; - if (YGNodeGetMeasureFunc(node) == NULL) { + if (YGNodeHasMeasureFunc(node)) { YGNodeSetMeasureFunc(node, YGMeasureView); } diff --git a/tests/YGMeasureTest.cpp b/tests/YGMeasureTest.cpp index a330523d..f41ec017 100644 --- a/tests/YGMeasureTest.cpp +++ b/tests/YGMeasureTest.cpp @@ -8,45 +8,51 @@ #include #include -static YGSize _measure(YGNodeRef node, - float width, - YGMeasureMode widthMode, - float height, - YGMeasureMode heightMode) { - int* measureCount = (int*)node->getContext(); +static YGSize _measure( + YGNodeRef node, + float width, + YGMeasureMode widthMode, + float height, + YGMeasureMode heightMode) { + int* measureCount = (int*) node->getContext(); if (measureCount) { (*measureCount)++; } return YGSize{ - .width = 10, .height = 10, + .width = 10, + .height = 10, }; } -static YGSize _simulate_wrapping_text(YGNodeRef node, - float width, - YGMeasureMode widthMode, - float height, - YGMeasureMode heightMode) { +static YGSize _simulate_wrapping_text( + YGNodeRef node, + float width, + YGMeasureMode widthMode, + float height, + YGMeasureMode heightMode) { if (widthMode == YGMeasureModeUndefined || width >= 68) { return YGSize{.width = 68, .height = 16}; } return YGSize{ - .width = 50, .height = 32, + .width = 50, + .height = 32, }; } -static YGSize _measure_assert_negative(YGNodeRef node, - float width, - YGMeasureMode widthMode, - float height, - YGMeasureMode heightMode) { +static YGSize _measure_assert_negative( + YGNodeRef node, + float width, + YGMeasureMode widthMode, + float height, + YGMeasureMode heightMode) { EXPECT_GE(width, 0); EXPECT_GE(height, 0); return YGSize{ - .width = 0, .height = 0, + .width = 0, + .height = 0, }; } @@ -148,7 +154,6 @@ TEST(YogaTest, dont_measure_when_min_equals_max_percentages) { YGNodeFreeRecursive(root); } - TEST(YogaTest, measure_nodes_with_margin_auto_and_stretch) { const YGNodeRef root = YGNodeNew(); YGNodeStyleSetWidth(root, 500); @@ -593,7 +598,7 @@ TEST(YogaTest, can_nullify_measure_func_on_any_node) { const YGNodeRef root = YGNodeNew(); YGNodeInsertChild(root, YGNodeNew(), 0); root->setMeasureFunc(nullptr); - ASSERT_TRUE(root->getMeasure() == NULL); + ASSERT_TRUE(!root->hasMeasureFunc()); YGNodeFreeRecursive(root); } @@ -635,14 +640,16 @@ TEST(YogaTest, cant_call_negative_measure_horizontal) { YGConfigFree(config); } -static YGSize _measure_90_10(YGNodeRef node, - float width, - YGMeasureMode widthMode, - float height, - YGMeasureMode heightMode) { +static YGSize _measure_90_10( + YGNodeRef node, + float width, + YGMeasureMode widthMode, + float height, + YGMeasureMode heightMode) { return YGSize{ - .width = 90, .height = 10, + .width = 90, + .height = 10, }; } diff --git a/tests/YGNodeCallbackTest.cpp b/tests/YGNodeCallbackTest.cpp new file mode 100644 index 00000000..234fc9c1 --- /dev/null +++ b/tests/YGNodeCallbackTest.cpp @@ -0,0 +1,81 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the LICENSE + * file in the root directory of this source tree. + */ +#include +#include +#include + +inline bool operator==(const YGSize& lhs, const YGSize& rhs) { + return lhs.width == rhs.width && lhs.height == rhs.height; +} + +void PrintTo(const YGSize&, std::ostream*); + +TEST(YGNode, hasMeasureFunc_initial) { + auto n = YGNode{}; + ASSERT_FALSE(n.hasMeasureFunc()); +} + +TEST(YGNode, hasMeasureFunc_with_measure_fn) { + auto n = YGNode{}; + n.setMeasureFunc([](YGNode*, float, YGMeasureMode, float, YGMeasureMode) { + return YGSize{}; + }); + ASSERT_TRUE(n.hasMeasureFunc()); +} + +TEST(YGNode, measure_with_measure_fn) { + auto n = YGNode{}; + + n.setMeasureFunc( + [](YGNode*, float w, YGMeasureMode wm, float h, YGMeasureMode hm) { + return YGSize{w * wm, h / hm}; + }); + + ASSERT_EQ( + n.measure(23, YGMeasureModeExactly, 24, YGMeasureModeAtMost), + (YGSize{23, 12})); +} + +TEST(YGNode, hasMeasureFunc_after_unset) { + auto n = YGNode{}; + n.setMeasureFunc([](YGNode*, float, YGMeasureMode, float, YGMeasureMode) { + return YGSize{}; + }); + + n.setMeasureFunc(nullptr); + ASSERT_FALSE(n.hasMeasureFunc()); +} + +TEST(YGNode, hasBaselineFunc_initial) { + auto n = YGNode{}; + ASSERT_FALSE(n.hasBaselineFunc()); +} + +TEST(YGNode, hasBaselineFunc_with_baseline_fn) { + auto n = YGNode{}; + n.setBaseLineFunc([](YGNode*, float, float) { return 0.0f; }); + ASSERT_TRUE(n.hasBaselineFunc()); +} + +TEST(YGNode, baseline_with_baseline_fn) { + auto n = YGNode{}; + n.setBaseLineFunc([](YGNode*, float w, float h) { return w + h; }); + + ASSERT_EQ(n.baseline(1.25f, 2.5f), 3.75f); +} + +TEST(YGNode, hasBaselineFunc_after_unset) { + auto n = YGNode{}; + n.setBaseLineFunc([](YGNode*, float, float) { return 0.0f; }); + + n.setBaseLineFunc(nullptr); + ASSERT_FALSE(n.hasBaselineFunc()); +} + +void PrintTo(const YGSize& size, std::ostream* os) { + *os << "YGSize{" << size.width << ", " << size.height << "}"; +} diff --git a/yoga/YGNode.cpp b/yoga/YGNode.cpp index 0b7fc3c7..9b06cb9c 100644 --- a/yoga/YGNode.cpp +++ b/yoga/YGNode.cpp @@ -270,8 +270,8 @@ YGNode& YGNode::operator=(const YGNode& node) { print_ = node.getPrintFunc(); hasNewLayout_ = node.getHasNewLayout(); nodeType_ = node.getNodeType(); - measure_ = node.getMeasure(); - baseline_ = node.getBaseline(); + measure_ = node.measure_; + baseline_ = node.baseline_; dirtied_ = node.getDirtied(); style_ = node.style_; layout_ = node.layout_; diff --git a/yoga/YGNode.h b/yoga/YGNode.h index ffb01832..776af0f9 100644 --- a/yoga/YGNode.h +++ b/yoga/YGNode.h @@ -63,12 +63,24 @@ public: return nodeType_; } - YGMeasureFunc getMeasure() const { - return measure_; + bool hasMeasureFunc() const noexcept { + return measure_ != nullptr; } - YGBaselineFunc getBaseline() const { - return baseline_; + YGSize measure( + float width, + YGMeasureMode widthMode, + float height, + YGMeasureMode heightMode) { + return measure_(this, width, widthMode, height, heightMode); + } + + bool hasBaselineFunc() const noexcept { + return baseline_ != nullptr; + } + + float baseline(float width, float height) { + return baseline_(this, width, height); } YGDirtiedFunc getDirtied() const { diff --git a/yoga/YGNodePrint.cpp b/yoga/YGNodePrint.cpp index 28b1c47e..5e51a405 100644 --- a/yoga/YGNodePrint.cpp +++ b/yoga/YGNodePrint.cpp @@ -214,7 +214,7 @@ void YGNodeToString( str, "bottom", node->getStyle().position, YGEdgeBottom); appendFormatedString(str, "\" "); - if (node->getMeasure() != nullptr) { + if (node->hasMeasureFunc()) { appendFormatedString(str, "has-custom-measure=\"true\""); } } diff --git a/yoga/Yoga.cpp b/yoga/Yoga.cpp index f15864db..2559b80b 100644 --- a/yoga/Yoga.cpp +++ b/yoga/Yoga.cpp @@ -145,16 +145,16 @@ void YGNodeSetContext(YGNodeRef node, void* context) { return node->setContext(context); } -YGMeasureFunc YGNodeGetMeasureFunc(YGNodeRef node) { - return node->getMeasure(); +bool YGNodeHasMeasureFunc(YGNodeRef node) { + return node->hasMeasureFunc(); } void YGNodeSetMeasureFunc(YGNodeRef node, YGMeasureFunc measureFunc) { node->setMeasureFunc(measureFunc); } -YGBaselineFunc YGNodeGetBaselineFunc(YGNodeRef node) { - return node->getBaseline(); +bool YGNodeHasBaselineFunc(YGNodeRef node) { + return node->hasBaselineFunc(); } void YGNodeSetBaselineFunc(YGNodeRef node, YGBaselineFunc baselineFunc) { @@ -395,7 +395,7 @@ void YGNodeInsertChild( YGAssertWithNode( node, - node->getMeasure() == nullptr, + !node->hasMeasureFunc(), "Cannot add child: Nodes with measure functions cannot have children."); node->cloneChildrenIfNeeded(); @@ -555,7 +555,7 @@ YGNodeRef YGNodeGetParent(const YGNodeRef node) { void YGNodeMarkDirty(const YGNodeRef node) { YGAssertWithNode( node, - node->getMeasure() != nullptr, + node->hasMeasureFunc(), "Only leaf nodes with custom measure functions" "should manually mark themselves as dirty"); @@ -1075,11 +1075,10 @@ static inline YGAlign YGNodeAlignItem( } static float YGBaseline(const YGNodeRef node) { - if (node->getBaseline() != nullptr) { + if (node->hasBaselineFunc()) { const float baseline = marker::MarkerSection::wrap( node, - node->getBaseline(), - node, + &YGNode::baseline, node->getLayout().measuredDimensions[YGDimensionWidth], node->getLayout().measuredDimensions[YGDimensionHeight]); YGAssertWithNode( @@ -1650,7 +1649,7 @@ static void YGNodeWithMeasureFuncSetMeasuredDimensions( const float ownerHeight) { YGAssertWithNode( node, - node->getMeasure() != nullptr, + node->hasMeasureFunc(), "Expected node to have custom measure function"); const float paddingAndBorderAxisRow = @@ -1694,8 +1693,7 @@ static void YGNodeWithMeasureFuncSetMeasuredDimensions( // Measure the text under the current constraints. const YGSize measuredSize = marker::MarkerSection::wrap( node, - node->getMeasure(), - node, + &YGNode::measure, innerWidth, widthMeasureMode, innerHeight, @@ -2737,7 +2735,7 @@ static void YGNodelayoutImpl( node->getTrailingPadding(flexColumnDirection, ownerWidth).unwrap(), YGEdgeBottom); - if (node->getMeasure() != nullptr) { + if (node->hasMeasureFunc()) { YGNodeWithMeasureFuncSetMeasuredDimensions( node, availableWidth, @@ -3741,7 +3739,7 @@ bool YGLayoutNodeInternal( // dimensions. We handle nodes with measure functions specially here because // they are the most expensive to measure, so it's worth avoiding redundant // measurements if at all possible. - if (node->getMeasure() != nullptr) { + if (node->hasMeasureFunc()) { const float marginAxisRow = node->getMarginForAxis(YGFlexDirectionRow, ownerWidth).unwrap(); const float marginAxisColumn = diff --git a/yoga/Yoga.h b/yoga/Yoga.h index e3ca1fea..86aa152a 100644 --- a/yoga/Yoga.h +++ b/yoga/Yoga.h @@ -132,9 +132,9 @@ WIN_EXPORT void YGNodeCopyStyle( WIN_EXPORT void* YGNodeGetContext(YGNodeRef node); WIN_EXPORT void YGNodeSetContext(YGNodeRef node, void* context); void YGConfigSetPrintTreeFlag(YGConfigRef config, bool enabled); -YGMeasureFunc YGNodeGetMeasureFunc(YGNodeRef node); +bool YGNodeHasMeasureFunc(YGNodeRef node); WIN_EXPORT void YGNodeSetMeasureFunc(YGNodeRef node, YGMeasureFunc measureFunc); -YGBaselineFunc YGNodeGetBaselineFunc(YGNodeRef node); +bool YGNodeHasBaselineFunc(YGNodeRef node); void YGNodeSetBaselineFunc(YGNodeRef node, YGBaselineFunc baselineFunc); YGDirtiedFunc YGNodeGetDirtiedFunc(YGNodeRef node); void YGNodeSetDirtiedFunc(YGNodeRef node, YGDirtiedFunc dirtiedFunc); diff --git a/yoga/instrumentation.h b/yoga/instrumentation.h index 5cc544c5..b8691c18 100644 --- a/yoga/instrumentation.h +++ b/yoga/instrumentation.h @@ -28,9 +28,12 @@ public: typename Data::type data = {}; template - static Ret wrap(YGNodeRef node, Ret (*fn)(Args...), Args... args) { + static Ret wrap( + YGNodeRef node, + Ret (YGNode::*method)(Args...), + Args... args) { MarkerSection section{node}; - return fn(std::forward(args)...); + return (node->*method)(std::forward(args)...); } private: