From 9d67989001c673daf18dcf2780126341c246dc45 Mon Sep 17 00:00:00 2001 From: Ahmed El-Helw Date: Fri, 13 May 2016 17:49:38 -0700 Subject: [PATCH] Don't clip overflowing Nodes Summary: As of D3235050, Nodes supports the optimization of removing clipped subviews from the hierarchy. However, because Nodes supports overflow:visible, this could cause issues when DrawCommands overflow the bounds of their parent container. This patch fixes this by not clipping any overflowing Nodes. Reviewed By: astreet Differential Revision: D3235072 --- .../flat/FlatNativeViewHierarchyManager.java | 5 +- .../facebook/react/flat/FlatShadowNode.java | 55 +++++++++++++++++++ .../react/flat/FlatUIViewOperationQueue.java | 18 ++++-- .../facebook/react/flat/FlatViewGroup.java | 6 +- .../com/facebook/react/flat/StateBuilder.java | 7 ++- 5 files changed, 83 insertions(+), 8 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 3ce3947da3..0403cc29a9 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatNativeViewHierarchyManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatNativeViewHierarchyManager.java @@ -64,10 +64,11 @@ import com.facebook.react.uimanager.ViewManagerRegistry; int reactTag, @Nullable DrawCommand[] drawCommands, @Nullable AttachDetachListener[] listeners, - @Nullable NodeRegion[] nodeRegions) { + @Nullable NodeRegion[] nodeRegions, + boolean hasOverflowingElements) { FlatViewGroup view = (FlatViewGroup) resolveView(reactTag); if (drawCommands != null) { - view.mountDrawCommands(drawCommands); + view.mountDrawCommands(drawCommands, hasOverflowingElements); } 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 139b2b2745..8483706f0f 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatShadowNode.java @@ -60,6 +60,11 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; private float mClipRight; private float mClipBottom; + // Used to track whether any of the NodeRegions overflow this Node. This is used to determine + // whether or not we can detach this Node in the context of a container with + // setRemoveClippedSubviews enabled. + private boolean mOverflowsContainer; + // last OnLayoutEvent info, only used when shouldNotifyOnLayout() is true. private int mLayoutX; private int mLayoutY; @@ -120,6 +125,11 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; @ReactProp(name = "overflow") public final void setOverflow(String overflow) { mClipToBounds = "hidden".equals(overflow); + if (mClipToBounds) { + mOverflowsContainer = false; + } else { + updateOverflowsContainer(); + } invalidate(); } @@ -255,6 +265,51 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; /* package */ final void setNodeRegions(NodeRegion[] nodeRegion) { mNodeRegions = nodeRegion; + updateOverflowsContainer(); + } + + /* package */ final void updateOverflowsContainer() { + boolean overflowsContainer = false; + int width = (int) (mNodeRegion.mRight - mNodeRegion.mLeft); + int height = (int) (mNodeRegion.mBottom - mNodeRegion.mTop); + if (!mClipToBounds && height > 0 && width > 0) { + for (NodeRegion region : mNodeRegions) { + if (region.mBottom - region.mTop > height || region.mRight - region.mLeft > width) { + overflowsContainer = true; + break; + } + } + } + + // if we don't overflow, let's check if any of the immediate children overflow. + // this is "indirectly recursive," since this method is called when setNodeRegions is called, + // and the children call setNodeRegions before their parent. consequently, when a node deep + // inside the tree overflows, its immediate parent has mOverflowsContainer set to true, and, + // 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) { + int children = getChildCount(); + for (int i = 0; i < children; i++) { + ReactShadowNode node = getChildAt(i); + if (node instanceof FlatShadowNode && ((FlatShadowNode) node).mOverflowsContainer) { + overflowsContainer = true; + break; + } + } + } + + // if things changed, notify the parent(s) about said changes - while in many cases, this will + // be extra work (since we process this for the parents after the children), in some cases, + // we may have no new node regions in the parent, but have a new node region in the child, and, + // as a result, the parent may not get the correct value for overflows container. + if (mOverflowsContainer != overflowsContainer) { + mOverflowsContainer = overflowsContainer; + } + } + + /* package */ final boolean getOverflowsContainer() { + return mOverflowsContainer; } /* package */ void updateNodeRegion( 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 5498573590..cc2fef85f3 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIViewOperationQueue.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIViewOperationQueue.java @@ -44,16 +44,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 UpdateMountState( int reactTag, @Nullable DrawCommand[] drawCommands, @Nullable AttachDetachListener[] listeners, - @Nullable NodeRegion[] nodeRegions) { + @Nullable NodeRegion[] nodeRegions, + boolean hasOverflowingElements) { mReactTag = reactTag; mDrawCommands = drawCommands; mAttachDetachListeners = listeners; mNodeRegions = nodeRegions; + mHasOverflowingElements = hasOverflowingElements; } @Override @@ -62,7 +65,8 @@ import com.facebook.react.uimanager.UIViewOperationQueue; mReactTag, mDrawCommands, mAttachDetachListeners, - mNodeRegions); + mNodeRegions, + mHasOverflowingElements); } } @@ -234,8 +238,14 @@ import com.facebook.react.uimanager.UIViewOperationQueue; int reactTag, @Nullable DrawCommand[] drawCommands, @Nullable AttachDetachListener[] listeners, - @Nullable NodeRegion[] nodeRegions) { - enqueueUIOperation(new UpdateMountState(reactTag, drawCommands, listeners, nodeRegions)); + @Nullable NodeRegion[] nodeRegions, + boolean hasOverflowingElements) { + enqueueUIOperation(new UpdateMountState( + reactTag, + drawCommands, + listeners, + nodeRegions, + hasOverflowingElements)); } 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 4ef0bf818f..ed23daade6 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java @@ -100,6 +100,8 @@ 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; /* package */ FlatViewGroup(Context context) { super(context); @@ -390,8 +392,9 @@ import com.facebook.react.views.view.ReactClippingViewGroupHelper; ++mDrawChildIndex; } - /* package */ void mountDrawCommands(DrawCommand[] drawCommands) { + /* package */ void mountDrawCommands(DrawCommand[] drawCommands, boolean hasOverflowingElements) { mDrawCommands = drawCommands; + mHasOverflowingElements = hasOverflowingElements; if (mRemoveClippedSubviews) { mDrawViewMap.clear(); for (DrawCommand drawCommand : mDrawCommands) { @@ -597,6 +600,7 @@ 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(), 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 f2cc1b046c..a9f6fb434c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java @@ -325,6 +325,10 @@ import com.facebook.react.uimanager.events.EventDispatcher; if (nodeRegions != null) { shouldUpdateMountState = true; node.setNodeRegions(nodeRegions); + } else if (descendantUpdated) { + // one of the descendant's value for overflows container may have changed, so + // we still need to update ours. + node.updateOverflowsContainer(); } if (shouldUpdateMountState) { @@ -332,7 +336,8 @@ import com.facebook.react.uimanager.events.EventDispatcher; node.getReactTag(), drawCommands, listeners, - nodeRegions); + nodeRegions, + node.getOverflowsContainer()); } if (node.hasUnseenUpdates()) {