From 5de560aee941c57d07060117fe1b09f9fdcaad2d Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Tue, 12 Sep 2023 15:31:33 -0700 Subject: [PATCH] Remove JNI Binding usage of layoutContext (#1377) Summary: X-link: https://github.com/facebook/react-native/pull/39402 Pull Request resolved: 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. Reviewed By: javache Differential Revision: D49179243 fbshipit-source-id: 2f0bd0e97d834f13b4cf65c755bbd64a878710ba --- java/jni/LayoutContext.cpp | 34 +++++++++++++++++++++++ java/jni/LayoutContext.h | 29 ++++++++++++++++++++ java/jni/YGJNIVanilla.cpp | 56 ++++++++++++++++---------------------- java/jni/YGJTypesVanilla.h | 2 ++ 4 files changed, 88 insertions(+), 33 deletions(-) create mode 100644 java/jni/LayoutContext.cpp create mode 100644 java/jni/LayoutContext.h diff --git a/java/jni/LayoutContext.cpp b/java/jni/LayoutContext.cpp new file mode 100644 index 0000000000..970812f84b --- /dev/null +++ b/java/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::Provider::Provider(PtrJNodeMapVanilla* data) { + getContexts().push(data); +} + +LayoutContext::Provider::~Provider() { + getContexts().pop(); +} + +/*static*/ PtrJNodeMapVanilla* LayoutContext::getNodeMap() { + return getContexts().empty() ? nullptr : getContexts().top(); +} + +} // namespace facebook::yoga::vanillajni diff --git a/java/jni/LayoutContext.h b/java/jni/LayoutContext.h new file mode 100644 index 0000000000..476ee82217 --- /dev/null +++ b/java/jni/LayoutContext.h @@ -0,0 +1,29 @@ +/* + * 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: + // Sets a context on the current thread for the duration of the Provider's + // lifetime. This context should be set during the layout process to allow + // layout callbacks to access context-data specific to the layout pass. + struct Provider { + explicit Provider(PtrJNodeMapVanilla* data); + ~Provider(); + }; + + static PtrJNodeMapVanilla* getNodeMap(); +}; + +} // namespace facebook::yoga::vanillajni diff --git a/java/jni/YGJNIVanilla.cpp b/java/jni/YGJNIVanilla.cpp index 86f740e1f5..747d937ba8 100644 --- a/java/jni/YGJNIVanilla.cpp +++ b/java/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::Provider contextProvider(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/java/jni/YGJTypesVanilla.h b/java/jni/YGJTypesVanilla.h index e5c44c0cf6..705e1eda6f 100644 --- a/java/jni/YGJTypesVanilla.h +++ b/java/jni/YGJTypesVanilla.h @@ -5,6 +5,8 @@ * LICENSE file in the root directory of this source tree. */ +#pragma once + #include #include