Skip to content

Commit

Permalink
Simplify bitfields (facebook#1393)
Browse files Browse the repository at this point in the history
Summary:
X-link: facebook/react-native#39485

Pull Request resolved: facebook#1393

These were previously packed into structs to allow zero initializing all the flags at once without needing a ctor. C++ 20 has in-class member initializer support for bitfields, which makes these look more like normal member variables.

Setting enum values is a bit jank right now, due to relying on C enums which are efftively int32_t, along with GCC `-Wconversion` being a bit aggressive in needing to explicitly mask. I have some ideas to fix this later (e.g. using scoped enums internally).

Differential Revision: D49265967

fbshipit-source-id: 0a5c7a24dc0eade1ca037f0c8660a9e442da198f
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Sep 18, 2023
1 parent aa6c49f commit 663757b
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 51 deletions.
8 changes: 4 additions & 4 deletions yoga/config/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,19 @@ Config::Config(YGLogger logger) : cloneNodeCallback_{nullptr} {
}

void Config::setUseWebDefaults(bool useWebDefaults) {
flags_.useWebDefaults = useWebDefaults;
useWebDefaults_ = useWebDefaults;
}

bool Config::useWebDefaults() const {
return flags_.useWebDefaults;
return useWebDefaults_;
}

void Config::setShouldPrintTree(bool printTree) {
flags_.printTree = printTree;
printTree_ = printTree;
}

bool Config::shouldPrintTree() const {
return flags_.printTree;
return printTree_;
}

void Config::setExperimentalFeatureEnabled(
Expand Down
13 changes: 3 additions & 10 deletions yoga/config/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,6 @@ bool configUpdateInvalidatesLayout(
const Config& oldConfig,
const Config& newConfig);

#pragma pack(push)
#pragma pack(1)
// Packed structure of <32-bit options to miminize size per node.
struct ConfigFlags {
bool useWebDefaults : 1;
bool printTree : 1;
};
#pragma pack(pop)

class YG_EXPORT Config : public ::YGConfig {
public:
Config(YGLogger logger);
Expand Down Expand Up @@ -82,7 +73,9 @@ class YG_EXPORT Config : public ::YGConfig {
YGCloneNodeFunc cloneNodeCallback_;
YGLogger logger_;

ConfigFlags flags_{};
bool useWebDefaults_ : 1 = false;
bool printTree_ : 1 = false;

ExperimentalFeatureSet experimentalFeatures_{};
Errata errata_ = Errata::None;
float pointScaleFactor_ = 1.0f;
Expand Down
19 changes: 6 additions & 13 deletions yoga/node/LayoutResults.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,6 @@

namespace facebook::yoga {

#pragma pack(push)
#pragma pack(1)
struct LayoutResultFlags {
uint32_t direction : 2;
bool hadOverflow : 1;
};
#pragma pack(pop)

struct LayoutResults {
// This value was chosen based on empirical data:
// 98% of analyzed layouts require less than 8 entries.
Expand All @@ -35,7 +27,8 @@ struct LayoutResults {
std::array<float, 4> padding = {};

private:
LayoutResultFlags flags_{};
uint32_t direction_ : 2 = static_cast<uint32_t>(YGDirectionInherit) & 0x03;
bool hadOverflow_ : 1 = false;

public:
uint32_t computedFlexBasisGeneration = 0;
Expand All @@ -53,18 +46,18 @@ struct LayoutResults {
CachedMeasurement cachedLayout{};

YGDirection direction() const {
return static_cast<YGDirection>(flags_.direction);
return static_cast<YGDirection>(direction_);
}

void setDirection(YGDirection direction) {
flags_.direction = static_cast<uint32_t>(direction) & 0x03;
direction_ = static_cast<uint32_t>(direction) & 0x03;
}

bool hadOverflow() const {
return flags_.hadOverflow;
return hadOverflow_;
}
void setHadOverflow(bool hadOverflow) {
flags_.hadOverflow = hadOverflow;
hadOverflow_ = hadOverflow;
}

bool operator==(LayoutResults layout) const;
Expand Down
14 changes: 8 additions & 6 deletions yoga/node/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,17 @@ Node::Node(const yoga::Config* config) : config_{config} {
yoga::assertFatal(
config != nullptr, "Attempting to construct Node with null config");

flags_.hasNewLayout = true;
if (config->useWebDefaults()) {
useWebDefaults();
}
}

Node::Node(Node&& node) {
hasNewLayout_ = node.hasNewLayout_;
isReferenceBaseline_ = node.isReferenceBaseline_;
isDirty_ = node.isDirty_;
nodeType_ = node.nodeType_;
context_ = node.context_;
flags_ = node.flags_;
measureFunc_ = node.measureFunc_;
baselineFunc_ = node.baselineFunc_;
printFunc_ = node.printFunc_;
Expand Down Expand Up @@ -271,10 +273,10 @@ void Node::setConfig(yoga::Config* config) {
}

void Node::setDirty(bool isDirty) {
if (isDirty == flags_.isDirty) {
if (isDirty == isDirty_) {
return;
}
flags_.isDirty = isDirty;
isDirty_ = isDirty;
if (isDirty && dirtiedFunc_) {
dirtiedFunc_(this);
}
Expand Down Expand Up @@ -473,7 +475,7 @@ void Node::cloneChildrenIfNeeded() {
}

void Node::markDirtyAndPropagate() {
if (!flags_.isDirty) {
if (!isDirty_) {
setDirty(true);
setLayoutComputedFlexBasis(FloatOptional());
if (owner_) {
Expand All @@ -483,7 +485,7 @@ void Node::markDirtyAndPropagate() {
}

void Node::markDirtyAndPropagateDownwards() {
flags_.isDirty = true;
isDirty_ = true;
for_each(children_.begin(), children_.end(), [](Node* childNode) {
childNode->markDirtyAndPropagateDownwards();
});
Expand Down
29 changes: 11 additions & 18 deletions yoga/node/Node.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,13 @@ struct YGNode {};

namespace facebook::yoga {

#pragma pack(push)
#pragma pack(1)
struct NodeFlags {
bool hasNewLayout : 1;
bool isReferenceBaseline : 1;
bool isDirty : 1;
NodeType nodeType : bitCount<NodeType>();
};
#pragma pack(pop)

class YG_EXPORT Node : public ::YGNode {
private:
bool hasNewLayout_ : 1 = true;
bool isReferenceBaseline_ : 1 = false;
bool isDirty_ : 1 = false;
NodeType nodeType_ : bitCount<NodeType>() = NodeType::Default;
void* context_ = nullptr;
NodeFlags flags_ = {};
YGMeasureFunc measureFunc_ = {nullptr};
YGBaselineFunc baselineFunc_ = {nullptr};
YGPrintFunc printFunc_ = {nullptr};
Expand Down Expand Up @@ -92,11 +85,11 @@ class YG_EXPORT Node : public ::YGNode {
void print();

bool getHasNewLayout() const {
return flags_.hasNewLayout;
return hasNewLayout_;
}

NodeType getNodeType() const {
return flags_.nodeType;
return nodeType_;
}

bool hasMeasureFunc() const noexcept {
Expand Down Expand Up @@ -142,7 +135,7 @@ class YG_EXPORT Node : public ::YGNode {
}

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

// returns the Node that owns this Node. An owner is used to identify
Expand Down Expand Up @@ -175,7 +168,7 @@ class YG_EXPORT Node : public ::YGNode {
}

bool isDirty() const {
return flags_.isDirty;
return isDirty_;
}

std::array<YGValue, 2> getResolvedDimensions() const {
Expand Down Expand Up @@ -250,11 +243,11 @@ class YG_EXPORT Node : public ::YGNode {
}

void setHasNewLayout(bool hasNewLayout) {
flags_.hasNewLayout = hasNewLayout;
hasNewLayout_ = hasNewLayout;
}

void setNodeType(NodeType nodeType) {
flags_.nodeType = nodeType;
nodeType_ = nodeType;
}

void setMeasureFunc(YGMeasureFunc measureFunc);
Expand All @@ -280,7 +273,7 @@ class YG_EXPORT Node : public ::YGNode {
}

void setIsReferenceBaseline(bool isReferenceBaseline) {
flags_.isReferenceBaseline = isReferenceBaseline;
isReferenceBaseline_ = isReferenceBaseline;
}

void setOwner(Node* owner) {
Expand Down

0 comments on commit 663757b

Please sign in to comment.