From 371f25a9116c74f5ed8ab58b2e993ebc1b3a507e Mon Sep 17 00:00:00 2001 From: Andy Street Date: Wed, 22 Feb 2017 09:56:52 -0800 Subject: [PATCH] Only call onLayout when layout has actually changed Summary: Developers are complaining about horrible lag (https://github.com/facebook/react-native/issues/11809) caused by PR https://github.com/facebook/react-native/pull/11222. The issue was that hasNewLayout in yoga is actually a dirty bit and indicates that either you OR one of your children has a new layout. We still need to manually check whether the component's layout actually is different from before. Reviewed By: sahrens Differential Revision: D4597545 fbshipit-source-id: 27d4605afd00badfdcdacae740ee2e477adee929 --- .../react/tests/LayoutEventsTestCase.java | 4 +- .../src/androidTest/js/LayoutEventsTestApp.js | 42 +++++++++++++++- .../react/uimanager/ReactShadowNode.java | 50 +++++++++++++------ 3 files changed, 80 insertions(+), 16 deletions(-) diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/tests/LayoutEventsTestCase.java b/ReactAndroid/src/androidTest/java/com/facebook/react/tests/LayoutEventsTestCase.java index 6e201641e7a505..fbd94d6d732751 100644 --- a/ReactAndroid/src/androidTest/java/com/facebook/react/tests/LayoutEventsTestCase.java +++ b/ReactAndroid/src/androidTest/java/com/facebook/react/tests/LayoutEventsTestCase.java @@ -28,8 +28,10 @@ protected String getReactApplicationKeyUnderTest() { * Creates a UI in JS and verifies the onLayout handler is called. */ public void testOnLayoutCalled() { - assertEquals(1, mStringRecordingModule.getCalls().size()); + assertEquals(3, mStringRecordingModule.getCalls().size()); assertEquals("10,10-100x100", mStringRecordingModule.getCalls().get(0)); + assertEquals("10,10-50x50", mStringRecordingModule.getCalls().get(1)); + assertEquals("0,0-50x50", mStringRecordingModule.getCalls().get(2)); } @Override diff --git a/ReactAndroid/src/androidTest/js/LayoutEventsTestApp.js b/ReactAndroid/src/androidTest/js/LayoutEventsTestApp.js index 06ba34e17245dc..9b354761edc4e9 100644 --- a/ReactAndroid/src/androidTest/js/LayoutEventsTestApp.js +++ b/ReactAndroid/src/androidTest/js/LayoutEventsTestApp.js @@ -16,18 +16,58 @@ var View = require('View'); var RecordingModule = require('NativeModules').Recording; +const LAYOUT_SPECS = [ + [10, 10, 100, 100], + [10, 10, 50, 50], + [0, 0, 50, 50], + [0, 0, 50, 50], +]; + class LayoutEventsTestApp extends React.Component { + + constructor() { + super(); + this.state = { + specNumber: 0, + }; + this.numParentLayouts = 0; + } + handleOnLayout = (e) => { var layout = e.nativeEvent.layout; RecordingModule.record(layout.x + ',' + layout.y + '-' + layout.width + 'x' + layout.height); + + if (this.state.specNumber >= LAYOUT_SPECS.length) { + // This will cause the test to fail + RecordingModule.record('Got an extraneous layout call'); + } else { + this.setState({ + specNumber: this.state.specNumber + 1, + }); + } + }; + + handleParentOnLayout = (e) => { + if (this.numParentLayouts > 0) { + // This will cause the test to fail - the parent's layout doesn't change + // so we should only get the event once. + RecordingModule.record('Got an extraneous layout call on the parent'); + } + this.numParentLayouts++; }; render() { + const layout = LAYOUT_SPECS[this.state.specNumber]; return ( + + style={{left: layout[0], top: layout[1], width: layout[2], height: layout[3]}}/> + ); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNode.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNode.java index cfe84b969cfcad..6bcfa50a16f9e2 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNode.java @@ -69,10 +69,10 @@ public class ReactShadowNode { private int mTotalNativeChildren = 0; private @Nullable ReactShadowNode mNativeParent; private @Nullable ArrayList mNativeChildren; - private float mAbsoluteLeft; - private float mAbsoluteTop; - private float mAbsoluteRight; - private float mAbsoluteBottom; + private int mScreenX; + private int mScreenY; + private int mScreenWidth; + private int mScreenHeight; private final Spacing mDefaultPadding = new Spacing(0); private final float[] mPadding = new float[Spacing.ALL + 1]; private final boolean[] mPaddingIsPercent = new boolean[Spacing.ALL + 1]; @@ -292,12 +292,34 @@ public void onCollectExtraUpdates(UIViewOperationQueue uiViewOperationQueue) { } if (hasNewLayout()) { - mAbsoluteLeft = Math.round(absoluteX + getLayoutX()); - mAbsoluteTop = Math.round(absoluteY + getLayoutY()); - mAbsoluteRight = Math.round(absoluteX + getLayoutX() + getLayoutWidth()); - mAbsoluteBottom = Math.round(absoluteY + getLayoutY() + getLayoutHeight()); - nativeViewHierarchyOptimizer.handleUpdateLayout(this); - return true; + float layoutX = getLayoutX(); + float layoutY = getLayoutY(); + int newAbsoluteLeft = Math.round(absoluteX + layoutX); + int newAbsoluteTop = Math.round(absoluteY + layoutY); + int newAbsoluteRight = Math.round(absoluteX + layoutX + getLayoutWidth()); + int newAbsoluteBottom = Math.round(absoluteY + layoutY + getLayoutHeight()); + + int newScreenX = Math.round(layoutX); + int newScreenY = Math.round(layoutY); + int newScreenWidth = newAbsoluteRight - newAbsoluteLeft; + int newScreenHeight = newAbsoluteBottom - newAbsoluteTop; + + boolean layoutHasChanged = + newScreenX != mScreenX || + newScreenY != mScreenY || + newScreenWidth != mScreenWidth || + newScreenHeight != mScreenHeight; + + mScreenX = newScreenX; + mScreenY = newScreenY; + mScreenWidth = newScreenWidth; + mScreenHeight = newScreenHeight; + + if (layoutHasChanged) { + nativeViewHierarchyOptimizer.handleUpdateLayout(this); + } + + return layoutHasChanged; } else { return false; } @@ -488,28 +510,28 @@ public final float getLayoutHeight() { * @return the x position of the corresponding view on the screen, rounded to pixels */ public int getScreenX() { - return Math.round(getLayoutX()); + return mScreenX; } /** * @return the y position of the corresponding view on the screen, rounded to pixels */ public int getScreenY() { - return Math.round(getLayoutY()); + return mScreenY; } /** * @return width corrected for rounding to pixels. */ public int getScreenWidth() { - return Math.round(mAbsoluteRight - mAbsoluteLeft); + return mScreenWidth; } /** * @return height corrected for rounding to pixels. */ public int getScreenHeight() { - return Math.round(mAbsoluteBottom - mAbsoluteTop); + return mScreenHeight; } public final YogaDirection getLayoutDirection() {