From 6f06f76e38af5a08e45675177157cbd0d5fd4743 Mon Sep 17 00:00:00 2001 From: Denis Koroskin Date: Sun, 20 Dec 2015 20:34:36 -0800 Subject: [PATCH] Implement overflow:visible/hidden attribute (defaults to visible) Summary: Prior to this patch DrawCommands weren't get clipped by parent DrawCommands at all. For example, a element with size 200x200 with overflow:hidden should clip its child which is 400x400, but this didn't happen. However, if parent would mount to an Android View, it would clip the child regardless of the overflow attribute value (because Android Views always clip whatever is drawing inside that View against its boundaries). This diff is fixing these issue, implementing overflow attribute support and making clipping behavior consistent between nodes that mount to View and nodes that don't. Reviewed By: ahmedre Differential Revision: D2768643 --- .../react/flat/AbstractDrawCommand.java | 58 +++++++++- .../react/flat/DrawBackgroundColor.java | 2 +- .../com/facebook/react/flat/DrawBorder.java | 2 +- .../react/flat/DrawImageWithPipeline.java | 14 +-- .../facebook/react/flat/DrawTextLayout.java | 2 +- .../facebook/react/flat/FlatShadowNode.java | 23 +++- .../facebook/react/flat/FlatViewGroup.java | 1 + .../com/facebook/react/flat/RCTImageView.java | 23 +++- .../java/com/facebook/react/flat/RCTText.java | 28 ++++- .../java/com/facebook/react/flat/RCTView.java | 27 ++++- .../com/facebook/react/flat/StateBuilder.java | 104 ++++++++++++++++-- 11 files changed, 249 insertions(+), 35 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 f5375a98d3..bb8b5c6590 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/AbstractDrawCommand.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/AbstractDrawCommand.java @@ -9,6 +9,8 @@ package com.facebook.react.flat; +import android.graphics.Canvas; + /** * Base class for all DrawCommands. Becomes immutable once it has its bounds set. Until then, a * subclass is able to mutate any of its properties (e.g. updating Layout in DrawTextLayout). @@ -22,8 +24,24 @@ package com.facebook.react.flat; 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 + public final void draw(FlatViewGroup parent, Canvas canvas) { + if (shouldClip()) { + canvas.save(); + canvas.clipRect(mClipLeft, mClipTop, mClipRight, mClipBottom); + onDraw(canvas); + canvas.restore(); + } else { + onDraw(canvas); + } + } + /** * Updates boundaries of the AbstractDrawCommand and freezes it. * Will return a frozen copy if the current AbstractDrawCommand cannot be mutated. @@ -32,16 +50,27 @@ package com.facebook.react.flat; float left, float top, float right, - float bottom) { + float bottom, + float clipLeft, + float clipTop, + float clipRight, + float clipBottom) { if (mFrozen) { // see if we can reuse it - if (boundsMatch(left, top, right, bottom)) { + boolean boundsMatch = boundsMatch(left, top, right, bottom); + boolean clipBoundsMatch = clipBoundsMatch(clipLeft, clipTop, clipRight, clipBottom); + if (boundsMatch && clipBoundsMatch) { return this; } try { AbstractDrawCommand copy = (AbstractDrawCommand) clone(); - copy.setBounds(left, top, right, bottom); + if (!boundsMatch) { + copy.setBounds(left, top, right, bottom); + } + if (!clipBoundsMatch) { + copy.setClipBounds(clipLeft, clipTop, clipRight, clipBottom); + } return copy; } catch (CloneNotSupportedException e) { // This should not happen since AbstractDrawCommand implements Cloneable @@ -50,6 +79,7 @@ package com.facebook.react.flat; } setBounds(left, top, right, bottom); + setClipBounds(clipLeft, clipTop, clipRight, clipBottom); mFrozen = true; return this; } @@ -103,6 +133,12 @@ package com.facebook.react.flat; return mBottom; } + protected abstract void onDraw(Canvas canvas); + + protected boolean shouldClip() { + return mLeft != mClipLeft || mTop != mClipTop || mRight != mClipRight || mBottom != mClipBottom; + } + protected void onBoundsChanged() { } @@ -118,10 +154,26 @@ package com.facebook.react.flat; 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/DrawBackgroundColor.java b/ReactAndroid/src/main/java/com/facebook/react/flat/DrawBackgroundColor.java index c2d4215d52..a8e62c5f87 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/DrawBackgroundColor.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/DrawBackgroundColor.java @@ -26,7 +26,7 @@ import android.graphics.Paint; } @Override - public void draw(FlatViewGroup parent, Canvas canvas) { + public void onDraw(Canvas canvas) { PAINT.setColor(mBackgroundColor); canvas.drawRect(getLeft(), getTop(), getRight(), getBottom(), PAINT); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/DrawBorder.java b/ReactAndroid/src/main/java/com/facebook/react/flat/DrawBorder.java index 66d68b5ed9..64ddf555a7 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/DrawBorder.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/DrawBorder.java @@ -169,7 +169,7 @@ import com.facebook.csslayout.Spacing; } @Override - public void draw(FlatViewGroup parent, Canvas canvas) { + protected void onDraw(Canvas canvas) { if (getBorderRadius() >= 0.5f) { drawRoundedBorders(canvas); } else { diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/DrawImageWithPipeline.java b/ReactAndroid/src/main/java/com/facebook/react/flat/DrawImageWithPipeline.java index 6cf23b8bba..f8f36d9eda 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/DrawImageWithPipeline.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/DrawImageWithPipeline.java @@ -80,7 +80,7 @@ import com.facebook.react.views.image.ImageResizeMode; } @Override - public void draw(FlatViewGroup parent, Canvas canvas) { + protected void onDraw(Canvas canvas) { Bitmap bitmap = Assertions.assumeNotNull(mBitmapRequestHelper).getBitmap(); if (bitmap == null) { return; @@ -88,11 +88,6 @@ import com.facebook.react.views.image.ImageResizeMode; PAINT.setColorFilter(mColorFilter); - if (mForceClip) { - canvas.save(); - canvas.clipRect(getLeft(), getTop(), getRight(), getBottom()); - } - if (getBorderRadius() < 0.5f) { canvas.drawBitmap(bitmap, mTransform, PAINT); } else { @@ -105,10 +100,11 @@ import com.facebook.react.views.image.ImageResizeMode; } drawBorders(canvas); + } - if (mForceClip) { - canvas.restore(); - } + @Override + protected boolean shouldClip() { + return mForceClip || super.shouldClip(); } @Override diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/DrawTextLayout.java b/ReactAndroid/src/main/java/com/facebook/react/flat/DrawTextLayout.java index 764a15eef9..18c428e1c7 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/DrawTextLayout.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/DrawTextLayout.java @@ -35,7 +35,7 @@ import android.text.Layout; } @Override - public void draw(FlatViewGroup parent, Canvas canvas) { + protected void onDraw(Canvas canvas) { float left = getLeft(); float top = getTop(); 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 2a0ce10ea7..d32256ed89 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatShadowNode.java @@ -47,6 +47,7 @@ import com.facebook.react.uimanager.ViewProps; private boolean mBackingViewIsCreated; private @Nullable DrawBackgroundColor mDrawBackground; private int mMoveToIndexInParent; + private boolean mIsOverflowVisible = true; /* package */ void handleUpdateProperties(CatalystStylesDiffMap styles) { if (!mountsToView()) { @@ -72,13 +73,21 @@ import com.facebook.react.uimanager.ViewProps; float left, float top, float right, - float bottom) { + float bottom, + float clipLeft, + float clipTop, + float clipRight, + float clipBottom) { if (mDrawBackground != null) { mDrawBackground = (DrawBackgroundColor) mDrawBackground.updateBoundsAndFreeze( left, top, right, - bottom); + bottom, + clipLeft, + clipTop, + clipRight, + clipBottom); stateBuilder.addDrawCommand(mDrawBackground); } } @@ -89,6 +98,16 @@ import com.facebook.react.uimanager.ViewProps; invalidate(); } + @ReactProp(name = "overflow") + public void setOverflow(String overflow) { + mIsOverflowVisible = "visible".equals(overflow); + invalidate(); + } + + public final boolean isOverflowVisible() { + return mIsOverflowVisible; + } + @Override public final int getScreenX() { return mViewLeft; diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java index 55a54b905e..52ab0e772a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java @@ -62,6 +62,7 @@ import com.facebook.react.uimanager.ReactCompoundView; /* package */ FlatViewGroup(Context context) { super(context); + setClipChildren(false); } @Override diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTImageView.java b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTImageView.java index df0f0885f0..445380310b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTImageView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTImageView.java @@ -54,15 +54,32 @@ import com.facebook.react.views.image.ImageResizeMode; float left, float top, float right, - float bottom) { - super.collectState(stateBuilder, left, top, right, bottom); + float bottom, + float clipLeft, + float clipTop, + float clipRight, + float clipBottom) { + super.collectState( + stateBuilder, + left, + top, + right, + bottom, + clipLeft, + clipTop, + clipRight, + clipBottom); if (mDrawImage.hasImageRequest()) { mDrawImage = (T) mDrawImage.updateBoundsAndFreeze( left, top, right, - bottom); + bottom, + clipLeft, + clipTop, + clipRight, + clipBottom); stateBuilder.addDrawCommand(mDrawImage); stateBuilder.addAttachDetachListener(mDrawImage); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java index 1b0a59b2fc..e6413b470d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java @@ -137,8 +137,22 @@ import com.facebook.react.uimanager.ViewProps; float left, float top, float right, - float bottom) { - super.collectState(stateBuilder, left, top, right, bottom); + float bottom, + float clipLeft, + float clipTop, + float clipRight, + float clipBottom) { + + super.collectState( + stateBuilder, + left, + top, + right, + bottom, + clipLeft, + clipRight, + clipTop, + clipBottom); if (mText == null) { // nothing to draw (empty text). @@ -158,7 +172,15 @@ import com.facebook.react.uimanager.ViewProps; INCLUDE_PADDING)); } - mDrawCommand = (DrawTextLayout) mDrawCommand.updateBoundsAndFreeze(left, top, right, bottom); + mDrawCommand = (DrawTextLayout) mDrawCommand.updateBoundsAndFreeze( + left, + top, + right, + bottom, + clipLeft, + clipTop, + clipRight, + clipBottom); stateBuilder.addDrawCommand(mDrawCommand); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTView.java b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTView.java index 97a9310bb2..3722ddb05c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTView.java @@ -27,11 +27,32 @@ import com.facebook.react.uimanager.ViewProps; float left, float top, float right, - float bottom) { - super.collectState(stateBuilder, left, top, right, bottom); + float bottom, + float clipLeft, + float clipTop, + float clipRight, + float clipBottom) { + super.collectState( + stateBuilder, + left, + top, + right, + bottom, + clipLeft, + clipTop, + clipRight, + clipBottom); if (mDrawBorder != null) { - mDrawBorder = (DrawBorder) mDrawBorder.updateBoundsAndFreeze(left, top, right, bottom); + mDrawBorder = (DrawBorder) mDrawBorder.updateBoundsAndFreeze( + left, + top, + right, + bottom, + clipLeft, + clipTop, + clipRight, + clipBottom); stateBuilder.addDrawCommand(mDrawBorder); } } 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 18ca45739e..31e0243f70 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java @@ -61,12 +61,23 @@ import com.facebook.react.uimanager.events.EventDispatcher; float width = node.getLayoutWidth(); float height = node.getLayoutHeight(); - collectStateForMountableNode(node, tag, width, height); - float left = node.getLayoutX(); float top = node.getLayoutY(); float right = left + width; float bottom = top + height; + + collectStateForMountableNode( + node, + tag, + left, + top, + right, + bottom, + Float.NEGATIVE_INFINITY, + Float.NEGATIVE_INFINITY, + Float.POSITIVE_INFINITY, + Float.POSITIVE_INFINITY); + node.updateNodeRegion(left, top, right, bottom); mViewsToUpdateBounds.add(node); @@ -166,8 +177,14 @@ import com.facebook.react.uimanager.events.EventDispatcher; private void collectStateForMountableNode( FlatShadowNode node, int tag, - float width, - float height) { + float left, + float top, + float right, + float bottom, + float clipLeft, + float clipTop, + float clipRight, + float clipBottom) { mDrawCommands.start(node.getDrawCommands()); mAttachDetachListeners.start(node.getAttachDetachListeners()); mNodeRegions.start(node.getNodeRegions()); @@ -181,6 +198,14 @@ import com.facebook.react.uimanager.events.EventDispatcher; isAndroidView = true; needsCustomLayoutForChildren = androidView.needsCustomLayoutForChildren(); + + // AndroidView might scroll (e.g. ScrollView) so we need to reset clip bounds here + // Otherwise, we might scroll clipped content. If AndroidView doesn't scroll, this is still + // harmless, because AndroidView will do its own clipping anyway. + clipLeft = Float.NEGATIVE_INFINITY; + clipTop = Float.NEGATIVE_INFINITY; + clipRight = Float.POSITIVE_INFINITY; + clipBottom = Float.POSITIVE_INFINITY; } else if (node.isVirtualAnchor()) { // If RCTText is mounted to View, virtual children will not receive any touch events // because they don't get added to nodeRegions, so nodeRegions will be empty and @@ -190,7 +215,18 @@ import com.facebook.react.uimanager.events.EventDispatcher; addNodeRegion(node.getNodeRegion()); } - collectStateRecursively(node, 0, 0, width, height, isAndroidView, needsCustomLayoutForChildren); + collectStateRecursively( + node, + left, + top, + right, + bottom, + clipLeft, + clipTop, + clipRight, + clipBottom, + isAndroidView, + needsCustomLayoutForChildren); boolean shouldUpdateMountState = false; final DrawCommand[] drawCommands = mDrawCommands.finish(); @@ -290,6 +326,10 @@ import com.facebook.react.uimanager.events.EventDispatcher; float top, float right, float bottom, + float parentClipLeft, + float parentClipTop, + float parentClipRight, + float parentClipBottom, boolean isAndroidView, boolean needsCustomLayoutForChildren) { if (node.hasNewLayout()) { @@ -307,7 +347,19 @@ import com.facebook.react.uimanager.events.EventDispatcher; Math.round(bottom - top))); } - node.collectState(this, left, top, right, bottom); + float clipLeft = Math.max(left, parentClipLeft); + float clipTop = Math.max(top, parentClipTop); + float clipRight = Math.min(right, parentClipRight); + float clipBottom = Math.min(bottom, parentClipBottom); + + node.collectState(this, left, top, right, bottom, clipLeft, clipTop, clipRight, clipBottom); + + if (node.isOverflowVisible()) { + clipLeft = parentClipLeft; + clipTop = parentClipTop; + clipRight = parentClipRight; + clipBottom = parentClipBottom; + } for (int i = 0, childCount = node.getChildCount(); i != childCount; ++i) { FlatShadowNode child = (FlatShadowNode) node.getChildAt(i); @@ -316,7 +368,16 @@ import com.facebook.react.uimanager.events.EventDispatcher; continue; } - processNodeAndCollectState(child, left, top, isAndroidView, needsCustomLayoutForChildren); + processNodeAndCollectState( + child, + left, + top, + clipLeft, + clipTop, + clipRight, + clipBottom, + isAndroidView, + needsCustomLayoutForChildren); } } @@ -337,6 +398,10 @@ import com.facebook.react.uimanager.events.EventDispatcher; FlatShadowNode node, float parentLeft, float parentTop, + float parentClipLeft, + float parentClipTop, + float parentClipRight, + float parentClipBottom, boolean parentIsAndroidView, boolean needsCustomLayout) { int tag = node.getReactTag(); @@ -359,13 +424,34 @@ import com.facebook.react.uimanager.events.EventDispatcher; mDrawCommands.add(DrawView.INSTANCE); } - collectStateForMountableNode(node, tag, width, height); + collectStateForMountableNode( + node, + tag, + left - left, + top - top, + right - left, + bottom - top, + parentClipLeft - left, + parentClipTop - top, + parentClipRight - left, + parentClipBottom - top); if (!needsCustomLayout) { mViewsToUpdateBounds.add(node); } } else { - collectStateRecursively(node, left, top, right, bottom, false, false); + collectStateRecursively( + node, + left, + top, + right, + bottom, + parentClipLeft, + parentClipTop, + parentClipRight, + parentClipBottom, + false, + false); addNodeRegion(node.getNodeRegion()); } }