From 7b348b4c81d6ba847fc45d14a4eba6dc1f003822 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Sun, 10 Jun 2018 22:37:32 -0700 Subject: [PATCH] Fix extreme TextInput slowness on Android This reverts 5898817fc "Implement letterSpacing on Android >= 5.0". Testing shows that that commit is the cause of #19126, where in a controlled TextInput after some text is first added, then deleted, further interaction with the TextInput becomes extremely slow. Fixes #19126. --- RNTester/js/TextExample.android.js | 30 ----------- RNTester/js/TextExample.ios.js | 14 ----- RNTester/js/TextInputExample.android.js | 25 --------- .../facebook/react/uimanager/ViewProps.java | 1 - .../views/text/CustomLetterSpacingSpan.java | 51 ------------------- .../views/text/ReactBaseTextShadowNode.java | 20 -------- .../react/views/textinput/ReactEditText.java | 25 --------- .../textinput/ReactTextInputManager.java | 8 --- 8 files changed, 174 deletions(-) delete mode 100644 ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java diff --git a/RNTester/js/TextExample.android.js b/RNTester/js/TextExample.android.js index 0b0bf195f68fb7..a4579d9d931cce 100644 --- a/RNTester/js/TextExample.android.js +++ b/RNTester/js/TextExample.android.js @@ -341,36 +341,6 @@ class TextExample extends React.Component<{}> { Holisticly formulate inexpensive ideas before best-of-breed benefits. Continually expedite magnetic potentialities rather than client-focused interfaces. - - - - letterSpacing = 0 - - - letterSpacing = 2 - - - letterSpacing = 9 - - - - With size and background color - - - - letterSpacing = -1 - - - [letterSpacing = 3] - - [Nested letterSpacing = 0] - - - [Nested letterSpacing = 6] - - - - diff --git a/RNTester/js/TextExample.ios.js b/RNTester/js/TextExample.ios.js index e5510390d0a353..4afcc78668e5ea 100644 --- a/RNTester/js/TextExample.ios.js +++ b/RNTester/js/TextExample.ios.js @@ -575,23 +575,9 @@ exports.examples = [ letterSpacing = 9 - - - With size and background color - - letterSpacing = -1 - - [letterSpacing = 3] - - [Nested letterSpacing = 0] - - - [Nested letterSpacing = 6] - - ); }, diff --git a/RNTester/js/TextInputExample.android.js b/RNTester/js/TextInputExample.android.js index 358d3fd043b02d..92843e3459f7fe 100644 --- a/RNTester/js/TextInputExample.android.js +++ b/RNTester/js/TextInputExample.android.js @@ -567,31 +567,6 @@ exports.examples = [ ); } }, - { - title: 'letterSpacing', - render: function() { - return ( - - - - - - - ); - } - }, { title: 'Passwords', render: function() { diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java index aee1a0c7d9a7ab..ec703e8cf9e317 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java @@ -88,7 +88,6 @@ public class ViewProps { public static final String FONT_STYLE = "fontStyle"; public static final String FONT_FAMILY = "fontFamily"; public static final String LINE_HEIGHT = "lineHeight"; - public static final String LETTER_SPACING = "letterSpacing"; public static final String NEEDS_OFFSCREEN_ALPHA_COMPOSITING = "needsOffscreenAlphaCompositing"; public static final String NUMBER_OF_LINES = "numberOfLines"; public static final String ELLIPSIZE_MODE = "ellipsizeMode"; diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java deleted file mode 100644 index d41338ccd93664..00000000000000 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java +++ /dev/null @@ -1,51 +0,0 @@ -/** - * Copyright (c) 2015-present, Facebook, Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.views.text; - -import android.annotation.TargetApi; -import android.os.Build; -import android.text.TextPaint; -import android.text.style.MetricAffectingSpan; - -import com.facebook.infer.annotation.Assertions; - -/** - * A {@link MetricAffectingSpan} that allows to set the letter spacing - * on the selected text span. - * - * The letter spacing is specified in pixels, which are converted to - * ems at paint time; this span must therefore be applied after any - * spans affecting font size. - */ -@TargetApi(Build.VERSION_CODES.LOLLIPOP) -public class CustomLetterSpacingSpan extends MetricAffectingSpan { - - private final float mLetterSpacing; - - public CustomLetterSpacingSpan(float letterSpacing) { - mLetterSpacing = letterSpacing; - } - - @Override - public void updateDrawState(TextPaint paint) { - apply(paint); - } - - @Override - public void updateMeasureState(TextPaint paint) { - apply(paint); - } - - private void apply(TextPaint paint) { - // mLetterSpacing and paint.getTextSize() are both in pixels, - // yielding an accurate em value. - if (!Float.isNaN(mLetterSpacing)) { - paint.setLetterSpacing(mLetterSpacing / paint.getTextSize()); - } - } -} diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java index 075c5be23edf3c..9b73869b1b9b69 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java @@ -118,14 +118,6 @@ private static void buildSpannedFromShadowNode( new SetSpanOperation( start, end, new BackgroundColorSpan(textShadowNode.mBackgroundColor))); } - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { - if (textShadowNode.mLetterSpacing != Float.NaN) { - ops.add(new SetSpanOperation( - start, - end, - new CustomLetterSpacingSpan(textShadowNode.mLetterSpacing))); - } - } if (textShadowNode.mFontSize != UNSET) { ops.add(new SetSpanOperation(start, end, new AbsoluteSizeSpan(textShadowNode.mFontSize))); } @@ -236,7 +228,6 @@ private static int parseNumericFontWeight(String fontWeightString) { } protected float mLineHeight = Float.NaN; - protected float mLetterSpacing = Float.NaN; protected boolean mIsColorSet = false; protected boolean mAllowFontScaling = true; protected int mColor; @@ -247,7 +238,6 @@ private static int parseNumericFontWeight(String fontWeightString) { protected int mFontSize = UNSET; protected float mFontSizeInput = UNSET; protected float mLineHeightInput = UNSET; - protected float mLetterSpacingInput = Float.NaN; protected int mTextAlign = Gravity.NO_GRAVITY; protected int mTextBreakStrategy = (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) ? 0 : Layout.BREAK_STRATEGY_HIGH_QUALITY; @@ -366,22 +356,12 @@ public void setLineHeight(float lineHeight) { markUpdated(); } - @ReactProp(name = ViewProps.LETTER_SPACING, defaultFloat = Float.NaN) - public void setLetterSpacing(float letterSpacing) { - mLetterSpacingInput = letterSpacing; - mLetterSpacing = mAllowFontScaling - ? PixelUtil.toPixelFromSP(mLetterSpacingInput) - : PixelUtil.toPixelFromDIP(mLetterSpacingInput); - markUpdated(); - } - @ReactProp(name = ViewProps.ALLOW_FONT_SCALING, defaultBoolean = true) public void setAllowFontScaling(boolean allowFontScaling) { if (allowFontScaling != mAllowFontScaling) { mAllowFontScaling = allowFontScaling; setFontSize(mFontSizeInput); setLineHeight(mLineHeightInput); - setLetterSpacing(mLetterSpacingInput); markUpdated(); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java index 7a9f34e2a6a0dd..e15dd5a7969cda 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java @@ -33,7 +33,6 @@ import android.widget.EditText; import com.facebook.infer.annotation.Assertions; import com.facebook.react.bridge.ReactContext; -import com.facebook.react.uimanager.PixelUtil; import com.facebook.react.uimanager.UIManagerModule; import com.facebook.react.views.text.CustomStyleSpan; import com.facebook.react.views.text.ReactTagSpan; @@ -81,7 +80,6 @@ public class ReactEditText extends EditText { private @Nullable ScrollWatcher mScrollWatcher; private final InternalKeyListener mKeyListener; private boolean mDetectScrollMovement = false; - private float mLetterSpacingPt = 0; private ReactViewBackgroundManager mReactBackgroundManager; @@ -629,29 +627,6 @@ public void setBorderStyle(@Nullable String style) { mReactBackgroundManager.setBorderStyle(style); } - public void setLetterSpacingPt(float letterSpacingPt) { - mLetterSpacingPt = letterSpacingPt; - updateLetterSpacing(); - } - - @Override - public void setTextSize (float size) { - super.setTextSize(size); - updateLetterSpacing(); - } - - @Override - public void setTextSize (int unit, float size) { - super.setTextSize(unit, size); - updateLetterSpacing(); - } - - protected void updateLetterSpacing() { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { - setLetterSpacing(PixelUtil.toPixelFromSP(mLetterSpacingPt) / getTextSize()); - } - } - /** * This class will redirect *TextChanged calls to the listeners only in the case where the text * is changed by the user, and not explicitly set by JS. diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java index 637027140ec010..beb22b98ff60d1 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java @@ -306,14 +306,6 @@ public void setOnScroll(final ReactEditText view, boolean onScroll) { } } - // Sets the letter spacing as an absolute point size. - // This extra handling, on top of what ReactBaseTextShadowNode already does, is required for the - // correct display of spacing in placeholder (hint) text. - @ReactProp(name = ViewProps.LETTER_SPACING, defaultFloat = 0) - public void setLetterSpacing(ReactEditText view, float letterSpacing) { - view.setLetterSpacingPt(letterSpacing); - } - @ReactProp(name = "placeholder") public void setPlaceholder(ReactEditText view, @Nullable String placeholder) { view.setHint(placeholder);