From e1ece03593490af91c6a55f6ef62efea6773ee2b Mon Sep 17 00:00:00 2001 From: Ahmed El-Helw Date: Thu, 9 Jun 2016 13:45:14 -0700 Subject: [PATCH] Improve removeClippedSubviews for overflowing views Summary: Historically, removeClippedSubviews for Nodes would not clip views that overflowed their parent. This patch changes that, so that Nodes can properly clip views when they are off screen (even if they have descendants that overflow the bounds of their parent). This is done by calculating a set of offsets from the actual width and height of the view, and using those in the clipping calculations. Reviewed By: sriramramani Differential Revision: D3409859 --- .../flat/FlatNativeViewHierarchyManager.java | 5 +- .../facebook/react/flat/FlatShadowNode.java | 69 +++++++++++++++++-- .../react/flat/FlatUIViewOperationQueue.java | 14 ++-- .../facebook/react/flat/FlatViewGroup.java | 27 ++++---- .../com/facebook/react/flat/StateBuilder.java | 2 +- 5 files changed, 88 insertions(+), 29 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatNativeViewHierarchyManager.java b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatNativeViewHierarchyManager.java index 4db2d180e..9127723d9 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatNativeViewHierarchyManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatNativeViewHierarchyManager.java @@ -13,6 +13,7 @@ import javax.annotation.Nullable; import java.util.Collection; +import android.graphics.Rect; import android.view.View; import android.view.View.MeasureSpec; import android.view.ViewGroup; @@ -67,10 +68,10 @@ import com.facebook.react.uimanager.ViewManagerRegistry; @Nullable DrawCommand[] drawCommands, @Nullable AttachDetachListener[] listeners, @Nullable NodeRegion[] nodeRegions, - boolean hasOverflowingElements) { + Rect logicalAdjustment) { FlatViewGroup view = (FlatViewGroup) resolveView(reactTag); if (drawCommands != null) { - view.mountDrawCommands(drawCommands, hasOverflowingElements); + view.mountDrawCommands(drawCommands, logicalAdjustment); } if (listeners != null) { view.mountAttachDetachListeners(listeners); 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 6b1016a90..10ee7fb93 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,8 @@ package com.facebook.react.flat; import javax.annotation.Nullable; +import android.graphics.Rect; + import com.facebook.csslayout.CSSNode; import com.facebook.infer.annotation.Assertions; import com.facebook.react.uimanager.LayoutShadowNode; @@ -40,6 +42,7 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; private static final String PROP_TRANSFORM = "transform"; private static final String PROP_REMOVE_CLIPPED_SUBVIEWS = ReactClippingViewGroupHelper.PROP_REMOVE_CLIPPED_SUBVIEWS; + private static final Rect LOGICAL_OFFSET_EMPTY = new Rect(); private DrawCommand[] mDrawCommands = DrawCommand.EMPTY_ARRAY; private AttachDetachListener[] mAttachDetachListeners = AttachDetachListener.EMPTY_ARRAY; @@ -66,6 +69,9 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; // whether or not we can detach this Node in the context of a container with // setRemoveClippedSubviews enabled. private boolean mOverflowsContainer; + // this Rect contains the offset to get the "logical bounds" (i.e. bounds that include taking + // into account overflow visible). + private Rect mLogicalOffset = LOGICAL_OFFSET_EMPTY; // last OnLayoutEvent info, only used when shouldNotifyOnLayout() is true. private int mLayoutX; @@ -246,7 +252,7 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; /** * Sets an array of DrawCommands to perform during the View's draw pass. StateBuilder uses old - * draw commands to compare to new draw commands and see if the View neds to be redrawn. + * draw commands to compare to new draw commands and see if the View needs to be redrawn. */ /* package */ final void setDrawCommands(DrawCommand[] drawCommands) { mDrawCommands = drawCommands; @@ -296,12 +302,48 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; boolean overflowsContainer = false; int width = (int) (mNodeRegion.mRight - mNodeRegion.mLeft); int height = (int) (mNodeRegion.mBottom - mNodeRegion.mTop); + + float leftBound = 0; + float rightBound = width; + float topBound = 0; + float bottomBound = height; + Rect logicalOffset = null; + + // when we are overflow:visible, we try to figure out if any of the children are outside + // of the bounds of this view. since NodeRegion bounds are relative to their parent (i.e. + // 0, 0 is always the start), we see how much outside of the bounds we are (negative left + // or top, or bottom that's more than height or right that's more than width). we set these + // offsets in mLogicalOffset for being able to more intelligently determine whether or not + // to clip certain subviews. if (!mClipToBounds && height > 0 && width > 0) { for (NodeRegion region : mNodeRegions) { - if (region.mBottom - region.mTop > height || region.mRight - region.mLeft > width) { + if (region.mLeft < leftBound) { + leftBound = region.mLeft; overflowsContainer = true; - break; } + + if (region.mRight > rightBound) { + rightBound = region.mRight; + overflowsContainer = true; + } + + if (region.mTop < topBound) { + topBound = region.mTop; + overflowsContainer = true; + } + + if (region.mBottom > bottomBound) { + bottomBound = region.mBottom; + overflowsContainer = true; + } + } + + if (overflowsContainer) { + logicalOffset = new Rect( + (int) leftBound, + (int) topBound, + (int) (rightBound - width), + (int) (bottomBound - height)); } } @@ -312,13 +354,24 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; // by extension, so do all of its ancestors, sufficing here to only check the immediate // child's mOverflowsContainer value instead of recursively asking if each child overflows its // container. - if (!overflowsContainer) { + if (!overflowsContainer && mNodeRegion != NodeRegion.EMPTY) { int children = getChildCount(); for (int i = 0; i < children; i++) { ReactShadowNode node = getChildAt(i); if (node instanceof FlatShadowNode && ((FlatShadowNode) node).mOverflowsContainer) { + Rect childLogicalOffset = ((FlatShadowNode) node).mLogicalOffset; + if (logicalOffset == null) { + logicalOffset = new Rect(); + } + // TODO: t11674025 - improve this - a grandparent may end up having smaller logical + // bounds than its children (because the grandparent's size may be larger than that of + // its child, so the grandchild overflows its parent but not its grandparent). currently, + // if a 100x100 view has a 5x5 view, and inside it has a 10x10 view, the inner most view + // overflows its parent but not its grandparent - the logical bounds on the grandparent + // will still be 5x5 (because they're inherited from the child's logical bounds). this + // has the effect of causing us to clip 5px later than we really have to. + logicalOffset.union(childLogicalOffset); overflowsContainer = true; - break; } } } @@ -329,11 +382,12 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; // as a result, the parent may not get the correct value for overflows container. if (mOverflowsContainer != overflowsContainer) { mOverflowsContainer = overflowsContainer; + mLogicalOffset = logicalOffset == null ? LOGICAL_OFFSET_EMPTY : logicalOffset; } } - /* package */ final boolean getOverflowsContainer() { - return mOverflowsContainer; + /* package */ final Rect getLogicalOffset() { + return mLogicalOffset; } /* package */ void updateNodeRegion( @@ -350,6 +404,7 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; protected final void setNodeRegion(NodeRegion nodeRegion) { mNodeRegion = nodeRegion; + updateOverflowsContainer(); } /* package */ final NodeRegion getNodeRegion() { diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIViewOperationQueue.java b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIViewOperationQueue.java index cc2fef85f..00d4b2554 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIViewOperationQueue.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIViewOperationQueue.java @@ -11,6 +11,8 @@ package com.facebook.react.flat; import javax.annotation.Nullable; +import android.graphics.Rect; + import com.facebook.react.bridge.Callback; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.uimanager.NoSuchNativeViewException; @@ -44,19 +46,19 @@ import com.facebook.react.uimanager.UIViewOperationQueue; private final @Nullable DrawCommand[] mDrawCommands; private final @Nullable AttachDetachListener[] mAttachDetachListeners; private final @Nullable NodeRegion[] mNodeRegions; - private final boolean mHasOverflowingElements; + private final Rect mLogicalAdjustment; private UpdateMountState( int reactTag, @Nullable DrawCommand[] drawCommands, @Nullable AttachDetachListener[] listeners, @Nullable NodeRegion[] nodeRegions, - boolean hasOverflowingElements) { + Rect logicalAdjustment) { mReactTag = reactTag; mDrawCommands = drawCommands; mAttachDetachListeners = listeners; mNodeRegions = nodeRegions; - mHasOverflowingElements = hasOverflowingElements; + mLogicalAdjustment = logicalAdjustment; } @Override @@ -66,7 +68,7 @@ import com.facebook.react.uimanager.UIViewOperationQueue; mDrawCommands, mAttachDetachListeners, mNodeRegions, - mHasOverflowingElements); + mLogicalAdjustment); } } @@ -239,13 +241,13 @@ import com.facebook.react.uimanager.UIViewOperationQueue; @Nullable DrawCommand[] drawCommands, @Nullable AttachDetachListener[] listeners, @Nullable NodeRegion[] nodeRegions, - boolean hasOverflowingElements) { + Rect logicalOffset) { enqueueUIOperation(new UpdateMountState( reactTag, drawCommands, listeners, nodeRegions, - hasOverflowingElements)); + logicalOffset)); } public void enqueueUpdateViewGroup(int reactTag, int[] viewsToAdd, int[] viewsToDetach) { 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 e7811bbae..0bfb8d2db 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java @@ -82,6 +82,7 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; private static final ArrayList LAYOUT_REQUESTS = new ArrayList<>(); private static final Rect VIEW_BOUNDS = new Rect(); + private static final Rect EMPTY_RECT = new Rect(); private @Nullable InvalidateCallback mInvalidateCallback; private DrawCommand[] mDrawCommands = DrawCommand.EMPTY_ARRAY; @@ -101,8 +102,9 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; // lookups in o(1) instead of o(log n) - trade space for time private final Map mDrawViewMap = new HashMap<>(); private final Map mClippedSubviews = new HashMap<>(); - // whether or not this FlatViewGroup has elements that overflow its bounds - private boolean mHasOverflowingElements; + // for overflow visible, these adjustments are what we can apply to know the actual bounds of + // a ViewGroup while taking overflowing elements into account. + private Rect mLogicalAdjustments = EMPTY_RECT; /* package */ FlatViewGroup(Context context) { super(context); @@ -393,9 +395,9 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; ++mDrawChildIndex; } - /* package */ void mountDrawCommands(DrawCommand[] drawCommands, boolean hasOverflowingElements) { + /* package */ void mountDrawCommands(DrawCommand[] drawCommands, Rect logicalAdjustments) { mDrawCommands = drawCommands; - mHasOverflowingElements = hasOverflowingElements; + mLogicalAdjustments = logicalAdjustments; if (mRemoveClippedSubviews) { mDrawViewMap.clear(); for (DrawCommand drawCommand : mDrawCommands) { @@ -602,10 +604,10 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; if (flatViewGroup != null) { // invisible if (clippingRect.intersects( - flatViewGroup.getLeft(), - flatViewGroup.getTop(), - flatViewGroup.getRight(), - flatViewGroup.getBottom())) { + flatViewGroup.getLeft() + flatViewGroup.mLogicalAdjustments.left, + flatViewGroup.getTop() + flatViewGroup.mLogicalAdjustments.top, + flatViewGroup.getRight() + flatViewGroup.mLogicalAdjustments.right, + flatViewGroup.getBottom() + flatViewGroup.mLogicalAdjustments.bottom)) { // now on the screen attachViewToParent( flatViewGroup, @@ -623,12 +625,11 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; Animation animation = flatChildView.getAnimation(); boolean isAnimating = animation != null && !animation.hasEnded(); if (!isAnimating && - !flatChildView.mHasOverflowingElements && !clippingRect.intersects( - view.getLeft(), - view.getTop(), - view.getRight(), - view.getBottom())) { + view.getLeft() + flatChildView.mLogicalAdjustments.left, + view.getTop() + flatChildView.mLogicalAdjustments.top, + view.getRight() + flatChildView.mLogicalAdjustments.right, + view.getBottom() + flatChildView.mLogicalAdjustments.bottom)) { // now off the screen mClippedSubviews.put(view.getId(), flatChildView); detachViewFromParent(view); 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 a9f6fb434..167d4cf22 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java @@ -337,7 +337,7 @@ import com.facebook.react.uimanager.events.EventDispatcher; drawCommands, listeners, nodeRegions, - node.getOverflowsContainer()); + node.getLogicalOffset()); } if (node.hasUnseenUpdates()) {