From b2f41e292192e6275e2938d29abd9607e0c30325 Mon Sep 17 00:00:00 2001 From: Seth Kirby Date: Mon, 15 Aug 2016 18:56:36 -0700 Subject: [PATCH] Refactor DrawView to avoid extra allocates. Summary: Previously, the first time we collected a draw view, we would make a clone, even though the draw view had never been mutated. This refactors draw view to avoid this extra allocate. Reviewed By: ahmedre Differential Revision: D3719056 --- .../react/flat/AbstractDrawCommand.java | 13 ++- .../com/facebook/react/flat/DrawView.java | 97 +++++++++++-------- .../facebook/react/flat/FlatShadowNode.java | 2 +- 3 files changed, 70 insertions(+), 42 deletions(-) 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 a3be1cbac..89f22b0eb 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/AbstractDrawCommand.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/AbstractDrawCommand.java @@ -42,7 +42,7 @@ import android.graphics.Color; && mClipRight == clipRight && mClipBottom == clipBottom; } - public final void setClipBounds( + protected final void setClipBounds( float clipLeft, float clipTop, float clipRight, @@ -185,6 +185,13 @@ import android.graphics.Color; return mFrozen; } + /** + * Mark this object as frozen, indicating that it should not be mutated. + */ + public final void freeze() { + mFrozen = true; + } + /** * Left position of this DrawCommand relative to the hosting View. */ @@ -226,7 +233,7 @@ import android.graphics.Color; /** * Updates boundaries of this DrawCommand. */ - private void setBounds(float left, float top, float right, float bottom) { + protected final void setBounds(float left, float top, float right, float bottom) { mLeft = left; mTop = top; mRight = right; @@ -238,7 +245,7 @@ import android.graphics.Color; /** * 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) { + protected final boolean boundsMatch(float left, float top, float right, float bottom) { return mLeft == left && mTop == top && mRight == right && mBottom == bottom; } } 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 b6e05923d..2180d3596 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/DrawView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/DrawView.java @@ -72,48 +72,53 @@ import android.graphics.RectF; float clipRight, float clipBottom, float clipRadius) { - DrawView drawView = (DrawView) - updateBoundsAndFreeze( - left, - top, - right, - bottom, - clipLeft, - clipTop, - clipRight, - clipBottom); - boolean clipRadiusChanged = Math.abs(mClipRadius - clipRadius) > 0.001f; - boolean logicalBoundsChanged = - !logicalBoundsMatch(logicalLeft, logicalTop, logicalRight, logicalBottom); - if (drawView == this && (clipRadiusChanged || logicalBoundsChanged)) { - // everything matches except the clip radius, so we clone the old one so that we can update - // the clip radius in the block below. - try { - drawView = (DrawView) clone(); - } catch (CloneNotSupportedException e) { - // This should not happen since AbstractDrawCommand implements Cloneable - throw new RuntimeException(e); - } + if (!isFrozen()) { + // We haven't collected this draw view yet, so we can just set everything. + setBounds(left, top, right, bottom); + setClipBounds(clipLeft, clipTop, clipRight, clipBottom); + setClipRadius(clipRadius); + setLogicalBounds(logicalLeft, logicalTop, logicalRight, logicalBottom); + freeze(); + return this; } - if (drawView != this) { - drawView.mClipRadius = clipRadius; - if (clipRadius > MINIMUM_ROUNDED_CLIPPING_VALUE) { - // update the path that we'll clip based on - drawView.updateClipPath(); - } else { - drawView.mPath = null; - } + boolean boundsMatch = boundsMatch(left, top, right, bottom); + boolean clipBoundsMatch = clipBoundsMatch(clipLeft, clipTop, clipRight, clipBottom); + boolean clipRadiusMatch = mClipRadius == clipRadius; + boolean logicalBoundsMatch = + logicalBoundsMatch(logicalLeft, logicalTop, logicalRight, logicalBottom); - if (logicalBoundsChanged) { - drawView.setLogicalBounds(logicalLeft, logicalTop, logicalRight, logicalBottom); - } - - // It is very important that we unset this, as our spec is that newly created DrawViews are - // handled differently by the FlatViewGroup. This is needed because updateBoundsAndFreeze - // uses .clone(), so we maintain the previous state. - drawView.mWasMounted = false; + // See if we can reuse the draw view. + if (boundsMatch && clipBoundsMatch && clipRadiusMatch && logicalBoundsMatch) { + return this; } + + DrawView drawView = (DrawView) mutableCopy(); + + if (!boundsMatch) { + drawView.setBounds(left, top, right, bottom); + } + + if (!clipBoundsMatch) { + drawView.setClipBounds(clipLeft, clipTop, clipRight, clipBottom); + } + + if (!logicalBoundsMatch) { + drawView.setLogicalBounds(logicalLeft, logicalTop, logicalRight, logicalBottom); + } + + if (!clipRadiusMatch || !boundsMatch) { + // If the bounds change, we need to update the clip path. + drawView.setClipRadius(clipRadius); + } + + // It is very important that we unset this, as our spec is that newly created DrawViews + // are handled differently by the FlatViewGroup. This is needed because clone() maintains + // the previous state. + drawView.mWasMounted = false; + + drawView.freeze(); + return drawView; } @@ -143,6 +148,22 @@ import android.graphics.RectF; } } + /** + * Set the clip radius. Should only be called when the clip radius is first set or when it + * changes, in order to avoid extra work. + * + * @param clipRadius The new clip radius. + */ + void setClipRadius(float clipRadius) { + mClipRadius = clipRadius; + if (clipRadius > MINIMUM_ROUNDED_CLIPPING_VALUE) { + // update the path that we'll clip based on + updateClipPath(); + } else { + mPath = null; + } + } + /** * Update the path with which we'll clip this view */ 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 d41b90b77..d65c371f3 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatShadowNode.java @@ -503,7 +503,7 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; float clipRight, float clipBottom) { Assertions.assumeNotNull(mDrawView); - if (mDrawView.reactTag == 0) { + if (mDrawView == EMPTY_DRAW_VIEW) { // This is the first time we have collected this DrawView, but we have to create a new // DrawView anyway, as reactTag is final, and our DrawView instance is the static copy. mDrawView = new DrawView(getReactTag());