Skip to content

Commit

Permalink
Only call onLayout when layout has actually changed
Browse files Browse the repository at this point in the history
Summary:
Developers are complaining about horrible lag (#11809) caused by PR #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
  • Loading branch information
astreet authored and facebook-github-bot committed Feb 22, 2017
1 parent 91b2dbb commit 15429e3
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
42 changes: 41 additions & 1 deletion ReactAndroid/src/androidTest/js/LayoutEventsTestApp.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<View
onLayout={this.handleParentOnLayout}
testID="parent"
style={{left: 0, top: 0, width: 500, height: 500}}>
<View
onLayout={this.handleOnLayout}
testID="container"
style={{left: 10, top: 10, width: 100, height: 100}}/>
style={{left: layout[0], top: layout[1], width: layout[2], height: layout[3]}}/>
</View>
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ public class ReactShadowNode {
private int mTotalNativeChildren = 0;
private @Nullable ReactShadowNode mNativeParent;
private @Nullable ArrayList<ReactShadowNode> 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];
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit 15429e3

Please sign in to comment.