From c10bbe559965dfb4c6ba474425620dd632ba7806 Mon Sep 17 00:00:00 2001 From: Emil Sjolander Date: Mon, 24 Oct 2016 10:35:43 -0700 Subject: [PATCH] Dont create a spacing object for returning margin, padding, border, and position Summary: The current implementation was made out of simplicity and to keep the same API as before. Now that the java version of csslayout is deprecated it is time to change the API to make the calls more efficient for the JNI version. This diff with reduce allocations as well as reduce the number of JNI calls done. Differential Revision: D4050773 --- .../com/facebook/react/flat/AndroidView.java | 6 ++---- .../flat/FlatARTSurfaceViewShadowNode.java | 3 ++- .../react/flat/FlatReactModalShadowNode.java | 3 ++- .../react/flat/NativeViewWrapper.java | 3 ++- .../java/com/facebook/react/flat/RCTText.java | 6 ++---- .../com/facebook/react/flat/RCTTextInput.java | 19 +++++++++++++------ .../com/facebook/react/flat/StateBuilder.java | 9 ++++----- 7 files changed, 27 insertions(+), 22 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/AndroidView.java b/ReactAndroid/src/main/java/com/facebook/react/flat/AndroidView.java index a53b2a26f..8fd10bbb8 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/AndroidView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/AndroidView.java @@ -9,8 +9,6 @@ package com.facebook.react.flat; -import com.facebook.csslayout.Spacing; - interface AndroidView { /** @@ -31,7 +29,7 @@ interface AndroidView { void resetPaddingChanged(); /** - * Get this node's padding, as defined by style + default padding. + * Get the padding for a certain spacingType defined in com.facebook.csslayout.Spacing */ - Spacing getPadding(); + float getPadding(int spacingType); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatARTSurfaceViewShadowNode.java b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatARTSurfaceViewShadowNode.java index 8ca4357ce..81476487b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatARTSurfaceViewShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatARTSurfaceViewShadowNode.java @@ -103,7 +103,8 @@ import com.facebook.react.views.art.ARTVirtualNode; @Override public void setPadding(int spacingType, float padding) { - if (getPadding().set(spacingType, padding)) { + if (getPadding(spacingType) != padding) { + setPadding(spacingType, padding); mPaddingChanged = true; dirty(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatReactModalShadowNode.java b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatReactModalShadowNode.java index c145b88da..a33abdd06 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatReactModalShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatReactModalShadowNode.java @@ -86,7 +86,8 @@ class FlatReactModalShadowNode extends FlatShadowNode implements AndroidView { @Override public void setPadding(int spacingType, float padding) { - if (getPadding().set(spacingType, padding)) { + if (getPadding(spacingType) != padding) { + setPadding(spacingType, padding); mPaddingChanged = true; dirty(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/NativeViewWrapper.java b/ReactAndroid/src/main/java/com/facebook/react/flat/NativeViewWrapper.java index aede449bf..4d944b64d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/NativeViewWrapper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/NativeViewWrapper.java @@ -102,7 +102,8 @@ import com.facebook.react.uimanager.ViewManager; @Override public void setPadding(int spacingType, float padding) { - if (getPadding().set(spacingType, padding)) { + if (getPadding(spacingType) != padding) { + setPadding(spacingType, padding); mPaddingChanged = true; dirty(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java index 518786455..7211a8bb3 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java @@ -173,10 +173,8 @@ import com.facebook.react.uimanager.annotations.ReactProp; updateNodeRegion = true; } - Spacing padding = getPadding(); - - left += padding.get(Spacing.LEFT); - top += padding.get(Spacing.TOP); + left += getPadding(Spacing.LEFT); + top += getPadding(Spacing.TOP); // these are actual right/bottom coordinates where this DrawCommand will draw. right = left + mDrawCommand.getLayoutWidth(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTTextInput.java b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTTextInput.java index 601bd2d0b..a0edc926b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTTextInput.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTTextInput.java @@ -90,12 +90,11 @@ public class RCTTextInput extends RCTVirtualText implements AndroidView, CSSNode TypedValue.COMPLEX_UNIT_PX, fontSize == UNSET ? (int) Math.ceil(PixelUtil.toPixelFromSP(ViewDefaults.FONT_SIZE_SP)) : fontSize); - Spacing padding = getPadding(); editText.setPadding( - (int) Math.ceil(padding.get(Spacing.START)), - (int) Math.ceil(padding.get(Spacing.TOP)), - (int) Math.ceil(padding.get(Spacing.END)), - (int) Math.ceil(padding.get(Spacing.BOTTOM))); + (int) Math.ceil(getPadding(Spacing.START)), + (int) Math.ceil(getPadding(Spacing.TOP)), + (int) Math.ceil(getPadding(Spacing.END)), + (int) Math.ceil(getPadding(Spacing.BOTTOM))); if (mNumberOfLines != UNSET) { editText.setLines(mNumberOfLines); @@ -128,7 +127,15 @@ public class RCTTextInput extends RCTVirtualText implements AndroidView, CSSNode super.onCollectExtraUpdates(uiViewOperationQueue); if (mJsEventCount != UNSET) { ReactTextUpdate reactTextUpdate = - new ReactTextUpdate(getText(), mJsEventCount, false, getPadding(), UNSET); + new ReactTextUpdate( + getText(), + mJsEventCount, + false, + getPadding(Spacing.START), + getPadding(Spacing.TOP), + getPadding(Spacing.END), + getPadding(Spacing.BOTTOM), + UNSET); // TODO: the Float.NaN should be replaced with the real line height see D3592781 uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate); } 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 200f0d525..43fc609bd 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java @@ -712,13 +712,12 @@ import com.facebook.react.uimanager.events.EventDispatcher; private void updateViewPadding(AndroidView androidView, int reactTag) { if (androidView.isPaddingChanged()) { - Spacing padding = androidView.getPadding(); mOperationsQueue.enqueueSetPadding( reactTag, - Math.round(padding.get(Spacing.LEFT)), - Math.round(padding.get(Spacing.TOP)), - Math.round(padding.get(Spacing.RIGHT)), - Math.round(padding.get(Spacing.BOTTOM))); + Math.round(androidView.getPadding(Spacing.LEFT)), + Math.round(androidView.getPadding(Spacing.TOP)), + Math.round(androidView.getPadding(Spacing.RIGHT)), + Math.round(androidView.getPadding(Spacing.BOTTOM))); androidView.resetPaddingChanged(); } }