Skip to content

Commit

Permalink
Breaking: Fix callback const-correctness (#39370)
Browse files Browse the repository at this point in the history
Summary:
X-link: facebook/yoga#1369

Pull Request resolved: #39370

This fixes const-correctness of callbacks (e.g. not letting a logger function modify nodes during layout). This helps us to continue to fix const-correctness issues inside of Yoga.

This change is breaking to the public API, since it requires a change in signature passed to Yoga.

Changelog: [Internal]

Differential Revision: https://internalfb.com/D49130714

fbshipit-source-id: 6565ebc675f993a304c7bd405f38646799894c89
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Sep 12, 2023
1 parent cacb81d commit 3dbf73e
Show file tree
Hide file tree
Showing 13 changed files with 73 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
NSString *const RCTBaseTextShadowViewEmbeddedShadowViewAttributeName =
@"RCTBaseTextShadowViewEmbeddedShadowViewAttributeName";

static void RCTInlineViewYogaNodeDirtied(YGNodeRef node)
static void RCTInlineViewYogaNodeDirtied(YGNodeConstRef node)
{
// An inline view (a view nested inside of a text node) does not have a parent
// in the Yoga tree. Consequently, we have to manually propagate the inline
Expand Down
10 changes: 7 additions & 3 deletions packages/react-native/Libraries/Text/Text/RCTTextShadowView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -368,8 +368,12 @@ - (CGFloat)lastBaselineForSize:(CGSize)size
return size.height + maximumDescender;
}

static YGSize
RCTTextShadowViewMeasure(YGNodeRef node, float width, YGMeasureMode widthMode, float height, YGMeasureMode heightMode)
static YGSize RCTTextShadowViewMeasure(
YGNodeConstRef node,
float width,
YGMeasureMode widthMode,
float height,
YGMeasureMode heightMode)
{
CGSize maximumSize = (CGSize){
widthMode == YGMeasureModeUndefined ? CGFLOAT_MAX : RCTCoreGraphicsFloatFromYogaFloat(width),
Expand Down Expand Up @@ -402,7 +406,7 @@ - (CGFloat)lastBaselineForSize:(CGSize)size
RCTYogaFloatFromCoreGraphicsFloat(size.height + epsilon)};
}

static float RCTTextShadowViewBaseline(YGNodeRef node, const float width, const float height)
static float RCTTextShadowViewBaseline(YGNodeConstRef node, const float width, const float height)
{
RCTTextShadowView *shadowTextView = (__bridge RCTTextShadowView *)YGNodeGetContext(node);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ - (CGFloat)lastBaselineForSize:(CGSize)size
}

static YGSize RCTBaseTextInputShadowViewMeasure(
YGNodeRef node,
YGNodeConstRef node,
float width,
YGMeasureMode widthMode,
float height,
Expand Down Expand Up @@ -302,7 +302,7 @@ static YGSize RCTBaseTextInputShadowViewMeasure(
RCTYogaFloatFromCoreGraphicsFloat(measuredSize.width), RCTYogaFloatFromCoreGraphicsFloat(measuredSize.height)};
}

static float RCTTextInputShadowViewBaseline(YGNodeRef node, const float width, const float height)
static float RCTTextInputShadowViewBaseline(YGNodeConstRef node, const float width, const float height)
{
RCTBaseTextInputShadowView *shadowTextView = (__bridge RCTBaseTextInputShadowView *)YGNodeGetContext(node);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ - (instancetype)initWithBridge:(RCTBridge *)bridge
}

static YGSize RCTWrapperShadowViewMeasure(
YGNodeRef node,
YGNodeConstRef node,
float width,
YGMeasureMode widthMode,
float height,
Expand Down
4 changes: 2 additions & 2 deletions packages/react-native/React/Views/RCTShadowView.m
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ + (YGConfigRef)yogaConfig

// YogaNode API

static void RCTPrint(YGNodeRef node)
static void RCTPrint(YGNodeConstRef node)
{
RCTShadowView *shadowView = (__bridge RCTShadowView *)YGNodeGetContext(node);
printf("%s(%lld), ", shadowView.viewName.UTF8String, (long long)shadowView.reactTag.integerValue);
Expand Down Expand Up @@ -568,7 +568,7 @@ - (void)setSize:(CGSize)size
// IntrinsicContentSize

static inline YGSize
RCTShadowViewMeasure(YGNodeRef node, float width, YGMeasureMode widthMode, float height, YGMeasureMode heightMode)
RCTShadowViewMeasure(YGNodeConstRef node, float width, YGMeasureMode widthMode, float height, YGMeasureMode heightMode)
{
RCTShadowView *shadowView = (__bridge RCTShadowView *)YGNodeGetContext(node);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ using namespace facebook::yoga;
using namespace facebook::yoga::vanillajni;

static inline ScopedLocalRef<jobject> YGNodeJobject(
YGNodeRef node,
YGNodeConstRef node,
void* layoutContext) {
return reinterpret_cast<PtrJNodeMapVanilla*>(layoutContext)->ref(node);
}
Expand Down Expand Up @@ -138,8 +138,8 @@ static jlong jni_YGNodeNewWithConfigJNI(
}

static int YGJNILogFunc(
const YGConfigRef config,
const YGNodeRef /*node*/,
const YGConfigConstRef config,
const YGNodeConstRef /*node*/,
YGLogLevel level,
void* /*layoutContext*/,
const char* format,
Expand Down Expand Up @@ -639,7 +639,7 @@ static void jni_YGNodeStyleSetBorderJNI(
yogaNodeRef, static_cast<YGEdge>(edge), static_cast<float>(border));
}

static void YGTransferLayoutDirection(YGNodeRef node, jobject javaNode) {
static void YGTransferLayoutDirection(YGNodeConstRef node, jobject javaNode) {
// Don't change this field name without changing the name of the field in
// Database.java
JNIEnv* env = getCurrentEnv();
Expand All @@ -655,7 +655,7 @@ static void YGTransferLayoutDirection(YGNodeRef node, jobject javaNode) {
}

static YGSize YGJNIMeasureFunc(
YGNodeRef node,
YGNodeConstRef node,
float width,
YGMeasureMode widthMode,
float height,
Expand Down Expand Up @@ -700,7 +700,7 @@ static void jni_YGNodeSetHasMeasureFuncJNI(
}

static float YGJNIBaselineFunc(
YGNodeRef node,
YGNodeConstRef node,
float width,
float height,
void* layoutContext) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
namespace facebook::react {

static int FabricDefaultYogaLog(
const YGConfigRef /*unused*/,
const YGNodeRef /*unused*/,
const YGConfigConstRef /*unused*/,
const YGNodeConstRef /*unused*/,
YGLogLevel level,
const char* format,
va_list args) {
Expand Down Expand Up @@ -787,9 +787,9 @@ Rect YogaLayoutableShadowNode::getContentBounds() const {

#pragma mark - Yoga Connectors

YGNode* YogaLayoutableShadowNode::yogaNodeCloneCallbackConnector(
YGNode* /*oldYogaNode*/,
YGNode* parentYogaNode,
YGNodeRef YogaLayoutableShadowNode::yogaNodeCloneCallbackConnector(
YGNodeConstRef /*oldYogaNode*/,
YGNodeConstRef parentYogaNode,
int childIndex) {
SystraceSection s("YogaLayoutableShadowNode::yogaNodeCloneCallbackConnector");

Expand All @@ -798,7 +798,7 @@ YGNode* YogaLayoutableShadowNode::yogaNodeCloneCallbackConnector(
}

YGSize YogaLayoutableShadowNode::yogaNodeMeasureCallbackConnector(
YGNode* yogaNode,
YGNodeConstRef yogaNode,
float width,
YGMeasureMode widthMode,
float height,
Expand Down Expand Up @@ -845,7 +845,7 @@ YGSize YogaLayoutableShadowNode::yogaNodeMeasureCallbackConnector(
}

YogaLayoutableShadowNode& YogaLayoutableShadowNode::shadowNodeFromContext(
YGNodeRef yogaNode) {
YGNodeConstRef yogaNode) {
return traitCast<YogaLayoutableShadowNode&>(
*static_cast<ShadowNode*>(YGNodeGetContext(yogaNode)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,16 +159,17 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode {
yoga::Config& config,
YGConfigRef previousConfig = nullptr);
static YGNodeRef yogaNodeCloneCallbackConnector(
YGNodeRef oldYogaNode,
YGNodeRef parentYogaNode,
YGNodeConstRef oldYogaNode,
YGNodeConstRef parentYogaNode,
int childIndex);
static YGSize yogaNodeMeasureCallbackConnector(
YGNodeRef yogaNode,
YGNodeConstRef yogaNode,
float width,
YGMeasureMode widthMode,
float height,
YGMeasureMode heightMode);
static YogaLayoutableShadowNode& shadowNodeFromContext(YGNodeRef yogaNode);
static YogaLayoutableShadowNode& shadowNodeFromContext(
YGNodeConstRef yogaNode);

#pragma mark - RTL Legacy Autoflip

Expand Down
24 changes: 9 additions & 15 deletions packages/react-native/ReactCommon/yoga/yoga/Yoga.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ using namespace facebook::yoga;

#ifdef ANDROID
static int YGAndroidLog(
const YGConfigRef config,
const YGNodeRef node,
const YGConfigConstRef config,
const YGNodeConstRef node,
YGLogLevel level,
const char* format,
va_list args);
#else
static int YGDefaultLog(
const YGConfigRef config,
const YGNodeRef node,
const YGConfigConstRef config,
const YGNodeConstRef node,
YGLogLevel level,
const char* format,
va_list args);
Expand All @@ -39,8 +39,8 @@ static int YGDefaultLog(
#ifdef ANDROID
#include <android/log.h>
static int YGAndroidLog(
const YGConfigRef /*config*/,
const YGNodeRef /*node*/,
const YGConfigConstRef /*config*/,
const YGNodeConstRef /*node*/,
YGLogLevel level,
const char* format,
va_list args) {
Expand Down Expand Up @@ -69,16 +69,12 @@ static int YGAndroidLog(
return result;
}
#else
#define YG_UNUSED(x) (void) (x);

static int YGDefaultLog(
const YGConfigRef config,
const YGNodeRef node,
const YGConfigConstRef /*config*/,
const YGNodeConstRef /*node*/,
YGLogLevel level,
const char* format,
va_list args) {
YG_UNUSED(config);
YG_UNUSED(node);
switch (level) {
case YGLogLevelError:
case YGLogLevelFatal:
Expand All @@ -91,8 +87,6 @@ static int YGDefaultLog(
return vprintf(format, args);
}
}

#undef YG_UNUSED
#endif

YOGA_EXPORT bool YGFloatIsUndefined(const float value) {
Expand Down Expand Up @@ -202,7 +196,7 @@ YOGA_EXPORT YGNodeRef YGNodeNew(void) {
return YGNodeNewWithConfig(YGConfigGetDefault());
}

YOGA_EXPORT YGNodeRef YGNodeClone(YGNodeRef oldNodeRef) {
YOGA_EXPORT YGNodeRef YGNodeClone(YGNodeConstRef oldNodeRef) {
auto oldNode = resolveRef(oldNodeRef);
const auto node = new yoga::Node(*oldNode);
yoga::assertFatalWithConfig(
Expand Down
22 changes: 12 additions & 10 deletions packages/react-native/ReactCommon/yoga/yoga/Yoga.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,28 +29,30 @@ typedef struct YGNode* YGNodeRef;
typedef const struct YGNode* YGNodeConstRef;

typedef YGSize (*YGMeasureFunc)(
YGNodeRef node,
YGNodeConstRef node,
float width,
YGMeasureMode widthMode,
float height,
YGMeasureMode heightMode);
typedef float (*YGBaselineFunc)(YGNodeRef node, float width, float height);
typedef void (*YGDirtiedFunc)(YGNodeRef node);
typedef void (*YGPrintFunc)(YGNodeRef node);
typedef void (*YGNodeCleanupFunc)(YGNodeRef node);
typedef float (*YGBaselineFunc)(YGNodeConstRef node, float width, float height);
typedef void (*YGDirtiedFunc)(YGNodeConstRef node);
typedef void (*YGPrintFunc)(YGNodeConstRef node);
typedef void (*YGNodeCleanupFunc)(YGNodeConstRef node);
typedef int (*YGLogger)(
YGConfigRef config,
YGNodeRef node,
YGConfigConstRef config,
YGNodeConstRef node,
YGLogLevel level,
const char* format,
va_list args);
typedef YGNodeRef (
*YGCloneNodeFunc)(YGNodeRef oldNode, YGNodeRef owner, int childIndex);
typedef YGNodeRef (*YGCloneNodeFunc)(
YGNodeConstRef oldNode,
YGNodeConstRef owner,
int childIndex);

// YGNode
WIN_EXPORT YGNodeRef YGNodeNew(void);
WIN_EXPORT YGNodeRef YGNodeNewWithConfig(YGConfigRef config);
WIN_EXPORT YGNodeRef YGNodeClone(YGNodeRef node);
WIN_EXPORT YGNodeRef YGNodeClone(YGNodeConstRef node);
WIN_EXPORT void YGNodeFree(YGNodeRef node);
WIN_EXPORT void YGNodeFreeRecursiveWithCleanupFunc(
YGNodeRef node,
Expand Down
21 changes: 4 additions & 17 deletions packages/react-native/ReactCommon/yoga/yoga/config/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,23 +107,10 @@ void Config::log(
void* logContext,
const char* format,
va_list args) const {
// TODO: Break log callback signatures to make them const correct

if (flags_.loggerUsesContext) {
logger_.withContext(
const_cast<yoga::Config*>(this),
const_cast<yoga::Node*>(node),
logLevel,
logContext,
format,
args);
logger_.withContext(this, node, logLevel, logContext, format, args);
} else {
logger_.noContext(
const_cast<yoga::Config*>(this),
const_cast<yoga::Node*>(node),
logLevel,
format,
args);
logger_.noContext(this, node, logLevel, format, args);
}
}

Expand All @@ -142,8 +129,8 @@ void Config::setCloneNodeCallback(std::nullptr_t) {
}

YGNodeRef Config::cloneNode(
YGNodeRef node,
YGNodeRef owner,
YGNodeConstRef node,
YGNodeConstRef owner,
int childIndex,
void* cloneContext) const {
YGNodeRef clone = nullptr;
Expand Down
12 changes: 6 additions & 6 deletions packages/react-native/ReactCommon/yoga/yoga/config/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ bool configUpdateInvalidatesLayout(Config* a, Config* b);
// Internal variants of log functions, currently used only by JNI bindings.
// TODO: Reconcile this with the public API
using LogWithContextFn = int (*)(
YGConfigRef config,
YGNodeRef node,
YGConfigConstRef config,
YGNodeConstRef node,
YGLogLevel level,
void* context,
const char* format,
va_list args);
using CloneWithContextFn = YGNodeRef (*)(
YGNodeRef node,
YGNodeRef owner,
YGNodeConstRef node,
YGNodeConstRef owner,
int childIndex,
void* cloneContext);

Expand Down Expand Up @@ -90,8 +90,8 @@ class YOGA_EXPORT Config : public ::YGConfig {
void setCloneNodeCallback(CloneWithContextFn cloneNode);
void setCloneNodeCallback(std::nullptr_t);
YGNodeRef cloneNode(
YGNodeRef node,
YGNodeRef owner,
YGNodeConstRef node,
YGNodeConstRef owner,
int childIndex,
void* cloneContext) const;

Expand Down
15 changes: 11 additions & 4 deletions packages/react-native/ReactCommon/yoga/yoga/node/Node.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,17 @@ struct NodeFlags {

class YOGA_EXPORT Node : public ::YGNode {
public:
using MeasureWithContextFn =
YGSize (*)(YGNode*, float, YGMeasureMode, float, YGMeasureMode, void*);
using BaselineWithContextFn = float (*)(YGNode*, float, float, void*);
using PrintWithContextFn = void (*)(YGNode*, void*);
// Internal variants of callbacks, currently used only by JNI bindings.
// TODO: Reconcile this with the public API
using MeasureWithContextFn = YGSize (*)(
YGNodeConstRef,
float,
YGMeasureMode,
float,
YGMeasureMode,
void*);
using BaselineWithContextFn = float (*)(YGNodeConstRef, float, float, void*);
using PrintWithContextFn = void (*)(YGNodeConstRef, void*);

private:
void* context_ = nullptr;
Expand Down

0 comments on commit 3dbf73e

Please sign in to comment.