From 3b63d7c3bcf20ba70ba2b464871eb10dbc3b444f Mon Sep 17 00:00:00 2001 From: Denis Koroskin Date: Thu, 18 Feb 2016 23:56:37 -0800 Subject: [PATCH] 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 --- .../flat/AbstractClippingDrawCommand.java | 60 +++++++++++++++++++ .../react/flat/AbstractDrawCommand.java | 44 ++------------ .../com/facebook/react/flat/DrawView.java | 31 +--------- 3 files changed, 68 insertions(+), 67 deletions(-) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/flat/AbstractClippingDrawCommand.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/AbstractClippingDrawCommand.java b/ReactAndroid/src/main/java/com/facebook/react/flat/AbstractClippingDrawCommand.java new file mode 100644 index 000000000..3ac0391c8 --- /dev/null +++ b/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); + } +} diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/AbstractDrawCommand.java b/ReactAndroid/src/main/java/com/facebook/react/flat/AbstractDrawCommand.java index cf16d5851..55b03c2d4 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/AbstractDrawCommand.java +++ b/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; - } } 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 f51e200d4..dc811a2a8 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/DrawView.java +++ b/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; - 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; + setClipBounds(clipLeft, clipTop, clipRight, 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(); }