Skip to content

Commit bc9168d

Browse files
ryancatfacebook-github-bot
authored andcommitted
Add overflowInset to RN Android ViewGroup as separate mount instruction
Summary: This diff adds `overflowInset` values in RN Android. These values are used to give an enlarged boundary of a view group that also contains all its children layout. Here is [the post](https://fb.workplace.com/groups/yogalayout/permalink/2264980363573947/) that discuss more on why this is useful. I steal the pic in that post here as TLDR: {F687030994} In the above case, we will get overflowInset for ViewGroup A as something like `top: 0, right: -20, bottom: -20, left: 0`. This has been added in the [Fabric core](https://fburl.com/code/f8c5tg7b) and [in IOS](https://fburl.com/code/vkh0hpt6). In Android, since we used to ignore all event coordinates outside of a ViewGroup boundary, this is not an issue. However, that caused unregistered touch area problem and got fixed in D30104853 (facebook@e35a963), which dropped the boundary check and made the hit test algorithm in [TouchTargetHelper.java](https://fburl.com/code/dj8jiz22) worse as we now need to explore all the child node under ReactRootNode. This perf issue is getting obvious when a view loads too many items, which matches our experience with "Hover getting slow after scrolling", "Hover getting slow after going back from PDP view", and "The saved list view (in Explore) is very fast (because it has very few components)" To fix this issue, I added the support to `overflowInset` to RN Android by 1. Sending the `overflowInset` values from Binding.cpp in RN Android as a separate mount instruction 2. Update `IntBufferBatchMountItem.java` to read the int buffer sent over JNI, and pass the `overflowInset` values to `SurfaceMountingManager.java` 3. Creating new interface `ReactOverflowViewWithInset.java` and extending the existing `ReactOverflowView.java` usages 4. Adding implementation of getter and setter for `overflowInset` in various views 5. Update `TouchTargetHelper.java` to read the values and check boundaries before exploring ViewGroup's children Note that in #3 I didn't change `ReactOverflowView.java` interface directly. I am concerned about backward compatibility issues in case this interface is being used in OSS. I suggest we deprecate it as we are not using it anymore in our code. Changelog: [Internal][Android] Reviewed By: JoshuaGross Differential Revision: D33133977 fbshipit-source-id: 64e3e837fe7ca6e6dbdbc836ab0615182e10f28c
1 parent 821382b commit bc9168d

10 files changed

+211
-10
lines changed

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountItem.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ CppMountItem CppMountItem::UpdatePaddingMountItem(
4545
ShadowView const &shadowView) {
4646
return {CppMountItem::Type::UpdatePadding, {}, {}, shadowView, -1};
4747
}
48+
CppMountItem CppMountItem::UpdateOverflowInsetMountItem(
49+
ShadowView const &shadowView) {
50+
return {CppMountItem::Type::UpdateOverflowInset, {}, {}, shadowView, -1};
51+
}
4852

4953
} // namespace react
5054
} // namespace facebook

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountItem.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ struct CppMountItem final {
4545

4646
static CppMountItem UpdatePaddingMountItem(ShadowView const &shadowView);
4747

48+
static CppMountItem UpdateOverflowInsetMountItem(
49+
ShadowView const &shadowView);
50+
4851
#pragma mark - Type
4952

5053
enum Type {
@@ -58,7 +61,8 @@ struct CppMountItem final {
5861
UpdateState = 64,
5962
UpdateLayout = 128,
6063
UpdateEventEmitter = 256,
61-
UpdatePadding = 512
64+
UpdatePadding = 512,
65+
UpdateOverflowInset = 1024
6266
};
6367

6468
#pragma mark - Fields

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.cpp

+58-1
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ static inline int getIntBufferSizeForType(CppMountItem::Type mountItemType) {
4343
return 5; // tag, top, left, bottom, right
4444
case CppMountItem::Type::UpdateLayout:
4545
return 6; // tag, x, y, w, h, DisplayType
46+
case CppMountItem::Type::UpdateOverflowInset:
47+
return 5; // tag, left, top, right, bottom
4648
case CppMountItem::Undefined:
4749
case CppMountItem::Multiple:
4850
return -1;
@@ -84,6 +86,7 @@ static inline void computeBufferSizes(
8486
std::vector<CppMountItem> &cppUpdateStateMountItems,
8587
std::vector<CppMountItem> &cppUpdatePaddingMountItems,
8688
std::vector<CppMountItem> &cppUpdateLayoutMountItems,
89+
std::vector<CppMountItem> &cppUpdateOverflowInsetMountItems,
8790
std::vector<CppMountItem> &cppUpdateEventEmitterMountItems) {
8891
CppMountItem::Type lastType = CppMountItem::Type::Undefined;
8992
int numSameType = 0;
@@ -128,6 +131,11 @@ static inline void computeBufferSizes(
128131
cppUpdateLayoutMountItems.size(),
129132
batchMountItemIntsSize,
130133
batchMountItemObjectsSize);
134+
updateBufferSizes(
135+
CppMountItem::Type::UpdateOverflowInset,
136+
cppUpdateOverflowInsetMountItems.size(),
137+
batchMountItemIntsSize,
138+
batchMountItemObjectsSize);
131139
updateBufferSizes(
132140
CppMountItem::Type::UpdateEventEmitter,
133141
cppUpdateEventEmitterMountItems.size(),
@@ -232,6 +240,7 @@ void FabricMountingManager::executeMount(
232240
std::vector<CppMountItem> cppUpdateStateMountItems;
233241
std::vector<CppMountItem> cppUpdatePaddingMountItems;
234242
std::vector<CppMountItem> cppUpdateLayoutMountItems;
243+
std::vector<CppMountItem> cppUpdateOverflowInsetMountItems;
235244
std::vector<CppMountItem> cppUpdateEventEmitterMountItems;
236245

237246
for (const auto &mutation : mutations) {
@@ -293,6 +302,16 @@ void FabricMountingManager::executeMount(
293302
CppMountItem::UpdateLayoutMountItem(
294303
mutation.newChildShadowView));
295304
}
305+
306+
// OverflowInset: This is the values indicating boundaries including
307+
// children of the current view. The layout of current view may not
308+
// change, and we separate this part from layout mount items to not
309+
// pack too much data there.
310+
if (oldChildShadowView.layoutMetrics.overflowInset !=
311+
newChildShadowView.layoutMetrics.overflowInset) {
312+
cppUpdateOverflowInsetMountItems.push_back(
313+
CppMountItem::UpdateOverflowInsetMountItem(newChildShadowView));
314+
}
296315
}
297316

298317
if (oldChildShadowView.eventEmitter !=
@@ -331,6 +350,13 @@ void FabricMountingManager::executeMount(
331350
// Layout
332351
cppUpdateLayoutMountItems.push_back(
333352
CppMountItem::UpdateLayoutMountItem(mutation.newChildShadowView));
353+
354+
// OverflowInset: This is the values indicating boundaries including
355+
// children of the current view. The layout of current view may not
356+
// change, and we separate this part from layout mount items to not
357+
// pack too much data there.
358+
cppUpdateOverflowInsetMountItems.push_back(
359+
CppMountItem::UpdateOverflowInsetMountItem(newChildShadowView));
334360
}
335361

336362
// EventEmitter
@@ -359,6 +385,7 @@ void FabricMountingManager::executeMount(
359385
cppUpdateStateMountItems,
360386
cppUpdatePaddingMountItems,
361387
cppUpdateLayoutMountItems,
388+
cppUpdateOverflowInsetMountItems,
362389
cppUpdateEventEmitterMountItems);
363390

364391
static auto createMountItemsIntBufferBatchContainer =
@@ -407,7 +434,7 @@ void FabricMountingManager::executeMount(
407434
int intBufferPosition = 0;
408435
int objBufferPosition = 0;
409436
int prevMountItemType = -1;
410-
jint temp[7];
437+
jint temp[6];
411438
for (int i = 0; i < cppCommonMountItems.size(); i++) {
412439
const auto &mountItem = cppCommonMountItems[i];
413440
const auto &mountItemType = mountItem.type;
@@ -594,6 +621,36 @@ void FabricMountingManager::executeMount(
594621
intBufferPosition += 6;
595622
}
596623
}
624+
if (!cppUpdateOverflowInsetMountItems.empty()) {
625+
writeIntBufferTypePreamble(
626+
CppMountItem::Type::UpdateOverflowInset,
627+
cppUpdateOverflowInsetMountItems.size(),
628+
env,
629+
intBufferArray,
630+
intBufferPosition);
631+
632+
for (const auto &mountItem : cppUpdateOverflowInsetMountItems) {
633+
auto layoutMetrics = mountItem.newChildShadowView.layoutMetrics;
634+
auto pointScaleFactor = layoutMetrics.pointScaleFactor;
635+
auto overflowInset = layoutMetrics.overflowInset;
636+
637+
int overflowInsetLeft =
638+
round(scale(overflowInset.left, pointScaleFactor));
639+
int overflowInsetTop = round(scale(overflowInset.top, pointScaleFactor));
640+
int overflowInsetRight =
641+
round(scale(overflowInset.right, pointScaleFactor));
642+
int overflowInsetBottom =
643+
round(scale(overflowInset.bottom, pointScaleFactor));
644+
645+
temp[0] = mountItem.newChildShadowView.tag;
646+
temp[1] = overflowInsetLeft;
647+
temp[2] = overflowInsetTop;
648+
temp[3] = overflowInsetRight;
649+
temp[4] = overflowInsetBottom;
650+
env->SetIntArrayRegion(intBufferArray, intBufferPosition, 5, temp);
651+
intBufferPosition += 5;
652+
}
653+
}
597654
if (!cppUpdateEventEmitterMountItems.empty()) {
598655
writeIntBufferTypePreamble(
599656
CppMountItem::Type::UpdateEventEmitter,

ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java

+30
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.facebook.react.fabric.mounting.mountitems.MountItem;
3333
import com.facebook.react.touch.JSResponderHandler;
3434
import com.facebook.react.uimanager.IllegalViewOperationException;
35+
import com.facebook.react.uimanager.ReactOverflowViewWithInset;
3536
import com.facebook.react.uimanager.ReactRoot;
3637
import com.facebook.react.uimanager.ReactStylesDiffMap;
3738
import com.facebook.react.uimanager.RootView;
@@ -758,6 +759,35 @@ public void updatePadding(int reactTag, int left, int top, int right, int bottom
758759
viewManager.setPadding(viewToUpdate, left, top, right, bottom);
759760
}
760761

762+
@UiThread
763+
public void updateOverflowInset(
764+
int reactTag,
765+
int overflowInsetLeft,
766+
int overflowInsetTop,
767+
int overflowInsetRight,
768+
int overflowInsetBottom) {
769+
if (isStopped()) {
770+
return;
771+
}
772+
773+
ViewState viewState = getViewState(reactTag);
774+
// Do not layout Root Views
775+
if (viewState.mIsRoot) {
776+
return;
777+
}
778+
779+
View viewToUpdate = viewState.mView;
780+
if (viewToUpdate == null) {
781+
throw new IllegalStateException("Unable to find View for tag: " + reactTag);
782+
}
783+
784+
if (viewToUpdate instanceof ReactOverflowViewWithInset) {
785+
((ReactOverflowViewWithInset) viewToUpdate)
786+
.setOverflowInset(
787+
overflowInsetLeft, overflowInsetTop, overflowInsetRight, overflowInsetBottom);
788+
}
789+
}
790+
761791
@UiThread
762792
public void updateState(final int reactTag, @Nullable StateWrapper stateWrapper) {
763793
UiThreadUtil.assertOnUiThread();

ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItem.java

+24
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ public class IntBufferBatchMountItem implements MountItem {
4949
static final int INSTRUCTION_UPDATE_LAYOUT = 128;
5050
static final int INSTRUCTION_UPDATE_EVENT_EMITTER = 256;
5151
static final int INSTRUCTION_UPDATE_PADDING = 512;
52+
static final int INSTRUCTION_UPDATE_OVERFLOW_INSET = 1024;
5253

5354
private final int mSurfaceId;
5455
private final int mCommitNumber;
@@ -163,11 +164,25 @@ public void execute(@NonNull MountingManager mountingManager) {
163164
int width = mIntBuffer[i++];
164165
int height = mIntBuffer[i++];
165166
int displayType = mIntBuffer[i++];
167+
166168
surfaceMountingManager.updateLayout(reactTag, x, y, width, height, displayType);
167169

168170
} else if (type == INSTRUCTION_UPDATE_PADDING) {
169171
surfaceMountingManager.updatePadding(
170172
mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++]);
173+
} else if (type == INSTRUCTION_UPDATE_OVERFLOW_INSET) {
174+
int reactTag = mIntBuffer[i++];
175+
int overflowInsetLeft = mIntBuffer[i++];
176+
int overflowInsetTop = mIntBuffer[i++];
177+
int overflowInsetRight = mIntBuffer[i++];
178+
int overflowInsetBottom = mIntBuffer[i++];
179+
180+
surfaceMountingManager.updateOverflowInset(
181+
reactTag,
182+
overflowInsetLeft,
183+
overflowInsetTop,
184+
overflowInsetRight,
185+
overflowInsetBottom);
171186
} else if (type == INSTRUCTION_UPDATE_EVENT_EMITTER) {
172187
surfaceMountingManager.updateEventEmitter(
173188
mIntBuffer[i++], castToEventEmitter(mObjBuffer[j++]));
@@ -251,6 +266,15 @@ public String toString() {
251266
mIntBuffer[i++],
252267
mIntBuffer[i++],
253268
mIntBuffer[i++]));
269+
} else if (type == INSTRUCTION_UPDATE_OVERFLOW_INSET) {
270+
s.append(
271+
String.format(
272+
"UPDATE OVERFLOWINSET [%d]: left:%d top:%d right:%d bottom:%d\n",
273+
mIntBuffer[i++],
274+
mIntBuffer[i++],
275+
mIntBuffer[i++],
276+
mIntBuffer[i++],
277+
mIntBuffer[i++]));
254278
} else if (type == INSTRUCTION_UPDATE_EVENT_EMITTER) {
255279
j += 1;
256280
s.append(String.format("UPDATE EVENTEMITTER [%d]\n", mIntBuffer[i++]));
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.uimanager;
9+
10+
import android.graphics.Rect;
11+
import android.view.View;
12+
13+
/**
14+
* Interface that should be implemented by {@link View} subclasses that support {@code overflow}
15+
* style and want to use the overflowInset values. This allows the overflow information to be used
16+
* by {@link TouchTargetHelper} to determine if a View is touchable.
17+
*/
18+
public interface ReactOverflowViewWithInset extends ReactOverflowView {
19+
/**
20+
* Get the overflow inset rect values which indicate the extensions to the boundaries of current
21+
* view that wraps all of its children views
22+
*
23+
* @return Rect of integers indicating the left, top, right, bottom pixel extensions. The values
24+
* are non-positive (indicating enlarged boundaries).
25+
*/
26+
Rect getOverflowInset();
27+
28+
/**
29+
* Set the overflow inset rect values which indicate the extensions to the boundaries of current
30+
* view that wraps all of its children views
31+
*/
32+
void setOverflowInset(int left, int top, int right, int bottom);
33+
}

ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java

+18-2
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,14 @@ private static View findTouchTargetView(
182182
if (!isTouchPointInView(eventCoords[0], eventCoords[1], view)) {
183183
// We don't allow touches on views that are outside the bounds of an `overflow: hidden` and
184184
// `overflow: scroll` View.
185-
if (view instanceof ReactOverflowView) {
186-
@Nullable String overflow = ((ReactOverflowView) view).getOverflow();
185+
if (view instanceof ReactOverflowViewWithInset) {
186+
// If the touch point is outside of the overflowinset for the view, we can safely ignore
187+
// it.
188+
if (!isTouchPointInViewWithOverflowInset(eventCoords[0], eventCoords[1], view)) {
189+
return null;
190+
}
191+
192+
@Nullable String overflow = ((ReactOverflowViewWithInset) view).getOverflow();
187193
if (ViewProps.HIDDEN.equals(overflow) || ViewProps.SCROLL.equals(overflow)) {
188194
return null;
189195
}
@@ -253,6 +259,16 @@ private static boolean isTouchPointInView(float x, float y, View view) {
253259
}
254260
}
255261

262+
private static boolean isTouchPointInViewWithOverflowInset(float x, float y, View view) {
263+
if (!(view instanceof ReactOverflowViewWithInset)) {
264+
return false;
265+
}
266+
267+
final Rect overflowInset = ((ReactOverflowViewWithInset) view).getOverflowInset();
268+
return (x >= -overflowInset.left && x < view.getWidth() - overflowInset.right)
269+
&& (y >= -overflowInset.top && y < view.getHeight() - overflowInset.bottom);
270+
}
271+
256272
/**
257273
* Returns the coordinates of a touch in the child View. It is transform aware and will invert the
258274
* transform Matrix to find the true local points This code is taken from {@link

ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java

+13-2
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
import com.facebook.react.uimanager.MeasureSpecAssertions;
4343
import com.facebook.react.uimanager.ReactClippingViewGroup;
4444
import com.facebook.react.uimanager.ReactClippingViewGroupHelper;
45-
import com.facebook.react.uimanager.ReactOverflowView;
45+
import com.facebook.react.uimanager.ReactOverflowViewWithInset;
4646
import com.facebook.react.uimanager.ViewProps;
4747
import com.facebook.react.uimanager.events.NativeGestureUtil;
4848
import com.facebook.react.views.scroll.ReactScrollViewHelper.HasFlingAnimator;
@@ -57,7 +57,7 @@
5757
public class ReactHorizontalScrollView extends HorizontalScrollView
5858
implements ReactClippingViewGroup,
5959
FabricViewStateManager.HasFabricViewStateManager,
60-
ReactOverflowView,
60+
ReactOverflowViewWithInset,
6161
HasScrollState,
6262
HasFlingAnimator {
6363

@@ -77,6 +77,7 @@ public class ReactHorizontalScrollView extends HorizontalScrollView
7777
private final @Nullable OverScroller mScroller;
7878
private final VelocityHelper mVelocityHelper = new VelocityHelper();
7979
private final Rect mRect = new Rect();
80+
private final Rect mOverflowInset = new Rect();
8081

8182
private boolean mActivelyScrolling;
8283
private @Nullable Rect mClippingRect;
@@ -256,6 +257,16 @@ public void setOverflow(String overflow) {
256257
return mOverflow;
257258
}
258259

260+
@Override
261+
public void setOverflowInset(int left, int top, int right, int bottom) {
262+
mOverflowInset.set(left, top, right, bottom);
263+
}
264+
265+
@Override
266+
public Rect getOverflowInset() {
267+
return mOverflowInset;
268+
}
269+
259270
@Override
260271
protected void onDraw(Canvas canvas) {
261272
if (DEBUG_MODE) {

0 commit comments

Comments
 (0)