Skip to content

Commit

Permalink
Allow lazy resolution of edge dimension values (facebook#41347)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebook#41347

X-link: facebook/yoga#1453

This follows the previous patterns used for `Gutters` and `Dimension`, where we hide CompactValue array implementation from `yoga::Style` callers.

This allows a single read of a style to only need access to the resolved values of a single edge, vs all edges. This is cheap now because the interface is the representation, but gets expensive if `StyleValuePool` is the actual implementation.

This prevents us from needing to resolve nine dimensions, in order to read a single value like `marginLeft`. Doing this, in the new style, also lets us remove `IdxRef` from the API.

We unroll the structure dependent parts in the props parsing code, for something more verbose, but also a bit clearer.

Changelog: [Internal]

Reviewed By: joevilches

Differential Revision: D50998164

fbshipit-source-id: 248396f9587e29d62cde05ae7512d8194f60c809
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Nov 14, 2023
1 parent e51a058 commit 6c4cb02
Show file tree
Hide file tree
Showing 21 changed files with 828 additions and 613 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ - (void)setUp
props.accessible = true;
auto &yogaStyle = props.yogaStyle;
yogaStyle.positionType() = yoga::PositionType::Absolute;
yogaStyle.position()[YGEdgeLeft] = YGValue{0, YGUnitPoint};
yogaStyle.position()[YGEdgeTop] = YGValue{0, YGUnitPoint};
yogaStyle.setPosition(YGEdgeLeft, yoga::CompactValue::of<YGUnitPoint>(0));
yogaStyle.setPosition(YGEdgeTop, yoga::CompactValue::of<YGUnitPoint>(0));
yogaStyle.setDimension(yoga::Dimension::Width, yoga::CompactValue::of<YGUnitPoint>(200));
yogaStyle.setDimension(yoga::Dimension::Height, yoga::CompactValue::of<YGUnitPoint>(200));
return sharedProps;
Expand Down Expand Up @@ -214,8 +214,8 @@ - (void)setUp
props.accessible = true;
auto &yogaStyle = props.yogaStyle;
yogaStyle.positionType() = yoga::PositionType::Absolute;
yogaStyle.position()[YGEdgeLeft] = YGValue{0, YGUnitPoint};
yogaStyle.position()[YGEdgeTop] = YGValue{30, YGUnitPoint};
yogaStyle.setPosition(YGEdgeLeft, yoga::CompactValue::of<YGUnitPoint>(0));
yogaStyle.setPosition(YGEdgeTop, yoga::CompactValue::of<YGUnitPoint>(30));
yogaStyle.setDimension(yoga::Dimension::Width, yoga::CompactValue::of<YGUnitPoint>(200));
yogaStyle.setDimension(yoga::Dimension::Height, yoga::CompactValue::of<YGUnitPoint>(50));
return sharedProps;
Expand Down Expand Up @@ -258,8 +258,8 @@ - (void)setUp
props.accessible = true;
auto &yogaStyle = props.yogaStyle;
yogaStyle.positionType() = yoga::PositionType::Absolute;
yogaStyle.position()[YGEdgeLeft] = YGValue{0, YGUnitPoint};
yogaStyle.position()[YGEdgeTop] = YGValue{90, YGUnitPoint};
yogaStyle.setPosition(YGEdgeLeft, yoga::CompactValue::of<YGUnitPoint>(0));
yogaStyle.setPosition(YGEdgeTop, yoga::CompactValue::of<YGUnitPoint>(90));
yogaStyle.setDimension(yoga::Dimension::Width, yoga::CompactValue::of<YGUnitPoint>(200));
yogaStyle.setDimension(yoga::Dimension::Height, yoga::CompactValue::of<YGUnitPoint>(50));
return sharedProps;
Expand Down Expand Up @@ -432,8 +432,8 @@ - (void)testEntireParagraphLink
props.accessibilityTraits = AccessibilityTraits::Link;
auto &yogaStyle = props.yogaStyle;
yogaStyle.positionType() = yoga::PositionType::Absolute;
yogaStyle.position()[YGEdgeLeft] = YGValue{0, YGUnitPoint};
yogaStyle.position()[YGEdgeTop] = YGValue{0, YGUnitPoint};
yogaStyle.setPosition(YGEdgeLeft, yoga::CompactValue::of<YGUnitPoint>(0));
yogaStyle.setPosition(YGEdgeTop, yoga::CompactValue::of<YGUnitPoint>(0));
yogaStyle.setDimension(yoga::Dimension::Width, yoga::CompactValue::of<YGUnitPoint>(200));
yogaStyle.setDimension(yoga::Dimension::Height, yoga::CompactValue::of<YGUnitPoint>(20));
return sharedProps;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ class SafeAreaViewComponentDescriptor final
: public ConcreteComponentDescriptor<SafeAreaViewShadowNode> {
using ConcreteComponentDescriptor::ConcreteComponentDescriptor;
void adopt(ShadowNode& shadowNode) const override {
auto& layoutableShadowNode =
auto& yogaLayoutableShadowNode =
static_cast<YogaLayoutableShadowNode&>(shadowNode);
auto& stateData = static_cast<const SafeAreaViewShadowNode::ConcreteState&>(
*shadowNode.getState())
.getData();
layoutableShadowNode.setPadding(stateData.padding);
yogaLayoutableShadowNode.setPadding(stateData.padding);

ConcreteComponentDescriptor::adopt(shadowNode);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class AndroidTextInputComponentDescriptor final
const ShadowNodeFamily::Shared& family) const override {
int surfaceId = family->getSurfaceId();

yoga::Style::Edges theme;
ThemePadding theme;
// TODO: figure out RTL/start/end/left/right stuff here
if (surfaceIdToThemePaddingMap_.find(surfaceId) !=
surfaceIdToThemePaddingMap_.end()) {
Expand All @@ -59,11 +59,16 @@ class AndroidTextInputComponentDescriptor final
fabricUIManager, surfaceId, defaultTextInputPaddingArray)) {
jfloat* defaultTextInputPadding =
env->GetFloatArrayElements(defaultTextInputPaddingArray, 0);
theme[YGEdgeStart] = (YGValue){defaultTextInputPadding[0], YGUnitPoint};
theme[YGEdgeEnd] = (YGValue){defaultTextInputPadding[1], YGUnitPoint};
theme[YGEdgeTop] = (YGValue){defaultTextInputPadding[2], YGUnitPoint};
theme[YGEdgeBottom] =
(YGValue){defaultTextInputPadding[3], YGUnitPoint};

theme.start =
yoga::CompactValue::of<YGUnitPoint>(defaultTextInputPadding[0]);
theme.end =
yoga::CompactValue::of<YGUnitPoint>(defaultTextInputPadding[1]);
theme.top =
yoga::CompactValue::of<YGUnitPoint>(defaultTextInputPadding[2]);
theme.bottom =
yoga::CompactValue::of<YGUnitPoint>(defaultTextInputPadding[3]);

surfaceIdToThemePaddingMap_.emplace(std::make_pair(surfaceId, theme));
env->ReleaseFloatArrayElements(
defaultTextInputPaddingArray, defaultTextInputPadding, JNI_ABORT);
Expand All @@ -77,10 +82,10 @@ class AndroidTextInputComponentDescriptor final
{},
{},
{},
((YGValue)theme[YGEdgeStart]).value,
((YGValue)theme[YGEdgeEnd]).value,
((YGValue)theme[YGEdgeTop]).value,
((YGValue)theme[YGEdgeBottom]).value)),
((YGValue)theme.start).value,
((YGValue)theme.end).value,
((YGValue)theme.top).value,
((YGValue)theme.bottom).value)),
family);
}

