From 299c9849645c24c8ceb802893e5cdddd25413f1b Mon Sep 17 00:00:00 2001 From: Oleksandr Melnykov Date: Thu, 3 Oct 2019 03:12:46 -0700 Subject: [PATCH] Pass surface ID to measure function in Java to retrieve themed Context Summary: We use `ViewManager.onMeasure` to perform measurements of Android views and pass the measured size back to Yoga. For Android in order to the report correct dimensions of a View, this View must be created using a Context that has a theme associated with it. Before, `onMeasure` only had ReactApplicationContext passed as the first parameter and ReactSwitch, for example, could not be measured correctly (because it uses the size of the thumb drawable, which is extracted from the current theme). This diff adds surfaceId as the first parameter of `FabricUIManager.measure`, so that we can retrieve ThemedReactContext and pass it to `ViewManager.onMeasure`. The size of the Switch component is still incorrect, but at least the size reported back to Yoga is the same as in Paper. So there is more investigation necessary why this happens in Fabric. I will investigate and publish another diff with the fix. Reviewed By: JoshuaGross, shergin Differential Revision: D17625959 fbshipit-source-id: 48197a61240fb13042bef3e9f5d681acacc702fb --- .../react/fabric/FabricUIManager.java | 23 +++++++++++++++++++ .../components/slider/SliderShadowNode.cpp | 2 +- .../android/SliderMeasurementsManager.cpp | 3 +++ .../android/SliderMeasurementsManager.h | 2 +- .../ios/SliderMeasurementsManager.cpp | 1 + .../platform/ios/SliderMeasurementsManager.h | 2 +- .../AndroidSwitchMeasurementsManager.cpp | 3 +++ .../AndroidSwitchMeasurementsManager.h | 2 +- .../androidswitch/AndroidSwitchShadowNode.cpp | 2 +- .../platform/ios/TextLayoutManager.mm | 12 ++++++---- 10 files changed, 42 insertions(+), 10 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index a3b69c3584c..5ea7fa5b7fa 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -333,6 +333,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { @DoNotStrip @SuppressWarnings("unused") private long measure( + int rootTag, String componentName, ReadableMap localData, ReadableMap props, @@ -341,7 +342,29 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { float maxWidth, float minHeight, float maxHeight) { + return mMountingManager.measure( + mReactContextForRootTag.get(rootTag), + componentName, + localData, + props, + state, + getYogaSize(minWidth, maxWidth), + getYogaMeasureMode(minWidth, maxWidth), + getYogaSize(minHeight, maxHeight), + getYogaMeasureMode(minHeight, maxHeight)); + } + @DoNotStrip + @SuppressWarnings("unused") + private long measure( + String componentName, + ReadableMap localData, + ReadableMap props, + ReadableMap state, + float minWidth, + float maxWidth, + float minHeight, + float maxHeight) { return mMountingManager.measure( mReactApplicationContext, componentName, diff --git a/ReactCommon/fabric/components/slider/SliderShadowNode.cpp b/ReactCommon/fabric/components/slider/SliderShadowNode.cpp index 6557295c2dc..39dd570865c 100644 --- a/ReactCommon/fabric/components/slider/SliderShadowNode.cpp +++ b/ReactCommon/fabric/components/slider/SliderShadowNode.cpp @@ -93,7 +93,7 @@ ImageSource SliderShadowNode::getThumbImageSource() const { Size SliderShadowNode::measure(LayoutConstraints layoutConstraints) const { if (SliderMeasurementsManager::shouldMeasureSlider()) { - return measurementsManager_->measure(layoutConstraints); + return measurementsManager_->measure(getSurfaceId(), layoutConstraints); } return {}; diff --git a/ReactCommon/fabric/components/slider/platform/android/SliderMeasurementsManager.cpp b/ReactCommon/fabric/components/slider/platform/android/SliderMeasurementsManager.cpp index c44288caefc..da8a5280219 100644 --- a/ReactCommon/fabric/components/slider/platform/android/SliderMeasurementsManager.cpp +++ b/ReactCommon/fabric/components/slider/platform/android/SliderMeasurementsManager.cpp @@ -17,6 +17,7 @@ namespace facebook { namespace react { Size SliderMeasurementsManager::measure( + SurfaceId surfaceId, LayoutConstraints layoutConstraints) const { { std::lock_guard lock(mutex_); @@ -31,6 +32,7 @@ Size SliderMeasurementsManager::measure( static auto measure = jni::findClassStatic("com/facebook/react/fabric/FabricUIManager") ->getMethod lock(mutex_); @@ -31,6 +32,7 @@ Size AndroidSwitchMeasurementsManager::measure( static auto measure = jni::findClassStatic("com/facebook/react/fabric/FabricUIManager") ->getMethodmeasure(layoutConstraints); + return measurementsManager_->measure(getSurfaceId(), layoutConstraints); } } // namespace react diff --git a/ReactCommon/fabric/textlayoutmanager/platform/ios/TextLayoutManager.mm b/ReactCommon/fabric/textlayoutmanager/platform/ios/TextLayoutManager.mm index 55be64985f0..e5f36454a1f 100644 --- a/ReactCommon/fabric/textlayoutmanager/platform/ios/TextLayoutManager.mm +++ b/ReactCommon/fabric/textlayoutmanager/platform/ios/TextLayoutManager.mm @@ -17,21 +17,23 @@ TextLayoutManager::TextLayoutManager(ContextContainer::Shared const &contextCont self_ = (__bridge_retained void *)[RCTTextLayoutManager new]; } -TextLayoutManager::~TextLayoutManager() { +TextLayoutManager::~TextLayoutManager() +{ CFRelease(self_); self_ = nullptr; } -void *TextLayoutManager::getNativeTextLayoutManager() const { +void *TextLayoutManager::getNativeTextLayoutManager() const +{ return self_; } Size TextLayoutManager::measure( AttributedString attributedString, ParagraphAttributes paragraphAttributes, - LayoutConstraints layoutConstraints) const { - RCTTextLayoutManager *textLayoutManager = - (__bridge RCTTextLayoutManager *)self_; + LayoutConstraints layoutConstraints) const +{ + RCTTextLayoutManager *textLayoutManager = (__bridge RCTTextLayoutManager *)self_; return [textLayoutManager measureWithAttributedString:attributedString paragraphAttributes:paragraphAttributes layoutConstraints:layoutConstraints];