Skip to content

Commit

Permalink
Breaking: size_t indices
Browse files Browse the repository at this point in the history
Summary:
X-link: facebook/yoga#1366

X-link: facebook/react-native#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: 6a004c160c4c50f68047b108508fd437156f5fac
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Sep 12, 2023
1 parent 8af101f commit b1ef248
Show file tree
Hide file tree
Showing 12 changed files with 55 additions and 59 deletions.
30 changes: 15 additions & 15 deletions lib/yoga/src/main/cpp/yoga/Yoga.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ YOGA_EXPORT void YGNodeFree(const YGNodeRef nodeRef) {
node->setOwner(nullptr);
}

const uint32_t childCount = YGNodeGetChildCount(node);
for (uint32_t i = 0; i < childCount; i++) {
const size_t childCount = node->getChildCount();
for (size_t i = 0; i < childCount; i++) {
auto child = node->getChild(i);
child->setOwner(nullptr);
}
Expand All @@ -236,8 +236,8 @@ YOGA_EXPORT void YGNodeFreeRecursiveWithCleanupFunc(
YGNodeCleanupFunc cleanup) {
const auto root = resolveRef(rootRef);

uint32_t skipped = 0;
while (YGNodeGetChildCount(root) > skipped) {
size_t skipped = 0;
while (root->getChildCount() > skipped) {
const auto child = root->getChild(skipped);
if (child->getOwner() != root) {
// Don't free shared nodes that we don't own.
Expand Down Expand Up @@ -297,7 +297,7 @@ YOGA_EXPORT bool YGNodeIsReferenceBaseline(YGNodeConstRef node) {
YOGA_EXPORT void YGNodeInsertChild(
const YGNodeRef ownerRef,
const YGNodeRef childRef,
const uint32_t index) {
const size_t index) {
auto owner = resolveRef(ownerRef);
auto child = resolveRef(childRef);

Expand All @@ -319,7 +319,7 @@ YOGA_EXPORT void YGNodeInsertChild(
YOGA_EXPORT void YGNodeSwapChild(
const YGNodeRef ownerRef,
const YGNodeRef childRef,
const uint32_t index) {
const size_t index) {
auto owner = resolveRef(ownerRef);
auto child = resolveRef(childRef);

Expand All @@ -333,7 +333,7 @@ YOGA_EXPORT void YGNodeRemoveChild(
auto owner = resolveRef(ownerRef);
auto excludedChild = resolveRef(excludedChildRef);

if (YGNodeGetChildCount(owner) == 0) {
if (owner->getChildCount() == 0) {
// This is an empty set. Nothing to remove.
return;
}
Expand All @@ -354,7 +354,7 @@ YOGA_EXPORT void YGNodeRemoveChild(
YOGA_EXPORT void YGNodeRemoveAllChildren(const YGNodeRef ownerRef) {
auto owner = resolveRef(ownerRef);

const uint32_t childCount = YGNodeGetChildCount(owner);
const size_t childCount = owner->getChildCount();
if (childCount == 0) {
// This is an empty set already. Nothing to do.
return;
Expand All @@ -363,7 +363,7 @@ YOGA_EXPORT void YGNodeRemoveAllChildren(const YGNodeRef ownerRef) {
if (firstChild->getOwner() == owner) {
// If the first child has this node as its owner, we assume that this child
// set is unique.
for (uint32_t i = 0; i < childCount; i++) {
for (size_t i = 0; i < childCount; i++) {
yoga::Node* oldChild = owner->getChild(i);
oldChild->setLayout({}); // layout is no longer valid
oldChild->setOwner(nullptr);
Expand All @@ -381,7 +381,7 @@ YOGA_EXPORT void YGNodeRemoveAllChildren(const YGNodeRef ownerRef) {
YOGA_EXPORT void YGNodeSetChildren(
const YGNodeRef ownerRef,
const YGNodeRef* childrenRefs,
const uint32_t count) {
const size_t count) {
auto owner = resolveRef(ownerRef);
auto children = reinterpret_cast<yoga::Node* const*>(childrenRefs);

Expand All @@ -391,7 +391,7 @@ YOGA_EXPORT void YGNodeSetChildren(

const std::vector<yoga::Node*> childrenVector = {children, children + count};
if (childrenVector.size() == 0) {
if (YGNodeGetChildCount(owner) > 0) {
if (owner->getChildCount() > 0) {
for (auto* child : owner->getChildren()) {
child->setLayout({});
child->setOwner(nullptr);
Expand All @@ -400,7 +400,7 @@ YOGA_EXPORT void YGNodeSetChildren(
owner->markDirtyAndPropagate();
}
} else {
if (YGNodeGetChildCount(owner) > 0) {
if (owner->getChildCount() > 0) {
for (auto* oldChild : owner->getChildren()) {
// Our new children may have nodes in common with the old children. We
// don't reset these common nodes.
Expand All @@ -420,7 +420,7 @@ YOGA_EXPORT void YGNodeSetChildren(
}

YOGA_EXPORT YGNodeRef
YGNodeGetChild(const YGNodeRef nodeRef, const uint32_t index) {
YGNodeGetChild(const YGNodeRef nodeRef, const size_t index) {
const auto node = resolveRef(nodeRef);

if (index < node->getChildren().size()) {
Expand All @@ -429,8 +429,8 @@ YGNodeGetChild(const YGNodeRef nodeRef, const uint32_t index) {
return nullptr;
}

YOGA_EXPORT uint32_t YGNodeGetChildCount(const YGNodeConstRef node) {
return static_cast<uint32_t>(resolveRef(node)->getChildren().size());
YOGA_EXPORT size_t YGNodeGetChildCount(const YGNodeConstRef node) {
return resolveRef(node)->getChildren().size();
}

YOGA_EXPORT YGNodeRef YGNodeGetOwner(const YGNodeRef node) {
Expand Down
16 changes: 7 additions & 9 deletions lib/yoga/src/main/cpp/yoga/Yoga.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <stdarg.h>
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>

#include <yoga/YGEnums.h>
Expand Down Expand Up @@ -47,7 +48,7 @@ typedef int (*YGLogger)(
typedef YGNodeRef (*YGCloneNodeFunc)(
YGNodeConstRef oldNode,
YGNodeConstRef owner,
int childIndex);
size_t childIndex);

// YGNode
WIN_EXPORT YGNodeRef YGNodeNew(void);
Expand All @@ -63,23 +64,20 @@ WIN_EXPORT void YGNodeReset(YGNodeRef node);
WIN_EXPORT void YGNodeInsertChild(
YGNodeRef node,
YGNodeRef child,
uint32_t index);
size_t index);

WIN_EXPORT void YGNodeSwapChild(
YGNodeRef node,
YGNodeRef child,
uint32_t index);
WIN_EXPORT void YGNodeSwapChild(YGNodeRef node, YGNodeRef child, size_t index);

WIN_EXPORT void YGNodeRemoveChild(YGNodeRef node, YGNodeRef child);
WIN_EXPORT void YGNodeRemoveAllChildren(YGNodeRef node);
WIN_EXPORT YGNodeRef YGNodeGetChild(YGNodeRef node, uint32_t index);
WIN_EXPORT YGNodeRef YGNodeGetChild(YGNodeRef node, size_t index);
WIN_EXPORT YGNodeRef YGNodeGetOwner(YGNodeRef node);
WIN_EXPORT YGNodeRef YGNodeGetParent(YGNodeRef node);
WIN_EXPORT uint32_t YGNodeGetChildCount(YGNodeConstRef node);
WIN_EXPORT size_t YGNodeGetChildCount(YGNodeConstRef node);
WIN_EXPORT void YGNodeSetChildren(
YGNodeRef owner,
const YGNodeRef* children,
uint32_t count);
size_t count);

WIN_EXPORT void YGNodeSetIsReferenceBaseline(
YGNodeRef node,
Expand Down
4 changes: 2 additions & 2 deletions lib/yoga/src/main/cpp/yoga/algorithm/Baseline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ float calculateBaseline(const yoga::Node* node, void* layoutContext) {
}

yoga::Node* baselineChild = nullptr;
const uint32_t childCount = YGNodeGetChildCount(node);
for (uint32_t i = 0; i < childCount; i++) {
const size_t childCount = node->getChildCount();
for (size_t i = 0; i < childCount; i++) {
auto child = node->getChild(i);
if (child->getLineIndex() > 0) {
break;
Expand Down
36 changes: 17 additions & 19 deletions lib/yoga/src/main/cpp/yoga/algorithm/CalculateLayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -965,8 +965,8 @@ static CollectFlexItemsRowValues calculateCollectFlexItemsRowValues(
const float mainAxisownerSize,
const float availableInnerWidth,
const float availableInnerMainDim,
const uint32_t startOfLineIndex,
const uint32_t lineCount) {
const size_t startOfLineIndex,
const size_t lineCount) {
CollectFlexItemsRowValues flexAlgoRowMeasurement = {};
flexAlgoRowMeasurement.relativeChildren.reserve(node->getChildren().size());

Expand All @@ -977,7 +977,7 @@ static CollectFlexItemsRowValues calculateCollectFlexItemsRowValues(
const float gap = node->getGapForAxis(mainAxis, availableInnerWidth).unwrap();

// Add items to the current line until it's full or we run out of items.
uint32_t endOfLineIndex = startOfLineIndex;
size_t endOfLineIndex = startOfLineIndex;
for (; endOfLineIndex < node->getChildren().size(); endOfLineIndex++) {
auto child = node->getChild(endOfLineIndex);
if (child->getStyle().display() == YGDisplayNone ||
Expand Down Expand Up @@ -1408,7 +1408,7 @@ static void resolveFlexibleLength(
static void YGJustifyMainAxis(
yoga::Node* const node,
CollectFlexItemsRowValues& collectedFlexItemsValues,
const uint32_t startOfLineIndex,
const size_t startOfLineIndex,
const YGFlexDirection mainAxis,
const YGFlexDirection crossAxis,
const YGMeasureMode measureModeMainDim,
Expand Down Expand Up @@ -1456,8 +1456,7 @@ static void YGJustifyMainAxis(
}

int numberOfAutoMarginsOnCurrentLine = 0;
for (uint32_t i = startOfLineIndex;
i < collectedFlexItemsValues.endOfLineIndex;
for (size_t i = startOfLineIndex; i < collectedFlexItemsValues.endOfLineIndex;
i++) {
auto child = node->getChild(i);
if (child->getStyle().positionType() != YGPositionTypeAbsolute) {
Expand Down Expand Up @@ -1517,8 +1516,7 @@ static void YGJustifyMainAxis(
float maxAscentForCurrentLine = 0;
float maxDescentForCurrentLine = 0;
bool isNodeBaselineLayout = isBaselineLayout(node);
for (uint32_t i = startOfLineIndex;
i < collectedFlexItemsValues.endOfLineIndex;
for (size_t i = startOfLineIndex; i < collectedFlexItemsValues.endOfLineIndex;
i++) {
const auto child = node->getChild(i);
const Style& childStyle = child->getStyle();
Expand Down Expand Up @@ -1906,11 +1904,11 @@ static void calculateLayoutImpl(
// STEP 4: COLLECT FLEX ITEMS INTO FLEX LINES

// Indexes of children that represent the first and last items in the line.
uint32_t startOfLineIndex = 0;
uint32_t endOfLineIndex = 0;
size_t startOfLineIndex = 0;
size_t endOfLineIndex = 0;

// Number of lines.
uint32_t lineCount = 0;
size_t lineCount = 0;

// Accumulated cross dimensions of all lines so far.
float totalLineCrossDim = 0;
Expand Down Expand Up @@ -2093,7 +2091,7 @@ static void calculateLayoutImpl(
// STEP 7: CROSS-AXIS ALIGNMENT
// We can skip child alignment if we're just measuring the container.
if (performLayout) {
for (uint32_t i = startOfLineIndex; i < endOfLineIndex; i++) {
for (size_t i = startOfLineIndex; i < endOfLineIndex; i++) {
const auto child = node->getChild(i);
if (child->getStyle().display() == YGDisplayNone) {
continue;
Expand Down Expand Up @@ -2291,10 +2289,10 @@ static void calculateLayoutImpl(
break;
}
}
uint32_t endIndex = 0;
for (uint32_t i = 0; i < lineCount; i++) {
const uint32_t startIndex = endIndex;
uint32_t ii;
size_t endIndex = 0;
for (size_t i = 0; i < lineCount; i++) {
const size_t startIndex = endIndex;
size_t ii;

// compute the line's height and find the endIndex
float lineHeight = 0;
Expand Down Expand Up @@ -2537,7 +2535,7 @@ static void calculateLayoutImpl(
// As we only wrapped in normal direction yet, we need to reverse the
// positions on wrap-reverse.
if (performLayout && node->getStyle().flexWrap() == YGWrapWrapReverse) {
for (uint32_t i = 0; i < childCount; i++) {
for (size_t i = 0; i < childCount; i++) {
const auto child = node->getChild(i);
if (child->getStyle().positionType() != YGPositionTypeAbsolute) {
child->setLayoutPosition(
Expand Down Expand Up @@ -2586,7 +2584,7 @@ static void calculateLayoutImpl(

// Set trailing position if necessary.
if (needsMainTrailingPos || needsCrossTrailingPos) {
for (uint32_t i = 0; i < childCount; i++) {
for (size_t i = 0; i < childCount; i++) {
const auto child = node->getChild(i);
if (child->getStyle().display() == YGDisplayNone) {
continue;
Expand Down Expand Up @@ -2710,7 +2708,7 @@ bool calculateLayoutInternal(
cachedResults = &layout->cachedLayout;
} else {
// Try to use the measurement cache.
for (uint32_t i = 0; i < layout->nextCachedMeasurementsIndex; i++) {
for (size_t i = 0; i < layout->nextCachedMeasurementsIndex; i++) {
if (canUseCachedMeasurement(
widthMeasureMode,
availableWidth,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ namespace facebook::yoga {
// and/or grow.

struct CollectFlexItemsRowValues {
uint32_t itemsOnLine;
size_t itemsOnLine;
float sizeConsumedOnCurrentLine;
float totalFlexGrowFactors;
float totalFlexShrinkScaledFactors;
uint32_t endOfLineIndex;
size_t endOfLineIndex;
std::vector<yoga::Node*> relativeChildren;
float remainingFreeSpace;
// The size of the mainDim for the row after considering size, padding, margin
Expand Down
4 changes: 2 additions & 2 deletions lib/yoga/src/main/cpp/yoga/algorithm/PixelGrid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ void roundLayoutResultsToPixelGrid(
absoluteNodeTop, pointScaleFactor, false, textRounding),
YGDimensionHeight);

const uint32_t childCount = YGNodeGetChildCount(node);
for (uint32_t i = 0; i < childCount; i++) {
const size_t childCount = node->getChildCount();
for (size_t i = 0; i < childCount; i++) {
roundLayoutResultsToPixelGrid(
node->getChild(i), pointScaleFactor, absoluteNodeLeft, absoluteNodeTop);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/yoga/src/main/cpp/yoga/config/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ void Config::setCloneNodeCallback(std::nullptr_t) {
YGNodeRef Config::cloneNode(
YGNodeConstRef node,
YGNodeConstRef owner,
int childIndex,
size_t childIndex,
void* cloneContext) const {
YGNodeRef clone = nullptr;
if (cloneNodeCallback_.noContext != nullptr) {
Expand Down
4 changes: 2 additions & 2 deletions lib/yoga/src/main/cpp/yoga/config/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ using LogWithContextFn = int (*)(
using CloneWithContextFn = YGNodeRef (*)(
YGNodeConstRef node,
YGNodeConstRef owner,
int childIndex,
size_t childIndex,
void* cloneContext);

#pragma pack(push)
Expand Down Expand Up @@ -92,7 +92,7 @@ class YOGA_EXPORT Config : public ::YGConfig {
YGNodeRef cloneNode(
YGNodeConstRef node,
YGNodeConstRef owner,
int childIndex,
size_t childIndex,
void* cloneContext) const;

private:
Expand Down
4 changes: 2 additions & 2 deletions lib/yoga/src/main/cpp/yoga/debug/NodeToString.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,9 @@ void nodeToString(
}
appendFormattedString(str, ">");

const uint32_t childCount = static_cast<uint32_t>(node->getChildren().size());
const size_t childCount = node->getChildCount();
if (options & YGPrintOptionsChildren && childCount > 0) {
for (uint32_t i = 0; i < childCount; i++) {
for (size_t i = 0; i < childCount; i++) {
appendFormattedString(str, "\n");
nodeToString(str, node->getChild(i), options, level + 1);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/yoga/src/main/cpp/yoga/node/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ YOGA_EXPORT void Node::clearChildren() {
// Other Methods

void Node::cloneChildrenIfNeeded(void* cloneContext) {
int i = 0;
size_t i = 0;
for (Node*& child : children_) {
if (child->getOwner() != this) {
child = resolveRef(config_->cloneNode(child, this, i, cloneContext));
Expand Down
6 changes: 3 additions & 3 deletions lib/yoga/src/main/cpp/yoga/node/Node.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class YOGA_EXPORT Node : public ::YGNode {
YGDirtiedFunc dirtied_ = nullptr;
Style style_ = {};
LayoutResults layout_ = {};
uint32_t lineIndex_ = 0;
size_t lineIndex_ = 0;
Node* owner_ = nullptr;
std::vector<Node*> children_ = {};
Config* config_;
Expand Down Expand Up @@ -146,7 +146,7 @@ class YOGA_EXPORT Node : public ::YGNode {

const LayoutResults& getLayout() const { return layout_; }

uint32_t getLineIndex() const { return lineIndex_; }
size_t getLineIndex() const { return lineIndex_; }

bool isReferenceBaseline() const { return flags_.isReferenceBaseline; }

Expand Down Expand Up @@ -276,7 +276,7 @@ class YOGA_EXPORT Node : public ::YGNode {

void setLayout(const LayoutResults& layout) { layout_ = layout; }

void setLineIndex(uint32_t lineIndex) { lineIndex_ = lineIndex; }
void setLineIndex(size_t lineIndex) { lineIndex_ = lineIndex; }

void setIsReferenceBaseline(bool isReferenceBaseline) {
flags_.isReferenceBaseline = isReferenceBaseline;
Expand Down
2 changes: 1 addition & 1 deletion lib/yogajni/src/main/cpp/jni/YGJNIVanilla.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ static void YGTransferLayoutOutputsRecursive(

YGNodeSetHasNewLayout(root, false);

for (uint32_t i = 0; i < YGNodeGetChildCount(root); i++) {
for (size_t i = 0; i < YGNodeGetChildCount(root); i++) {
YGTransferLayoutOutputsRecursive(
env, thiz, YGNodeGetChild(root, i), layoutContext);
}
Expand Down

0 comments on commit b1ef248

Please sign in to comment.