Expand All @@ -99,7 +104,7 @@ class AndroidTextInputComponentDescriptor final
int surfaceId = textInputShadowNode.getSurfaceId();
if (surfaceIdToThemePaddingMap_.find(surfaceId) !=
surfaceIdToThemePaddingMap_.end()) {
yoga::Style::Edges theme = surfaceIdToThemePaddingMap_[surfaceId];
ThemePadding theme = surfaceIdToThemePaddingMap_[surfaceId];

auto& textInputProps = textInputShadowNode.getConcreteProps();

Expand All @@ -108,29 +113,34 @@ class AndroidTextInputComponentDescriptor final
// TODO: T62959168 account for RTL and paddingLeft when setting default
// paddingStart, and vice-versa with paddingRight/paddingEnd.
// For now this assumes no RTL.
yoga::Style::Edges result = textInputProps.yogaStyle.padding();
ThemePadding result{
.start = textInputProps.yogaStyle.padding(YGEdgeStart),
.end = textInputProps.yogaStyle.padding(YGEdgeEnd),
.top = textInputProps.yogaStyle.padding(YGEdgeTop),
.bottom = textInputProps.yogaStyle.padding(YGEdgeBottom)};

bool changedPadding = false;
if (!textInputProps.hasPadding && !textInputProps.hasPaddingStart &&
!textInputProps.hasPaddingLeft &&
!textInputProps.hasPaddingHorizontal) {
changedPadding = true;
result[YGEdgeStart] = theme[YGEdgeStart];
result.start = theme.start;
}
if (!textInputProps.hasPadding && !textInputProps.hasPaddingEnd &&
!textInputProps.hasPaddingRight &&
!textInputProps.hasPaddingHorizontal) {
changedPadding = true;
result[YGEdgeEnd] = theme[YGEdgeEnd];
result.end = theme.end;
}
if (!textInputProps.hasPadding && !textInputProps.hasPaddingTop &&
!textInputProps.hasPaddingVertical) {
changedPadding = true;
result[YGEdgeTop] = theme[YGEdgeTop];
result.top = theme.top;
}
if (!textInputProps.hasPadding && !textInputProps.hasPaddingBottom &&
!textInputProps.hasPaddingVertical) {
changedPadding = true;
result[YGEdgeBottom] = theme[YGEdgeBottom];
result.bottom = theme.bottom;
}

// If the TextInput initially does not have paddingLeft or paddingStart, a
Expand All @@ -141,21 +151,26 @@ class AndroidTextInputComponentDescriptor final
if ((textInputProps.hasPadding || textInputProps.hasPaddingLeft ||
textInputProps.hasPaddingHorizontal) &&
!textInputProps.hasPaddingStart) {
result[YGEdgeStart] = YGValueUndefined;
result.start = yoga::CompactValue::ofUndefined();
}
if ((textInputProps.hasPadding || textInputProps.hasPaddingRight ||
textInputProps.hasPaddingHorizontal) &&
!textInputProps.hasPaddingEnd) {
result[YGEdgeEnd] = YGValueUndefined;
result.end = yoga::CompactValue::ofUndefined();
}

// Note that this is expensive: on every adopt, we need to set the Yoga
// props again, which normally only happens during prop parsing. Every
// commit, state update, etc, will incur this cost.
if (changedPadding) {
// Set new props on node
const_cast<AndroidTextInputProps&>(textInputProps).yogaStyle.padding() =
result;
yoga::Style& style =
const_cast<AndroidTextInputProps&>(textInputProps).yogaStyle;
style.setPadding(YGEdgeStart, result.start);
style.setPadding(YGEdgeEnd, result.end);
style.setPadding(YGEdgeTop, result.top);
style.setPadding(YGEdgeBottom, result.bottom);

// Communicate new props to Yoga part of the node
textInputShadowNode.updateYogaProps();
}
Expand All @@ -168,13 +183,19 @@ class AndroidTextInputComponentDescriptor final
}

