Extract rect clipping logic into AbstractClippingDrawCommand

Summary: Both DrawView and AbstractDrawCommand have clipping logic, this diff is moving out logic into a common base class. This also reverts the screenshot tests fix, which was causing issues with overflow: visible.

Reviewed By: ahmedre

Differential Revision: D2933780

ReactAndroid/src/main/java/com/facebook/react/flat/AbstractClippingDrawCommand.java

@@ -0,0 +1,60 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+package com.facebook.react.flat;
+
+import android.graphics.Canvas;
+
+/* package */ abstract class AbstractClippingDrawCommand implements DrawCommand {
+
+  private float mClipLeft;
+  private float mClipTop;
+  private float mClipRight;
+  private float mClipBottom;
+
+  public final boolean clipBoundsMatch(
+      float clipLeft,
+      float clipTop,
+      float clipRight,
+      float clipBottom) {
+    return mClipLeft == clipLeft && mClipTop == clipTop
+        && mClipRight == clipRight && mClipBottom == clipBottom;
+  }
+
+  public final void setClipBounds(
+      float clipLeft,
+      float clipTop,
+      float clipRight,
+      float clipBottom) {
+    mClipLeft = clipLeft;
+    mClipTop = clipTop;
+    mClipRight = clipRight;
+    mClipBottom = clipBottom;
+  }
+
+  public final float getClipLeft() {
+    return mClipLeft;
+  }
+
+  public final float getClipTop() {
+    return mClipTop;
+  }
+
+  public final float getClipRight() {
+    return mClipRight;
+  }
+
+  public final float getClipBottom() {
+    return mClipBottom;
+  }
+
+  protected final void applyClipping(Canvas canvas) {
+    canvas.clipRect(mClipLeft, mClipTop, mClipRight, mClipBottom);
+  }
+}

ReactAndroid/src/main/java/com/facebook/react/flat/AbstractDrawCommand.java

@@ -18,16 +18,13 @@ import android.graphics.Canvas;
  * The idea is to be able to reuse unmodified objects when we build up DrawCommands before we ship
  * them to UI thread, but we can only do that if DrawCommands are immutable.
  */
-/* package */ abstract class AbstractDrawCommand implements DrawCommand, Cloneable {
+/* package */ abstract class AbstractDrawCommand extends AbstractClippingDrawCommand
+    implements Cloneable {
 
   private float mLeft;
   private float mTop;
   private float mRight;
   private float mBottom;
-  private float mClipLeft;
-  private float mClipTop;
-  private float mClipRight;
-  private float mClipBottom;
   private boolean mFrozen;
 
   @Override
@@ -35,7 +32,7 @@ import android.graphics.Canvas;
     onPreDraw(parent, canvas);
     if (shouldClip()) {
       canvas.save();
-      canvas.clipRect(mClipLeft, mClipTop, mClipRight, mClipBottom);
+      applyClipping(canvas);
       onDraw(canvas);
       canvas.restore();
     } else {
@@ -137,26 +134,11 @@ import android.graphics.Canvas;
     return mBottom;
   }
 
-  public final float getClipLeft() {
-    return mClipLeft;
-  }
-
-  public final float getClipTop() {
-    return mClipTop;
-  }
-
-  public final float getClipRight() {
-    return mClipRight;
-  }
-
-  public final float getClipBottom() {
-    return mClipBottom;
-  }
-
   protected abstract void onDraw(Canvas canvas);
 
   protected boolean shouldClip() {
-    return mLeft < mClipLeft || mTop < mClipTop || mRight > mClipRight || mBottom > mClipBottom;
+    return mLeft < getClipLeft() || mTop < getClipTop() ||
+        mRight > getClipRight() || mBottom > getClipBottom();
   }
 
   protected void onBoundsChanged() {
@@ -174,26 +156,10 @@ import android.graphics.Canvas;
     onBoundsChanged();
   }
 
-  private void setClipBounds(float clipLeft, float clipTop, float clipRight, float clipBottom) {
-    mClipLeft = clipLeft;
-    mClipTop = clipTop;
-    mClipRight = clipRight;
-    mClipBottom = clipBottom;
-  }
-
   /**
    * Returns true if boundaries match and don't need to be updated. False otherwise.
    */
   private boolean boundsMatch(float left, float top, float right, float bottom) {
     return mLeft == left && mTop == top && mRight == right && mBottom == bottom;
   }
-
-  private boolean clipBoundsMatch(
-      float clipLeft,
-      float clipTop,
-      float clipRight,
-      float clipBottom) {
-    return mClipLeft == clipLeft && mClipTop == clipTop &&
-        mClipRight == clipRight && mClipBottom == clipBottom;
-  }
 }

ReactAndroid/src/main/java/com/facebook/react/flat/DrawView.java

@@ -10,44 +10,19 @@
 package com.facebook.react.flat;
 
 import android.graphics.Canvas;
-import android.graphics.Rect;
 
-/* package */ final class DrawView implements DrawCommand {
+/* package */ final class DrawView extends AbstractClippingDrawCommand {
 
   /* package */ static final DrawView INSTANCE = new DrawView(0, 0, 0, 0);
-  private static final Rect TMP_CLIP_RECT = new Rect();
-
-  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;
+    setClipBounds(clipLeft, clipTop, clipRight, clipBottom);
-    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) {
-    // This should not be required, except that there is a bug in Canvas that only shows up in
-    // screenshot tests where Canvas incorrectly applies clip rect caused by integer overflows
-    // because software Canvas is actually using ints for bounds, not floats.
-    canvas.getClipBounds(TMP_CLIP_RECT);
-    TMP_CLIP_RECT.intersect(
-        Math.round(mClipLeft),
-        Math.round(mClipTop),
-        Math.round(mClipRight),
-        Math.round(mClipBottom));
-
     canvas.save();
-    canvas.clipRect(TMP_CLIP_RECT);
+    applyClipping(canvas);
     parent.drawNextChild(canvas);
     canvas.restore();
   }