From b321ea98d336ce44ba3657e708fd5682162159ba Mon Sep 17 00:00:00 2001
From: Denis Koroskin <dkoroskin@fb.com>
Date: Mon, 4 Jan 2016 18:54:17 -0800
Subject: [PATCH] Properly clip child Views in FlatViewGroup

Summary:
Everything (but Views) are drawn using AbstractDrawCommand (or its derivative, like DrawBorder or DrawBackgroundColor) and supports clipping properly. Views however are drawn using an assumption that Android will clip them manually. This is however not always true. One example is if an element A mounts to a View, and its parent B doesn't but has overflow: hidden and thus should clip the child.

There are 2 ways to fix this:
- pop every element that has overflow: hidden to its own View. In this case, its children will always be correctly clipped. This is however very inefficient, especially if overflow: hidden is default (which it is!) which means that almost every React element must be backed by an Android View.
- add clipping information to DrawView, similar to how AbstractDrawCommand has it.

This diff implements the latter approach.

Reviewed By: ahmedre

Differential Revision: D2792375
---
 .../com/facebook/react/flat/DrawView.java     | 22 +++++++++++++++++--
 .../facebook/react/flat/FlatShadowNode.java   | 17 ++++++++++----
 .../com/facebook/react/flat/StateBuilder.java |  6 ++++-
 3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/DrawView.java b/ReactAndroid/src/main/java/com/facebook/react/flat/DrawView.java
index 9ffc25823..e8b22746f 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/flat/DrawView.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/flat/DrawView.java
@@ -13,12 +13,30 @@ import android.graphics.Canvas;
 
 /* package */ final class DrawView implements DrawCommand {
 
-  /* package */ static DrawView INSTANCE = new DrawView();
+  /* package */ static DrawView INSTANCE = new DrawView(0, 0, 0, 0);
 
-  private DrawView() {}
+  private final float mClipLeft;
+  private final float mClipTop;
+  private final float mClipRight;
+  private final float mClipBottom;
+
+  public DrawView(float clipLeft, float clipTop, float clipRight, float clipBottom) {
+    mClipLeft = clipLeft;
+    mClipTop = clipTop;
+    mClipRight = clipRight;
+    mClipBottom = clipBottom;
+  }
+
+  public boolean clipBoundsMatch(float clipLeft, float clipTop, float clipRight, float clipBottom) {
+    return mClipLeft == clipLeft && mClipTop == clipTop
+        && mClipRight == clipRight && mClipBottom == clipBottom;
+  }
 
   @Override
   public void draw(FlatViewGroup parent, Canvas canvas) {
+    canvas.save();
+    canvas.clipRect(mClipLeft, mClipTop, mClipRight, mClipBottom);
     parent.drawNextChild(canvas);
+    canvas.restore();
   }
 }
diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatShadowNode.java b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatShadowNode.java
index d32256ed8..d4af1867c 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatShadowNode.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatShadowNode.java
@@ -11,6 +11,7 @@ package com.facebook.react.flat;
 
 import javax.annotation.Nullable;
 
+import com.facebook.infer.annotation.Assertions;
 import com.facebook.react.uimanager.CatalystStylesDiffMap;
 import com.facebook.react.uimanager.LayoutShadowNode;
 import com.facebook.react.uimanager.ReactProp;
@@ -43,8 +44,8 @@ import com.facebook.react.uimanager.ViewProps;
   private int mViewTop;
   private int mViewRight;
   private int mViewBottom;
-  private boolean mMountsToView;
   private boolean mBackingViewIsCreated;
+  private @Nullable DrawView mDrawView;
   private @Nullable DrawBackgroundColor mDrawBackground;
   private int mMoveToIndexInParent;
   private boolean mIsOverflowVisible = true;
@@ -253,16 +254,24 @@ import com.facebook.react.uimanager.ViewProps;
   }
 
   /* package */ final void forceMountToView() {
-    if (!mMountsToView) {
-      mMountsToView = true;
+    if (mDrawView == null) {
+      mDrawView = DrawView.INSTANCE;
       if (getParent() != null) {
         invalidate();
       }
     }
   }
 
+  /* package */ final DrawView collectDrawView(float left, float top, float right, float bottom) {
+    if (!Assertions.assumeNotNull(mDrawView).clipBoundsMatch(left, top, right, bottom)) {
+      mDrawView = new DrawView(left, top, right, bottom);
+    }
+
+    return mDrawView;
+  }
+
   /* package */ final boolean mountsToView() {
-    return mMountsToView;
+    return mDrawView != null;
   }
 
   /* package */ final boolean isBackingViewCreated() {
diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java
index 989a27618..f75b6655f 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java
@@ -425,7 +425,11 @@ import com.facebook.react.uimanager.events.EventDispatcher;
 
       addNativeChild(node);
       if (!parentIsAndroidView) {
-        mDrawCommands.add(DrawView.INSTANCE);
+        mDrawCommands.add(node.collectDrawView(
+            parentClipLeft,
+            parentClipTop,
+            parentClipRight,
+            parentClipBottom));
       }
 
       collectStateForMountableNode(