private:
struct ThemePadding {
yoga::CompactValue start;
yoga::CompactValue end;
yoga::CompactValue top;
yoga::CompactValue bottom;
};

// TODO T68526882: Unify with Binding::UIManagerJavaDescriptor
constexpr static auto UIManagerJavaDescriptor =
"com/facebook/react/fabric/FabricUIManager";

SharedTextLayoutManager textLayoutManager_;
mutable std::unordered_map<int, yoga::Style::Edges>
surfaceIdToThemePaddingMap_;
mutable std::unordered_map<int, ThemePadding> surfaceIdToThemePaddingMap_;
};

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -358,20 +358,20 @@ BorderMetrics BaseViewProps::resolveBorderMetrics(
bool{layoutMetrics.layoutDirection == LayoutDirection::RightToLeft};

auto borderWidths = CascadedBorderWidths{
/* .left = */ optionalFloatFromYogaValue(yogaStyle.border()[YGEdgeLeft]),
/* .top = */ optionalFloatFromYogaValue(yogaStyle.border()[YGEdgeTop]),
/* .left = */ optionalFloatFromYogaValue(yogaStyle.border(YGEdgeLeft)),
/* .top = */ optionalFloatFromYogaValue(yogaStyle.border(YGEdgeTop)),
/* .right = */
optionalFloatFromYogaValue(yogaStyle.border()[YGEdgeRight]),
optionalFloatFromYogaValue(yogaStyle.border(YGEdgeRight)),
/* .bottom = */
optionalFloatFromYogaValue(yogaStyle.border()[YGEdgeBottom]),
optionalFloatFromYogaValue(yogaStyle.border(YGEdgeBottom)),
/* .start = */
optionalFloatFromYogaValue(yogaStyle.border()[YGEdgeStart]),
/* .end = */ optionalFloatFromYogaValue(yogaStyle.border()[YGEdgeEnd]),
optionalFloatFromYogaValue(yogaStyle.border(YGEdgeStart)),
/* .end = */ optionalFloatFromYogaValue(yogaStyle.border(YGEdgeEnd)),
/* .horizontal = */
optionalFloatFromYogaValue(yogaStyle.border()[YGEdgeHorizontal]),
optionalFloatFromYogaValue(yogaStyle.border(YGEdgeHorizontal)),
/* .vertical = */
optionalFloatFromYogaValue(yogaStyle.border()[YGEdgeVertical]),
/* .all = */ optionalFloatFromYogaValue(yogaStyle.border()[YGEdgeAll]),
optionalFloatFromYogaValue(yogaStyle.border(YGEdgeVertical)),
/* .all = */ optionalFloatFromYogaValue(yogaStyle.border(YGEdgeAll)),
};

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,17 @@ void ViewShadowNode::initialize() noexcept {
viewProps.removeClippedSubviews ||
HostPlatformViewTraitsInitializer::formsStackingContext(viewProps);

bool hasBorder = [&]() {
for (int edge = YGEdgeLeft; edge != YGEdgeAll; ++edge) {
if (viewProps.yogaStyle.border(static_cast<YGEdge>(edge)).isDefined()) {
return true;
}
}
return false;
}();

bool formsView = formsStackingContext ||
isColorMeaningful(viewProps.backgroundColor) ||
!(viewProps.yogaStyle.border() == yoga::Style::Edges{}) ||
isColorMeaningful(viewProps.backgroundColor) || hasBorder ||
!viewProps.testId.empty() ||
HostPlatformViewTraitsInitializer::formsView(viewProps);

Expand Down
Loading

0 comments on commit 6c4cb02

Please sign in to comment.