Skip to content

Commit

Permalink
ReactViewGroup - track clipped views (#47987)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #47987

Store a tag value for whether the view is added or removed, to better track the state instead of checking view.getParent().

Changelog: [Internal]

Reviewed By: javache

Differential Revision: D66383241

fbshipit-source-id: 16521eb4052e9473be058a00cfe29d7f198b7861
  • Loading branch information
Thomas Nardone authored and facebook-github-bot committed Dec 2, 2024
1 parent 69ecaef commit 53bc661
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 13 deletions.
3 changes: 2 additions & 1 deletion packages/react-native/ReactAndroid/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,8 @@ android {
"src/main/res/shell",
"src/main/res/views/alert",
"src/main/res/views/modal",
"src/main/res/views/uimanager"))
"src/main/res/views/uimanager",
"src/main/res/views/view"))
java.exclude("com/facebook/react/processing")
java.exclude("com/facebook/react/module/processing")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public BaseViewManager(@Nullable ReactApplicationContext reactContext) {
view.setTag(R.id.accessibility_actions, null);
view.setTag(R.id.accessibility_value, null);
view.setTag(R.id.accessibility_state_expanded, null);
view.setTag(R.id.view_clipped, null);

// This indirectly calls (and resets):
// setTranslationX
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ public void shutdown() {
// {@link ViewGroup#getChildCount} so those methods may return views that are not attached.
// This is risky but allows us to perform a correct cleanup in {@link NativeViewHierarchyManager}.
private boolean mRemoveClippedSubviews;
private volatile boolean mInSubviewClippingLoop;
private @Nullable View[] mAllChildren;
private int mAllChildrenCount;
private @Nullable Rect mClippingRect;
Expand Down Expand Up @@ -158,6 +159,7 @@ private void initView() {
setClipChildren(false);

mRemoveClippedSubviews = false;
mInSubviewClippingLoop = false;
mAllChildren = null;
mAllChildrenCount = 0;
mClippingRect = null;
Expand Down Expand Up @@ -363,6 +365,7 @@ public void setRemoveClippedSubviews(boolean removeClippedSubviews) {
View child = getChildAt(i);
mAllChildren[i] = child;
child.addOnLayoutChangeListener(mChildrenLayoutChangeListener);
setViewClipped(child, false);
}
updateClippingRect();
} else {
Expand Down Expand Up @@ -407,6 +410,7 @@ public void updateClippingRect() {

private void updateClippingToRect(Rect clippingRect) {
Assertions.assertNotNull(mAllChildren);
mInSubviewClippingLoop = true;
int clippedSoFar = 0;
for (int i = 0; i < mAllChildrenCount; i++) {
try {
Expand All @@ -415,7 +419,7 @@ private void updateClippingToRect(Rect clippingRect) {
int realClippedSoFar = 0;
Set<View> uniqueViews = new HashSet<>();
for (int j = 0; j < i; j++) {
realClippedSoFar += isViewClipped(mAllChildren[j]) ? 1 : 0;
realClippedSoFar += isViewClipped(mAllChildren[j], null) ? 1 : 0;
uniqueViews.add(mAllChildren[j]);
}

Expand All @@ -436,10 +440,11 @@ private void updateClippingToRect(Rect clippingRect) {
+ uniqueViews.size(),
e);
}
if (isViewClipped(mAllChildren[i])) {
if (isViewClipped(mAllChildren[i], i)) {
clippedSoFar++;
}
}
mInSubviewClippingLoop = false;
}

private void updateSubviewClipStatus(Rect clippingRect, int idx, int clippedSoFar) {
Expand All @@ -458,14 +463,16 @@ private void updateSubviewClipStatus(Rect clippingRect, int idx, int clippedSoFa
// it won't be size and located properly.
Animation animation = child.getAnimation();
boolean isAnimating = animation != null && !animation.hasEnded();
if (!intersects && !isViewClipped(child) && !isAnimating) {
if (!intersects && !isViewClipped(child, idx) && !isAnimating) {
setViewClipped(child, true);
// We can try saving on invalidate call here as the view that we remove is out of visible area
// therefore invalidation is not necessary.
removeViewInLayout(child);
needUpdateClippingRecursive = true;
} else if (intersects && isViewClipped(child)) {
} else if (intersects && isViewClipped(child, idx)) {
int adjustedIdx = idx - clippedSoFar;
Assertions.assertCondition(adjustedIdx >= 0);
setViewClipped(child, false);
addViewInLayout(child, adjustedIdx, sDefaultLayoutParam, true);
invalidate();
needUpdateClippingRecursive = true;
Expand Down Expand Up @@ -497,19 +504,21 @@ private void updateSubviewClipStatus(View subview) {
subview.getLeft(), subview.getTop(), subview.getRight(), subview.getBottom());

// If it was intersecting before, should be attached to the parent
boolean oldIntersects = !isViewClipped(subview);
boolean oldIntersects = !isViewClipped(subview, null);

if (intersects != oldIntersects) {
mInSubviewClippingLoop = true;
int clippedSoFar = 0;
for (int i = 0; i < mAllChildrenCount; i++) {
if (mAllChildren[i] == subview) {
updateSubviewClipStatus(mClippingRect, i, clippedSoFar);
break;
}
if (isViewClipped(mAllChildren[i])) {
if (isViewClipped(mAllChildren[i], i)) {
clippedSoFar++;
}
}
mInSubviewClippingLoop = false;
}
}

Expand Down Expand Up @@ -541,7 +550,7 @@ private boolean customDrawOrderDisabled() {
@Override
public void onViewAdded(View child) {
UiThreadUtil.assertOnUiThread();

checkViewClippingTag(child, Boolean.FALSE);
if (!customDrawOrderDisabled()) {
getDrawingOrderHelper().handleAddView(child);
setChildrenDrawingOrderEnabled(getDrawingOrderHelper().shouldEnableCustomDrawingOrder());
Expand All @@ -554,7 +563,7 @@ public void onViewAdded(View child) {
@Override
public void onViewRemoved(View child) {
UiThreadUtil.assertOnUiThread();

checkViewClippingTag(child, Boolean.TRUE);
if (!customDrawOrderDisabled()) {
if (indexOfChild(child) == -1) {
return;
Expand All @@ -567,6 +576,21 @@ public void onViewRemoved(View child) {
super.onViewRemoved(child);
}

private void checkViewClippingTag(View child, Boolean expectedTag) {
if (mInSubviewClippingLoop) {
Object tag = child.getTag(R.id.view_clipped);
if (!expectedTag.equals(tag)) {
ReactSoftExceptionLogger.logSoftException(
"ReactViewGroup.onViewRemoved",
new ReactNoCrashSoftException(
"View clipping tag mismatch: tag=" + tag + " expected=" + expectedTag));
}
}
if (mRemoveClippedSubviews) {
child.setTag(R.id.view_clipped, expectedTag);
}
}

@Override
protected int getChildDrawingOrder(int childCount, int index) {
UiThreadUtil.assertOnUiThread();
Expand Down Expand Up @@ -638,19 +662,22 @@ View getChildAtWithSubviewClippingEnabled(int index) {
/*package*/ void addViewWithSubviewClippingEnabled(
final View child, int index, ViewGroup.LayoutParams params) {
Assertions.assertCondition(mRemoveClippedSubviews);
setViewClipped(child, true); // the view has not been added, so it is "clipped"
addInArray(child, index);

// we add view as "clipped" and then run {@link #updateSubviewClipStatus} to conditionally
// attach it
Rect clippingRect = Assertions.assertNotNull(mClippingRect);
View[] childArray = Assertions.assertNotNull(mAllChildren);
mInSubviewClippingLoop = true;
int clippedSoFar = 0;
for (int i = 0; i < index; i++) {
if (isViewClipped(childArray[i])) {
if (isViewClipped(childArray[i], i)) {
clippedSoFar++;
}
}
updateSubviewClipStatus(clippingRect, index, clippedSoFar);
mInSubviewClippingLoop = false;
child.addOnLayoutChangeListener(mChildrenLayoutChangeListener);

if (child instanceof ReactClippingProhibitedView) {
Expand Down Expand Up @@ -685,10 +712,10 @@ public void run() {
View[] childArray = Assertions.assertNotNull(mAllChildren);
view.removeOnLayoutChangeListener(mChildrenLayoutChangeListener);
int index = indexOfChildInAllChildren(view);
if (!isViewClipped(childArray[index])) {
if (!isViewClipped(childArray[index], index)) {
int clippedSoFar = 0;
for (int i = 0; i < index; i++) {
if (isViewClipped(childArray[i])) {
if (isViewClipped(childArray[i], i)) {
clippedSoFar++;
}
}
Expand All @@ -709,9 +736,27 @@ public void run() {

/**
* @return {@code true} if the view has been removed from the ViewGroup.
* @param index For logging - index of the view in {@code mAllChildren}, or {@code null} to skip
* logging.
*/
private boolean isViewClipped(View view) {
private boolean isViewClipped(View view, @Nullable Integer index) {
Object tag = view.getTag(R.id.view_clipped);
if (tag != null) {
return (boolean) tag;
}
ViewParent parent = view.getParent();
if (index != null) {
ReactSoftExceptionLogger.logSoftException(
"ReactViewGroup.isViewClipped",
new ReactNoCrashSoftException(
"View missing clipping tag: index="
+ index
+ " parentNull="
+ (parent == null)
+ " parentThis="
+ (parent == this)));
}
// fallback - parent *should* be null if the view was removed
if (parent == null) {
return true;
} else {
Expand All @@ -720,6 +765,10 @@ private boolean isViewClipped(View view) {
}
}

private static void setViewClipped(View view, boolean clipped) {
view.setTag(R.id.view_clipped, clipped);
}

private int indexOfChildInAllChildren(View child) {
final int count = mAllChildrenCount;
final View[] childArray = Assertions.assertNotNull(mAllChildren);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<resources>
<!-- tag used to store state of ReactViewGroup subview clipping -->
<item type="id" name="view_clipped"/>
</resources>

0 comments on commit 53bc661

Please sign in to comment.