fix clipping issue

ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java

@@ -351,8 +351,6 @@ public void updateClippingRect() {
 
     ReactClippingViewGroupHelper.calculateClippingRect(this, mClippingRect);
     updateClippingToRect(mClippingRect);
-    // see comment at `updateDrawingOrderHelper`.
-    updateDrawingOrderHelper();
   }
 
   private void updateClippingToRect(Rect clippingRect) {
@@ -384,18 +382,54 @@ private void updateSubviewClipStatus(Rect clippingRect, int idx, int clippedSoFa
     Animation animation = child.getAnimation();
     boolean isAnimating = animation != null && !animation.hasEnded();
 
+    // NOTE (apkumar):
+    //
+    // The `preventClipping` logic here, and the `getDrawingOrderHelper().*`
+    // calls within the if-else statements below, are added in our fork. They
+    // work together to support `removeClippedSubviews` in the presence of
+    // animated subviews.
+    // 
+    // Typically, when `removeClippedSubviews` is turned on, you run the risk
+    // of animated subviews being clipped when they shouldn't be, since their
+    // bounding rectangle may be outside the clipping window, but due to the
+    // animation transforming the view, the actual rendering _would be_ inside
+    // the clipping window. To fix this, we added a `preventClipping` prop to
+    // Views, and here we simply never clip any View that has that prop set to
+    // true.
+    //
+    // That change fixes the clipping issue, but exposed a second problem: when
+    // `removeClippedSubviews` is turned on, React Native's zIndex system is
+    // not respected. The issue is that, normally, the drawing order helper is
+    // informed of new and removed views via handleAddView and
+    // handleRemoveView, called in `addView` and `removeView` respectively.
+    // However, when removeClippedSubviews is true,
+    // `addViewWithSubviewClippingEnabled` is called _instead of_ `addView`,
+    // which _does not_ call into the drawing order helper's handleAddView
+    // (with a similar story for removing views). Because of this, the drawing
+    // order helper is not aware of any child views, and so does not perform
+    // any of the z-indexing logic it normally does. 
+    //
+    // To fix that second issue, we simply have to call handleRemoveView /
+    // handleAddView explicitly here, when the clipping logic adds or removes
+    // views because of their intersection with the clipping window.
     boolean preventClipping = false;
     if (child instanceof ReactViewGroup) {
       preventClipping = ((ReactViewGroup)child).getPreventClipping();
     }
 
     if (!intersects && child.getParent() != null && !isAnimating && !preventClipping) {
+      getDrawingOrderHelper().handleRemoveView(child);
+      setChildrenDrawingOrderEnabled(getDrawingOrderHelper().shouldEnableCustomDrawingOrder());
       // We can try saving on invalidate call here as the view that we remove is out of visible area
       // therefore invalidation is not necessary.
       super.removeViewsInLayout(idx - clippedSoFar, 1);
       needUpdateClippingRecursive = true;
     } else if ((intersects || preventClipping) && child.getParent() == null) {
+      getDrawingOrderHelper().handleAddView(child);
+      setChildrenDrawingOrderEnabled(getDrawingOrderHelper().shouldEnableCustomDrawingOrder());
+
       super.addViewInLayout(child, idx - clippedSoFar, sDefaultLayoutParam, true);
+
       invalidate();
       needUpdateClippingRecursive = true;
     } else if (intersects || preventClipping) {
@@ -730,46 +764,6 @@ private ReactViewBackgroundDrawable getOrCreateReactViewBackground() {
     return mReactBackgroundDrawable;
   }
 
-  // NOTE(apkumar)
-  // 
-  // This is added in our (Discord's) fork. 
-  //
-  // Normally, the drawing order helper is informed of new and removed views
-  // via handleAddView and handleRemoveView, called in `addView` and
-  // `removeView` respectively. However, when removeClippedSubviews is true,
-  // `addViewWithSubviewClippingEnabled` is called _instead of_ `addView`,
-  // which _does not_ call into the drawing order helper's handleAddView (with
-  // a similar story for removing views). Because of this, the drawing order
-  // helper is not aware of any child views, and so does not perform any of the
-  // z-indexing logic it normally does.
-  //
-  // There were two ways to fix this: we could either simply call
-  // `handleAddView` in `addViewWithSubviewClippingEnabled` (similar for the
-  // remove view path), or we could explicitly update the drawing order helper
-  // after we attach/remove views as part of the clipping process. Here, we've
-  // opted for the second approach.
-  //
-  // I've opted for the second approach because the drawing helper's `update()`
-  // call looks through the view group's children to figure out which ones have
-  // z indices. This implies that the `handleAddView`/`handleRemoveView` calls
-  // should, semantically, be 1-1 with the view group's children changing (so
-  // that calling `update()` after one of the `handle*` calls does not change
-  // anything). Since `addViewWithSubviewClippingEnabled` doesn't actually add
-  // the view to the children (that happens during the clipping process), I
-  // thought it made more sense to just explicitly update the drawing helper.
-  //
-  // Note that a third option would have been to call `handleAddView` and
-  // `handleRemoveView` in a more fine-grained way during the clipping process,
-  // but that is just slightly more intrusive, and it doesn't really feel like
-  // that would be any sort of performance boost. It's possible that in some
-  // cases there is a meaningful performance boost (otherwise I'm not sure why
-  // the drawing order helper even has the `handle*` API), so if we find that
-  // to be the case, we can be more fine-grained.
-  private void updateDrawingOrderHelper() {
-    getDrawingOrderHelper().update();
-    setChildrenDrawingOrderEnabled(getDrawingOrderHelper().shouldEnableCustomDrawingOrder());
-  }
-
   @Override
   public @Nullable Rect getHitSlopRect() {
     return mHitSlopRect;