Skip to content

Commit

Permalink
Remove legacy layout diffing
Browse files Browse the repository at this point in the history 
Summary:
This removes some unused flags which will cause Yoga to layout every tree twice, then diffing the tree, reporting whether the whole tree is different. This is too expensive to run outside of local experimentation, but we have more nuanced ways to implement the `YGNodeLayoutAffectedByQuirk` I am wanting to add.

Changelog: [Internal]

Reviewed By: lunaleaps

Differential Revision: D42406917

fbshipit-source-id: b415ed02768f6b59de3a6fa90c60c750d56fd4b0
  • Loading branch information
@NickGerleman @facebook-github-bot
NickGerleman authored and facebook-github-bot committed Jan 19, 2023
1 parent e162b07 commit 26580a3
Show file tree
Hide file tree
Showing 11 changed files with 1 addition and 246 deletions.
8 changes: 0 additions & 8 deletions ReactAndroid/src/main/java/com/facebook/yoga/YogaConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,6 @@ public abstract class YogaConfig {
*/
public abstract void setUseLegacyStretchBehaviour(boolean useLegacyStretchBehaviour);

/**
* If this flag is set then yoga would diff the layout without legacy flag and would set a bool in
* YogaNode(mDoesLegacyStretchFlagAffectsLayout) with true if the layouts were different and false
* if not
*/
public abstract void setShouldDiffLayoutWithoutLegacyStretchBehaviour(
boolean shouldDiffLayoutWithoutLegacyStretchBehaviour);

public abstract void setLogger(YogaLogger logger);

public abstract YogaLogger getLogger();
Expand Down
11 changes: 0 additions & 11 deletions ReactAndroid/src/main/java/com/facebook/yoga/YogaConfigJNIBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,6 @@ public void setUseLegacyStretchBehaviour(boolean useLegacyStretchBehaviour) {
YogaNative.jni_YGConfigSetUseLegacyStretchBehaviourJNI(mNativePointer, useLegacyStretchBehaviour);
}

/**
* If this flag is set then yoga would diff the layout without legacy flag and would set a bool in
* YogaNode(mDoesLegacyStretchFlagAffectsLayout) with true if the layouts were different and false
* if not
*/
public void setShouldDiffLayoutWithoutLegacyStretchBehaviour(
boolean shouldDiffLayoutWithoutLegacyStretchBehaviour) {
YogaNative.jni_YGConfigSetShouldDiffLayoutWithoutLegacyStretchBehaviourJNI(
mNativePointer, shouldDiffLayoutWithoutLegacyStretchBehaviour);
}

public void setLogger(YogaLogger logger) {
mLogger = logger;
YogaNative.jni_YGConfigSetLoggerJNI(mNativePointer, logger);
Expand Down
1 change: 0 additions & 1 deletion ReactAndroid/src/main/java/com/facebook/yoga/YogaNative.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ public class YogaNative {
static native void jni_YGConfigSetPrintTreeFlagJNI(long nativePointer, boolean enable);
static native void jni_YGConfigSetPointScaleFactorJNI(long nativePointer, float pixelsInPoint);
static native void jni_YGConfigSetUseLegacyStretchBehaviourJNI(long nativePointer, boolean useLegacyStretchBehaviour);
static native void jni_YGConfigSetShouldDiffLayoutWithoutLegacyStretchBehaviourJNI(long nativePointer, boolean shouldDiffLayoutWithoutLegacyStretchBehaviour);
static native void jni_YGConfigSetLoggerJNI(long nativePointer, YogaLogger logger);

// YGNode related
Expand Down
7 changes: 0 additions & 7 deletions ReactAndroid/src/main/java/com/facebook/yoga/YogaNodeJNIBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ public abstract class YogaNodeJNIBase extends YogaNode implements Cloneable {
private static final byte MARGIN = 1;
private static final byte PADDING = 2;
private static final byte BORDER = 4;
private static final byte DOES_LEGACY_STRETCH_BEHAVIOUR = 8;
private static final byte HAS_NEW_LAYOUT = 16;

private static final byte LAYOUT_EDGE_SET_FLAG_INDEX = 0;
Expand Down Expand Up @@ -606,12 +605,6 @@ public float getLayoutHeight() {
return arr != null ? arr[LAYOUT_HEIGHT_INDEX] : 0;
}

public boolean getDoesLegacyStretchFlagAffectsLayout() {
return arr != null
&& (((int) arr[LAYOUT_EDGE_SET_FLAG_INDEX] & DOES_LEGACY_STRETCH_BEHAVIOUR)
== DOES_LEGACY_STRETCH_BEHAVIOUR);
}

@Override
public float getLayoutMargin(YogaEdge edge) {
if (arr != null && ((int) arr[LAYOUT_EDGE_SET_FLAG_INDEX] & MARGIN) == MARGIN) {
Expand Down
1 change: 0 additions & 1 deletion ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNI.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ const short int LAYOUT_BORDER_START_INDEX = 14;

namespace {

const int DOES_LEGACY_STRETCH_BEHAVIOUR = 8;
const int HAS_NEW_LAYOUT = 16;

union YGNodeContext {
Expand Down
15 changes: 0 additions & 15 deletions ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNIVanilla.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,6 @@ static void jni_YGConfigSetExperimentalFeatureEnabledJNI(
config, static_cast<YGExperimentalFeature>(feature), enabled);
}

static void jni_YGConfigSetShouldDiffLayoutWithoutLegacyStretchBehaviourJNI(
JNIEnv* env,
jobject obj,
jlong nativePointer,
jboolean enabled) {
const YGConfigRef config = _jlong2YGConfigRef(nativePointer);
YGConfigSetShouldDiffLayoutWithoutLegacyStretchBehaviour(config, enabled);
}

static void jni_YGConfigSetUseWebDefaultsJNI(
JNIEnv* env,
jobject obj,
Expand Down Expand Up @@ -294,9 +285,6 @@ static void YGTransferLayoutOutputsRecursive(

int fieldFlags = edgesSet.get();
fieldFlags |= HAS_NEW_LAYOUT;
if (YGNodeLayoutGetDidLegacyStretchFlagAffectLayout(root)) {
fieldFlags |= DOES_LEGACY_STRETCH_BEHAVIOUR;
}

const int arrSize = 6 + (marginFieldSet ? 4 : 0) + (paddingFieldSet ? 4 : 0) +
(borderFieldSet ? 4 : 0);
Expand Down Expand Up @@ -776,9 +764,6 @@ static JNINativeMethod methods[] = {
{"jni_YGConfigSetUseLegacyStretchBehaviourJNI",
"(JZ)V",
(void*) jni_YGConfigSetUseLegacyStretchBehaviourJNI},
{"jni_YGConfigSetShouldDiffLayoutWithoutLegacyStretchBehaviourJNI",
"(JZ)V",
(void*) jni_YGConfigSetShouldDiffLayoutWithoutLegacyStretchBehaviourJNI},
{"jni_YGConfigSetLoggerJNI",
"(JLcom/facebook/yoga/YogaLogger;)V",
(void*) jni_YGConfigSetLoggerJNI},
Expand Down
25 changes: 1 addition & 24 deletions ReactCommon/yoga/yoga/YGLayout.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,8 @@ struct YGLayout {

private:
static constexpr size_t directionOffset = 0;
static constexpr size_t didUseLegacyFlagOffset =
directionOffset + facebook::yoga::detail::bitWidthFn<YGDirection>();
static constexpr size_t doesLegacyStretchFlagAffectsLayoutOffset =
didUseLegacyFlagOffset + 1;
static constexpr size_t hadOverflowOffset =
doesLegacyStretchFlagAffectsLayoutOffset + 1;
directionOffset + facebook::yoga::detail::bitWidthFn<YGDirection>();
uint8_t flags = 0;

public:
Expand Down Expand Up @@ -56,25 +52,6 @@ struct YGLayout {
flags, directionOffset, direction);
}

bool didUseLegacyFlag() const {
return facebook::yoga::detail::getBooleanData(
flags, didUseLegacyFlagOffset);
}

void setDidUseLegacyFlag(bool val) {
facebook::yoga::detail::setBooleanData(flags, didUseLegacyFlagOffset, val);
}

bool doesLegacyStretchFlagAffectsLayout() const {
return facebook::yoga::detail::getBooleanData(
flags, doesLegacyStretchFlagAffectsLayoutOffset);
}

void setDoesLegacyStretchFlagAffectsLayout(bool val) {
facebook::yoga::detail::setBooleanData(
flags, doesLegacyStretchFlagAffectsLayoutOffset, val);
}

bool hadOverflow() const {
return facebook::yoga::detail::getBooleanData(flags, hadOverflowOffset);
}
Expand Down
47 changes: 0 additions & 47 deletions ReactCommon/yoga/yoga/YGNode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -560,53 +560,6 @@ YGFloatOptional YGNode::getTrailingPaddingAndBorder(
YGFloatOptional(getTrailingBorder(axis));
}

bool YGNode::didUseLegacyFlag() {
bool didUseLegacyFlag = layout_.didUseLegacyFlag();
if (didUseLegacyFlag) {
return true;
}
for (const auto& child : children_) {
if (child->layout_.didUseLegacyFlag()) {
didUseLegacyFlag = true;
break;
}
}
return didUseLegacyFlag;
}

void YGNode::setLayoutDoesLegacyFlagAffectsLayout(
bool doesLegacyFlagAffectsLayout) {
layout_.setDoesLegacyStretchFlagAffectsLayout(doesLegacyFlagAffectsLayout);
}

void YGNode::setLayoutDidUseLegacyFlag(bool didUseLegacyFlag) {
layout_.setDidUseLegacyFlag(didUseLegacyFlag);
}

bool YGNode::isLayoutTreeEqualToNode(const YGNode& node) const {
if (children_.size() != node.children_.size()) {
return false;
}
if (layout_ != node.layout_) {
return false;
}
if (children_.size() == 0) {
return true;
}

bool isLayoutTreeEqual = true;
YGNodeRef otherNodeChildren = nullptr;
for (std::vector<YGNodeRef>::size_type i = 0; i < children_.size(); ++i) {
otherNodeChildren = node.children_[i];
isLayoutTreeEqual =
children_[i]->isLayoutTreeEqualToNode(*otherNodeChildren);
if (!isLayoutTreeEqual) {
return false;
}
}
return isLayoutTreeEqual;
}

void YGNode::reset() {
YGAssertWithNode(
this,
Expand Down
4 changes: 0 additions & 4 deletions ReactCommon/yoga/yoga/YGNode.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,6 @@ struct YOGA_EXPORT YGNode {
const float mainSize,
const float crossSize,
const float ownerWidth);
void setLayoutDoesLegacyFlagAffectsLayout(bool doesLegacyFlagAffectsLayout);
void setLayoutDidUseLegacyFlag(bool didUseLegacyFlag);
void markDirtyAndPropogateDownwards();

// Other methods
Expand All @@ -351,8 +349,6 @@ struct YOGA_EXPORT YGNode {
float resolveFlexGrow() const;
float resolveFlexShrink() const;
bool isNodeFlexible();
bool didUseLegacyFlag();
bool isLayoutTreeEqualToNode(const YGNode& node) const;
void reset();
};

Expand Down
123 changes: 0 additions & 123 deletions ReactCommon/yoga/yoga/Yoga.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,6 @@ YOGA_EXPORT bool YGNodeIsDirty(YGNodeRef node) {
return node->isDirty();
}

YOGA_EXPORT bool YGNodeLayoutGetDidUseLegacyFlag(const YGNodeRef node) {
return node->didUseLegacyFlag();
}

YOGA_EXPORT void YGNodeMarkDirtyAndPropogateToDescendants(
const YGNodeRef node) {
return node->markDirtyAndPropogateDownwards();
Expand Down Expand Up @@ -220,32 +216,6 @@ YOGA_EXPORT YGNodeRef YGNodeClone(YGNodeRef oldNode) {
return node;
}

static YGConfigRef YGConfigClone(const YGConfig& oldConfig) {
const YGConfigRef config = new YGConfig(oldConfig);
YGAssert(config != nullptr, "Could not allocate memory for config");
gConfigInstanceCount++;
return config;
}

static YGNodeRef YGNodeDeepClone(YGNodeRef oldNode) {
auto config = YGConfigClone(*oldNode->getConfig());
auto node = new YGNode{*oldNode, config};
node->setOwner(nullptr);
Event::publish<Event::NodeAllocation>(node, {node->getConfig()});

YGVector vec = YGVector();
vec.reserve(oldNode->getChildren().size());
YGNodeRef childNode = nullptr;
for (auto* item : oldNode->getChildren()) {
childNode = YGNodeDeepClone(item);
childNode->setOwner(node);
vec.push_back(childNode);
}
node->setChildren(vec);

return node;
}

YOGA_EXPORT void YGNodeFree(const YGNodeRef node) {
if (YGNodeRef owner = node->getOwner()) {
owner->removeChild(node);
Expand All @@ -263,17 +233,6 @@ YOGA_EXPORT void YGNodeFree(const YGNodeRef node) {
delete node;
}

static void YGConfigFreeRecursive(const YGNodeRef root) {
if (root->getConfig() != nullptr) {
gConfigInstanceCount--;
delete root->getConfig();
}
// Delete configs recursively for childrens
for (auto* child : root->getChildren()) {
YGConfigFreeRecursive(child);
}
}

YOGA_EXPORT void YGNodeFreeRecursiveWithCleanupFunc(
const YGNodeRef root,
YGNodeCleanupFunc cleanup) {
Expand Down Expand Up @@ -993,11 +952,6 @@ YG_NODE_LAYOUT_RESOLVED_PROPERTY_IMPL(float, Margin, margin);
YG_NODE_LAYOUT_RESOLVED_PROPERTY_IMPL(float, Border, border);
YG_NODE_LAYOUT_RESOLVED_PROPERTY_IMPL(float, Padding, padding);

YOGA_EXPORT bool YGNodeLayoutGetDidLegacyStretchFlagAffectLayout(
const YGNodeRef node) {
return node->getLayout().doesLegacyStretchFlagAffectsLayout();
}

std::atomic<uint32_t> gCurrentGenerationCount(0);

bool YGLayoutNodeInternal(
Expand Down Expand Up @@ -3051,9 +3005,6 @@ static void YGNodelayoutImpl(
collectedFlexItemsValues.sizeConsumedOnCurrentLine;
}

if (node->getConfig()->useLegacyStretchBehaviour) {
node->setLayoutDidUseLegacyFlag(true);
}
sizeBasedOnContent = !node->getConfig()->useLegacyStretchBehaviour;
}
}
Expand Down Expand Up @@ -4205,13 +4156,6 @@ static void YGRoundToPixelGrid(
}
}

static void unsetUseLegacyFlagRecursively(YGNodeRef node) {
node->getConfig()->useLegacyStretchBehaviour = false;
for (auto child : node->getChildren()) {
unsetUseLegacyFlagRecursively(child);
}
}

YOGA_EXPORT void YGNodeCalculateLayoutWithContext(
const YGNodeRef node,
const float ownerWidth,
Expand Down Expand Up @@ -4297,67 +4241,6 @@ YOGA_EXPORT void YGNodeCalculateLayoutWithContext(
}

Event::publish<Event::LayoutPassEnd>(node, {layoutContext, &markerData});

// We want to get rid off `useLegacyStretchBehaviour` from YGConfig. But we
// aren't sure whether client's of yoga have gotten rid off this flag or not.
// So logging this in YGLayout would help to find out the call sites depending
// on this flag. This check would be removed once we are sure no one is
// dependent on this flag anymore. The flag
// `shouldDiffLayoutWithoutLegacyStretchBehaviour` in YGConfig will help to
// run experiments.
if (node->getConfig()->shouldDiffLayoutWithoutLegacyStretchBehaviour &&
node->didUseLegacyFlag()) {
const YGNodeRef nodeWithoutLegacyFlag = YGNodeDeepClone(node);
nodeWithoutLegacyFlag->resolveDimension();
// Recursively mark nodes as dirty
nodeWithoutLegacyFlag->markDirtyAndPropogateDownwards();
gCurrentGenerationCount.fetch_add(1, std::memory_order_relaxed);
// Rerun the layout, and calculate the diff
unsetUseLegacyFlagRecursively(nodeWithoutLegacyFlag);
LayoutData layoutMarkerData = {};
if (YGLayoutNodeInternal(
nodeWithoutLegacyFlag,
width,
height,
ownerDirection,
widthMeasureMode,
heightMeasureMode,
ownerWidth,
ownerHeight,
true,
LayoutPassReason::kInitial,
nodeWithoutLegacyFlag->getConfig(),
layoutMarkerData,
layoutContext,
0, // tree root
gCurrentGenerationCount.load(std::memory_order_relaxed))) {
nodeWithoutLegacyFlag->setPosition(
nodeWithoutLegacyFlag->getLayout().direction(),
ownerWidth,
ownerHeight,
ownerWidth);
YGRoundToPixelGrid(
nodeWithoutLegacyFlag,
nodeWithoutLegacyFlag->getConfig()->pointScaleFactor,
0.0f,
0.0f);

// Set whether the two layouts are different or not.
auto neededLegacyStretchBehaviour =
!nodeWithoutLegacyFlag->isLayoutTreeEqualToNode(*node);
node->setLayoutDoesLegacyFlagAffectsLayout(neededLegacyStretchBehaviour);

#ifdef DEBUG
if (nodeWithoutLegacyFlag->getConfig()->printTree) {
YGNodePrint(
nodeWithoutLegacyFlag,
(YGPrintOptions) (YGPrintOptionsLayout | YGPrintOptionsChildren | YGPrintOptionsStyle));
}
#endif
}
YGConfigFreeRecursive(nodeWithoutLegacyFlag);
YGNodeFreeRecursive(nodeWithoutLegacyFlag);
}
}

YOGA_EXPORT void YGNodeCalculateLayout(
Expand All @@ -4381,12 +4264,6 @@ YOGA_EXPORT void YGConfigSetLogger(const YGConfigRef config, YGLogger logger) {
}
}

YOGA_EXPORT void YGConfigSetShouldDiffLayoutWithoutLegacyStretchBehaviour(
const YGConfigRef config,
const bool shouldDiffLayout) {
config->shouldDiffLayoutWithoutLegacyStretchBehaviour = shouldDiffLayout;
}

void YGAssert(const bool condition, const char* message) {
if (!condition) {
Log::log(YGNodeRef{nullptr}, YGLogLevelFatal, nullptr, "%s\n", message);
Expand Down
Loading

0 comments on commit 26580a3

Please sign in to comment.