From e7d8d2c3ab8d5c11cfae0795c015f2a5db24cc09 Mon Sep 17 00:00:00 2001 From: Denis Koroskin Date: Mon, 11 Jan 2016 20:03:15 -0800 Subject: [PATCH] Don't use NodeRegion to update View bounds Summary: NodeRegions are touch regions within hosting View, and while in most cases they are the same as View boundaries, there is one case where it's not true: TextNodeRegion. When mounted to a View, TextNodeRegion will have a bounds of (0,0,width,height) which is clearly different from (left,top,right,bottom). Initially I assumed they would always be the same so we could use information stored in NodeRegion (should probably be called TouchRegion) to update node's View boundaries, but it breaks RCTTextView when it mount to a View (because it would either contain incorrect bounds, or View will be laid out incorrectly). Right now touch is not working on RCTView that mounts to a View. To fix the issue, separate the 2 concepts. Reviewed By: ahmedre Differential Revision: D2816268 --- .../react/flat/FlatUIViewOperationQueue.java | 15 ++++++-- .../com/facebook/react/flat/StateBuilder.java | 34 +++++++++++-------- 2 files changed, 32 insertions(+), 17 deletions(-) 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 765bde94b88..dce0de70bc0 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIViewOperationQueue.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatUIViewOperationQueue.java @@ -86,7 +86,7 @@ import com.facebook.react.uimanager.UIViewOperationQueue; /** * UIOperation that updates View bounds for a View defined by reactTag. */ - private final class UpdateViewBounds implements UIOperation { + public final class UpdateViewBounds implements UIOperation { private final int mReactTag; private final int mLeft; @@ -234,11 +234,20 @@ import com.facebook.react.uimanager.UIViewOperationQueue; enqueueUIOperation(new UpdateViewGroup(reactTag, viewsToAdd, viewsToDetach)); } + public UpdateViewBounds createUpdateViewBounds( + int reactTag, + int left, + int top, + int right, + int bottom) { + return new UpdateViewBounds(reactTag, left, top, right, bottom); + } + /** * Enqueues a new UIOperation that will update View bounds for a View defined by reactTag. */ - public void enqueueUpdateViewBounds(int reactTag, int left, int top, int right, int bottom) { - enqueueUIOperation(new UpdateViewBounds(reactTag, left, top, right, bottom)); + public void enqueueUpdateViewBounds(UpdateViewBounds updateViewBounds) { + enqueueUIOperation(updateViewBounds); } public void enqueueSetPadding( 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 f75b6655f25..e973dc257ef 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java @@ -42,9 +42,10 @@ import com.facebook.react.uimanager.events.EventDispatcher; private final ArrayList mViewsToDetachAllChildrenFrom = new ArrayList<>(); private final ArrayList mViewsToDetach = new ArrayList<>(); - private final ArrayList mViewsToUpdateBounds = new ArrayList<>(); private final ArrayList mViewsToDrop = new ArrayList<>(); private final ArrayList mOnLayoutEvents = new ArrayList<>(); + private final ArrayList mUpdateViewBoundsOperations = + new ArrayList<>(); private @Nullable FlatUIViewOperationQueue.DetachAllChildrenFromViews mDetachAllChildrenFromViews; @@ -84,7 +85,7 @@ import com.facebook.react.uimanager.events.EventDispatcher; node.updateNodeRegion(left, top, right, bottom); - mViewsToUpdateBounds.add(node); + updateViewBounds(node, left, top, right, bottom); if (mDetachAllChildrenFromViews != null) { int[] viewsToDetachAllChildrenFrom = collectViewTags(mViewsToDetachAllChildrenFrom); @@ -94,10 +95,10 @@ import com.facebook.react.uimanager.events.EventDispatcher; mDetachAllChildrenFromViews = null; } - for (int i = 0, size = mViewsToUpdateBounds.size(); i != size; ++i) { - updateViewBounds(mViewsToUpdateBounds.get(i)); + for (int i = 0, size = mUpdateViewBoundsOperations.size(); i != size; ++i) { + mOperationsQueue.enqueueUpdateViewBounds(mUpdateViewBoundsOperations.get(i)); } - mViewsToUpdateBounds.clear(); + mUpdateViewBoundsOperations.clear(); // This could be more efficient if EventDispatcher had a batch mode // to avoid multiple synchronized calls. @@ -156,13 +157,16 @@ import com.facebook.react.uimanager.events.EventDispatcher; /** * Updates boundaries of a View that a give nodes maps to. */ - private void updateViewBounds(FlatShadowNode node) { - NodeRegion nodeRegion = node.getNodeRegion(); - - int viewLeft = Math.round(nodeRegion.mLeft); - int viewTop = Math.round(nodeRegion.mTop); - int viewRight = Math.round(nodeRegion.mRight); - int viewBottom = Math.round(nodeRegion.mBottom); + private void updateViewBounds( + FlatShadowNode node, + float left, + float top, + float right, + float bottom) { + int viewLeft = Math.round(left); + int viewTop = Math.round(top); + int viewRight = Math.round(right); + int viewBottom = Math.round(bottom); if (node.getViewLeft() == viewLeft && node.getViewTop() == viewTop && node.getViewRight() == viewRight && node.getViewBottom() == viewBottom) { // nothing changed. @@ -172,7 +176,9 @@ import com.facebook.react.uimanager.events.EventDispatcher; // this will optionally measure and layout the View this node maps to. node.setViewBounds(viewLeft, viewTop, viewRight, viewBottom); int tag = node.getReactTag(); - mOperationsQueue.enqueueUpdateViewBounds(tag, viewLeft, viewTop, viewRight, viewBottom); + + mUpdateViewBoundsOperations.add( + mOperationsQueue.createUpdateViewBounds(tag, viewLeft, viewTop, viewRight, viewBottom)); } /** @@ -445,7 +451,7 @@ import com.facebook.react.uimanager.events.EventDispatcher; parentClipBottom - top); if (!needsCustomLayout) { - mViewsToUpdateBounds.add(node); + updateViewBounds(node, left, top, right, bottom); } } else { collectStateRecursively(