Skip to content

Commit

Permalink
Don't transfer layout outputs to java for unset edges
Browse files Browse the repository at this point in the history 
Summary:
See facebook/yoga#483. We should not transfer the layout if the layout didn't change. (using ```hasNewLayout```). This also changes that the lock on the java node is only aquired if needed, and it holds the lock only for the time the values are set and not for the time all it's children are set.
Closes facebook/yoga#484

Reviewed By: astreet

Differential Revision: D4802966

Pulled By: emilsjolander

fbshipit-source-id: e8a8f2280ad6b25b98fc68b07eac68e0ec80fe3e
  • Loading branch information
@woehrl01 @facebook-github-bot
woehrl01 authored and facebook-github-bot committed Apr 1, 2017
1 parent 67ac4e8 commit ca6f662
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 28 deletions.
45 changes: 30 additions & 15 deletions lib/yoga/yoga/YogaNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,14 @@ public static void setLogger(YogaLogger logger) {
private long mNativePointer;
private Object mData;

private boolean mHasSetPadding = false;
private boolean mHasSetMargin = false;
private boolean mHasSetBorder = false;
/* Those flags needs be in sync with YGJNI.cpp */
private final static int MARGIN = 1;
private final static int PADDING = 2;
private final static int BORDER = 4;

@DoNotStrip
private int mEdgeSetFlag = 0;

private boolean mHasSetPosition = false;

@DoNotStrip
Expand Down Expand Up @@ -113,16 +118,26 @@ protected void finalize() throws Throwable {
private native void jni_YGNodeReset(long nativePointer);
@Override
public void reset() {
mHasSetPadding = false;
mHasSetMargin = false;
mHasSetBorder = false;
mEdgeSetFlag = 0;
mHasSetPosition = false;
mHasNewLayout = true;

mWidth = YogaConstants.UNDEFINED;
mHeight = YogaConstants.UNDEFINED;
mTop = YogaConstants.UNDEFINED;
mLeft = YogaConstants.UNDEFINED;
mMarginLeft = 0;
mMarginTop = 0;
mMarginRight = 0;
mMarginBottom = 0;
mPaddingLeft = 0;
mPaddingTop = 0;
mPaddingRight = 0;
mPaddingBottom = 0;
mBorderLeft = 0;
mBorderTop = 0;
mBorderRight = 0;
mBorderBottom = 0;
mLayoutDirection = 0;

mMeasureFunction = null;
Expand Down Expand Up @@ -382,7 +397,7 @@ public void setFlexBasisAuto() {
private native Object jni_YGNodeStyleGetMargin(long nativePointer, int edge);
@Override
public YogaValue getMargin(YogaEdge edge) {
if (!mHasSetMargin) {
if (!((mEdgeSetFlag & MARGIN) == MARGIN)) {
return YogaValue.UNDEFINED;
}
return (YogaValue) jni_YGNodeStyleGetMargin(mNativePointer, edge.intValue());
Expand All @@ -391,28 +406,28 @@ public YogaValue getMargin(YogaEdge edge) {
private native void jni_YGNodeStyleSetMargin(long nativePointer, int edge, float margin);
@Override
public void setMargin(YogaEdge edge, float margin) {
mHasSetMargin = true;
mEdgeSetFlag |= MARGIN;
jni_YGNodeStyleSetMargin(mNativePointer, edge.intValue(), margin);
}

private native void jni_YGNodeStyleSetMarginPercent(long nativePointer, int edge, float percent);
@Override
public void setMarginPercent(YogaEdge edge, float percent) {
mHasSetMargin = true;
mEdgeSetFlag |= MARGIN;
jni_YGNodeStyleSetMarginPercent(mNativePointer, edge.intValue(), percent);
}

private native void jni_YGNodeStyleSetMarginAuto(long nativePointer, int edge);
@Override
public void setMarginAuto(YogaEdge edge) {
mHasSetMargin = true;
mEdgeSetFlag |= MARGIN;
jni_YGNodeStyleSetMarginAuto(mNativePointer, edge.intValue());
}

private native Object jni_YGNodeStyleGetPadding(long nativePointer, int edge);
@Override
public YogaValue getPadding(YogaEdge edge) {
if (!mHasSetPadding) {
if (!((mEdgeSetFlag & PADDING) == PADDING)) {
return YogaValue.UNDEFINED;
}
return (YogaValue) jni_YGNodeStyleGetPadding(mNativePointer, edge.intValue());
Expand All @@ -421,21 +436,21 @@ public YogaValue getPadding(YogaEdge edge) {
private native void jni_YGNodeStyleSetPadding(long nativePointer, int edge, float padding);
@Override
public void setPadding(YogaEdge edge, float padding) {
mHasSetPadding = true;
mEdgeSetFlag |= PADDING;
jni_YGNodeStyleSetPadding(mNativePointer, edge.intValue(), padding);
}

private native void jni_YGNodeStyleSetPaddingPercent(long nativePointer, int edge, float percent);
@Override
public void setPaddingPercent(YogaEdge edge, float percent) {
mHasSetPadding = true;
mEdgeSetFlag |= PADDING;
jni_YGNodeStyleSetPaddingPercent(mNativePointer, edge.intValue(), percent);
}

private native float jni_YGNodeStyleGetBorder(long nativePointer, int edge);
@Override
public float getBorder(YogaEdge edge) {
if (!mHasSetBorder) {
if (!((mEdgeSetFlag & BORDER) == BORDER)) {
return YogaConstants.UNDEFINED;
}
return jni_YGNodeStyleGetBorder(mNativePointer, edge.intValue());
Expand All @@ -444,7 +459,7 @@ public float getBorder(YogaEdge edge) {
private native void jni_YGNodeStyleSetBorder(long nativePointer, int edge, float border);
@Override
public void setBorder(YogaEdge edge, float border) {
mHasSetBorder = true;
mEdgeSetFlag |= BORDER;
jni_YGNodeStyleSetBorder(mNativePointer, edge.intValue(), border);
}

Expand Down
39 changes: 27 additions & 12 deletions lib/yogajni/jni/YGJNI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,31 +46,46 @@ static void YGTransferLayoutOutputsRecursive(YGNodeRef root) {
static auto borderRightField = obj->getClass()->getField<jfloat>("mBorderRight");
static auto borderBottomField = obj->getClass()->getField<jfloat>("mBorderBottom");

static auto edgeSetFlagField = obj->getClass()->getField<jint>("mEdgeSetFlag");
static auto hasNewLayoutField = obj->getClass()->getField<jboolean>("mHasNewLayout");

/* Those flags needs be in sync with YogaNode.java */
const int MARGIN = 1;
const int PADDING = 2;
const int BORDER = 4;

int hasEdgeSetFlag = (int)obj->getFieldValue(edgeSetFlagField);

obj->setFieldValue(widthField, YGNodeLayoutGetWidth(root));
obj->setFieldValue(heightField, YGNodeLayoutGetHeight(root));
obj->setFieldValue(leftField, YGNodeLayoutGetLeft(root));
obj->setFieldValue(topField, YGNodeLayoutGetTop(root));

obj->setFieldValue(marginLeftField, YGNodeLayoutGetMargin(root, YGEdgeLeft));
obj->setFieldValue(marginTopField, YGNodeLayoutGetMargin(root, YGEdgeTop));
obj->setFieldValue(marginRightField, YGNodeLayoutGetMargin(root, YGEdgeRight));
obj->setFieldValue(marginBottomField, YGNodeLayoutGetMargin(root, YGEdgeBottom));
if((hasEdgeSetFlag & MARGIN) == MARGIN){
obj->setFieldValue(marginLeftField, YGNodeLayoutGetMargin(root, YGEdgeLeft));
obj->setFieldValue(marginTopField, YGNodeLayoutGetMargin(root, YGEdgeTop));
obj->setFieldValue(marginRightField, YGNodeLayoutGetMargin(root, YGEdgeRight));
obj->setFieldValue(marginBottomField, YGNodeLayoutGetMargin(root, YGEdgeBottom));
}

obj->setFieldValue(paddingLeftField, YGNodeLayoutGetPadding(root, YGEdgeLeft));
obj->setFieldValue(paddingTopField, YGNodeLayoutGetPadding(root, YGEdgeTop));
obj->setFieldValue(paddingRightField, YGNodeLayoutGetPadding(root, YGEdgeRight));
obj->setFieldValue(paddingBottomField, YGNodeLayoutGetPadding(root, YGEdgeBottom));
if((hasEdgeSetFlag & PADDING) == PADDING){
obj->setFieldValue(paddingLeftField, YGNodeLayoutGetPadding(root, YGEdgeLeft));
obj->setFieldValue(paddingTopField, YGNodeLayoutGetPadding(root, YGEdgeTop));
obj->setFieldValue(paddingRightField, YGNodeLayoutGetPadding(root, YGEdgeRight));
obj->setFieldValue(paddingBottomField, YGNodeLayoutGetPadding(root, YGEdgeBottom));
}

obj->setFieldValue(borderLeftField, YGNodeLayoutGetBorder(root, YGEdgeLeft));
obj->setFieldValue(borderTopField, YGNodeLayoutGetBorder(root, YGEdgeTop));
obj->setFieldValue(borderRightField, YGNodeLayoutGetBorder(root, YGEdgeRight));
obj->setFieldValue(borderBottomField, YGNodeLayoutGetBorder(root, YGEdgeBottom));
if((hasEdgeSetFlag & BORDER) == BORDER){
obj->setFieldValue(borderLeftField, YGNodeLayoutGetBorder(root, YGEdgeLeft));
obj->setFieldValue(borderTopField, YGNodeLayoutGetBorder(root, YGEdgeTop));
obj->setFieldValue(borderRightField, YGNodeLayoutGetBorder(root, YGEdgeRight));
obj->setFieldValue(borderBottomField, YGNodeLayoutGetBorder(root, YGEdgeBottom));
}

obj->setFieldValue<jboolean>(hasNewLayoutField, true);
YGTransferLayoutDirection(root, obj);
YGNodeSetHasNewLayout(root, false);

for (uint32_t i = 0; i < YGNodeGetChildCount(root); i++) {
YGTransferLayoutOutputsRecursive(YGNodeGetChild(root, i));
}
Expand Down
3 changes: 2 additions & 1 deletion src/test/java/com/facebook/litho/LayoutStateCalculateTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1937,7 +1937,8 @@ public void testLayoutOutputWithCachedLayoutSpecWithMeasure() {
final Component rootContainer = new InlineLayoutSpec() {
@Override
protected ComponentLayout onCreateLayout(ComponentContext c) {
return Container.create(c).flexDirection(YogaFlexDirection.COLUMN).flexShrink(0).alignContent(YogaAlign.FLEX_START)
return Container.create(c)
.flexDirection(YogaFlexDirection.COLUMN)
.paddingPx(YogaEdge.HORIZONTAL, horizontalPadding)
.child(sizeDependentComponentSpy)
.build();
Expand Down

0 comments on commit ca6f662

Please sign in to comment.