From 8d86e23c76a3ff11f1409c8fb60361dd63b67ac9 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Tue, 12 Sep 2023 02:30:44 -0700 Subject: [PATCH] Remove JNI Binding usage of layoutContext (#39402) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/39402 X-link: https://github.com/facebook/yoga/pull/1377 To avoid keeping a per-node mapping on native Yoga nodes to Java nodes, a per-layout context was added, to be able to pass information from the start of the layout, to measure functions, log functions, etc. The way this was done was super invasive, and added quite a few private APIs used only by the JNI functions. This change removes the context-using functions from the JNI bindings in favor of it managing its own context. Next diff removes all the cruft. Differential Revision: D49179243 fbshipit-source-id: 0bd0ac754bb47604963eea83644663dc5f99fc75 --- .../first-party/yogajni/jni/LayoutContext.cpp | 34 +++++++++++ .../first-party/yogajni/jni/LayoutContext.h | 26 +++++++++ .../first-party/yogajni/jni/YGJNIVanilla.cpp | 56 ++++++++----------- .../first-party/yogajni/jni/YGJTypesVanilla.h | 2 + 4 files changed, 85 insertions(+), 33 deletions(-) create mode 100644 packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/LayoutContext.cpp create mode 100644 packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/LayoutContext.h diff --git a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/LayoutContext.cpp b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/LayoutContext.cpp new file mode 100644 index 00000000000000..e3e42bcfa50de1 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/LayoutContext.cpp @@ -0,0 +1,34 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include + +#include "LayoutContext.h" + +namespace facebook::yoga::vanillajni { + +namespace { +std::stack& getContexts() { + static thread_local std::stack contexts; + return contexts; +} + +} // namespace + +LayoutContext::Holder::Holder(PtrJNodeMapVanilla* data) { + getContexts().push(data); +} + +LayoutContext::Holder::~Holder() { + getContexts().pop(); +} + +/*static*/ PtrJNodeMapVanilla* LayoutContext::getNodeMap() { + return getContexts().empty() ? nullptr : getContexts().top(); +} + +} // namespace facebook::yoga::vanillajni diff --git a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/LayoutContext.h b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/LayoutContext.h new file mode 100644 index 00000000000000..eda2a82135ff10 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/LayoutContext.h @@ -0,0 +1,26 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include "YGJTypesVanilla.h" + +namespace facebook::yoga::vanillajni { + +// TODO: This should not be exported or used outside of the JNI bindings +class YG_EXPORT LayoutContext { +public: + struct Holder { + explicit Holder(PtrJNodeMapVanilla* data); + ~Holder(); + }; + + static PtrJNodeMapVanilla* getNodeMap(); +}; + +} // namespace facebook::yoga::vanillajni diff --git a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNIVanilla.cpp b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNIVanilla.cpp index 86f740e1f579f9..4387eea4ed6a5c 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNIVanilla.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNIVanilla.cpp @@ -14,22 +14,17 @@ #include #include #include "YogaJniException.h" +#include "LayoutContext.h" #include #include -// TODO: Reconcile missing layoutContext functionality from callbacks in the C -// API and use that -#include - using namespace facebook; using namespace facebook::yoga; using namespace facebook::yoga::vanillajni; -static inline ScopedLocalRef YGNodeJobject( - YGNodeConstRef node, - void* layoutContext) { - return reinterpret_cast(layoutContext)->ref(node); +static inline ScopedLocalRef YGNodeJobject(YGNodeConstRef node) { + return LayoutContext::getNodeMap()->ref(node); } static inline YGNodeRef _jlong2YGNodeRef(jlong addr) { @@ -129,7 +124,6 @@ static int YGJNILogFunc( const YGConfigConstRef config, const YGNodeConstRef /*node*/, YGLogLevel level, - void* /*layoutContext*/, const char* format, va_list args) { va_list argsCopy; @@ -187,7 +181,7 @@ static void jni_YGConfigSetLoggerJNI( } *context = newGlobalRef(env, logger); - static_cast(config)->setLogger(YGJNILogFunc); + YGConfigSetLogger(config, YGJNILogFunc); } else { if (context != nullptr) { delete context; @@ -278,12 +272,11 @@ static void jni_YGNodeRemoveChildJNI( static void YGTransferLayoutOutputsRecursive( JNIEnv* env, jobject thiz, - YGNodeRef root, - void* layoutContext) { + YGNodeRef root) { if (!YGNodeGetHasNewLayout(root)) { return; } - auto obj = YGNodeJobject(root, layoutContext); + auto obj = YGNodeJobject(root); if (!obj) { return; } @@ -351,8 +344,7 @@ static void YGTransferLayoutOutputsRecursive( YGNodeSetHasNewLayout(root, false); for (size_t i = 0; i < YGNodeGetChildCount(root); i++) { - YGTransferLayoutOutputsRecursive( - env, thiz, YGNodeGetChild(root, i), layoutContext); + YGTransferLayoutOutputsRecursive(env, thiz, YGNodeGetChild(root, i)); } } @@ -366,21 +358,22 @@ static void jni_YGNodeCalculateLayoutJNI( jobjectArray javaNodes) { try { - void* layoutContext = nullptr; + PtrJNodeMapVanilla* layoutContext = nullptr; auto map = PtrJNodeMapVanilla{}; if (nativePointers) { map = PtrJNodeMapVanilla{nativePointers, javaNodes}; layoutContext = ↦ } + LayoutContext::Holder holder(layoutContext); + const YGNodeRef root = _jlong2YGNodeRef(nativePointer); - YGNodeCalculateLayoutWithContext( + YGNodeCalculateLayout( root, static_cast(width), static_cast(height), - YGNodeStyleGetDirection(_jlong2YGNodeRef(nativePointer)), - layoutContext); - YGTransferLayoutOutputsRecursive(env, obj, root, layoutContext); + YGNodeStyleGetDirection(_jlong2YGNodeRef(nativePointer))); + YGTransferLayoutOutputsRecursive(env, obj, root); } catch (const YogaJniException& jniException) { ScopedLocalRef throwable = jniException.getThrowable(); if (throwable.get()) { @@ -647,9 +640,8 @@ static YGSize YGJNIMeasureFunc( float width, YGMeasureMode widthMode, float height, - YGMeasureMode heightMode, - void* layoutContext) { - if (auto obj = YGNodeJobject(node, layoutContext)) { + YGMeasureMode heightMode) { + if (auto obj = YGNodeJobject(node)) { YGTransferLayoutDirection(node, obj.get()); JNIEnv* env = getCurrentEnv(); auto objectClass = facebook::yoga::vanillajni::make_local_ref( @@ -683,16 +675,13 @@ static void jni_YGNodeSetHasMeasureFuncJNI( jobject /*obj*/, jlong nativePointer, jboolean hasMeasureFunc) { - static_cast(_jlong2YGNodeRef(nativePointer)) - ->setMeasureFunc(hasMeasureFunc ? YGJNIMeasureFunc : nullptr); + YGNodeSetMeasureFunc( + _jlong2YGNodeRef(nativePointer), + hasMeasureFunc ? YGJNIMeasureFunc : nullptr); } -static float YGJNIBaselineFunc( - YGNodeConstRef node, - float width, - float height, - void* layoutContext) { - if (auto obj = YGNodeJobject(node, layoutContext)) { +static float YGJNIBaselineFunc(YGNodeConstRef node, float width, float height) { + if (auto obj = YGNodeJobject(node)) { JNIEnv* env = getCurrentEnv(); auto objectClass = facebook::yoga::vanillajni::make_local_ref( env, env->GetObjectClass(obj.get())); @@ -710,8 +699,9 @@ static void jni_YGNodeSetHasBaselineFuncJNI( jobject /*obj*/, jlong nativePointer, jboolean hasBaselineFunc) { - static_cast(_jlong2YGNodeRef(nativePointer)) - ->setBaselineFunc(hasBaselineFunc ? YGJNIBaselineFunc : nullptr); + YGNodeSetBaselineFunc( + _jlong2YGNodeRef(nativePointer), + hasBaselineFunc ? YGJNIBaselineFunc : nullptr); } static void jni_YGNodePrintJNI( diff --git a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJTypesVanilla.h b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJTypesVanilla.h index e5c44c0cf6480e..705e1eda6fa960 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJTypesVanilla.h +++ b/packages/react-native/ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJTypesVanilla.h @@ -5,6 +5,8 @@ * LICENSE file in the root directory of this source tree. */ +#pragma once + #include #include