From a475c85480b2bc2a8d036b4b1ea29f6a8e749ac5 Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Tue, 26 Aug 2014 21:55:48 -0700 Subject: [PATCH 1/2] Introduce a mechanism to hide the indicator speculatively This is an optional optimization to reduce the UI latency. Imagine that the commit indicator is now displayed and the composing text is being updated, it is highly likely that the commit indicator will disappear unless the application rejects the setComposingText request. If we assume that the application will accept the new composing text without any modifications, we can hide the indicator without waiting for the arrival of new CursorAnchorInfo event. This optimization isn't dangerous because we can show the indicator again when we receive new CursorAnchorInfo event and the assumption is turned out to be invalid. Change-Id: Id59c6607a1029782410611e768791af9984f14ac --- .../inputmethod/keyboard/TextDecorator.java | 35 ++++++++++++++----- .../latin/inputlogic/InputLogic.java | 33 ++++++++++++----- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/java/src/com/android/inputmethod/keyboard/TextDecorator.java b/java/src/com/android/inputmethod/keyboard/TextDecorator.java index 0eb8b443a..178516134 100644 --- a/java/src/com/android/inputmethod/keyboard/TextDecorator.java +++ b/java/src/com/android/inputmethod/keyboard/TextDecorator.java @@ -158,7 +158,7 @@ public class TextDecorator { if (!currentFullScreenMode && fullScreenMode) { // Currently full screen mode is not supported. // TODO: Support full screen mode. - hideIndicator(); + mUiOperator.hideUi(); } mIsFullScreenMode = fullScreenMode; } @@ -193,17 +193,36 @@ public class TextDecorator { layoutImmediately(); } - private void hideIndicator() { - mUiOperator.hideUi(); + /** + * Hides indicator if the new composing text doesn't match the expected one. + * + *

Calling this method is optional but recommended whenever the new composition is passed to + * the application. The motivation of this method is to reduce the UI latency. With this method, + * we can hide the indicator without waiting the arrival of the + * {@link InputMethodService#onUpdateCursorAnchorInfo(CursorAnchorInfo)} callback, assuming that + * the application accepts the new composing text without any modification. Even if this + * assumption is false, the indicator will be shown again when + * {@link InputMethodService#onUpdateCursorAnchorInfo(CursorAnchorInfo)} is actually received. + *

+ * + * @param newComposingText the new composing text that is being passed to the application. + */ + public void hideIndicatorIfNecessary(final CharSequence newComposingText) { + if (mMode != MODE_COMMIT && mMode != MODE_ADD_TO_DICTIONARY) { + return; + } + if (!TextUtils.equals(newComposingText, mWaitingWord.mWord)) { + mUiOperator.hideUi(); + } } private void cancelLayoutInternalUnexpectedly(final String message) { - hideIndicator(); + mUiOperator.hideUi(); Log.d(TAG, message); } private void cancelLayoutInternalExpectedly(final String message) { - hideIndicator(); + mUiOperator.hideUi(); if (DEBUG) { Log.d(TAG, message); } @@ -261,7 +280,7 @@ public class TextDecorator { lastCharRectFlag & CursorAnchorInfoCompatWrapper.CHARACTER_RECT_TYPE_MASK; if (lastCharRect == null || matrix == null || lastCharRectType != CursorAnchorInfoCompatWrapper.CHARACTER_RECT_TYPE_FULLY_VISIBLE) { - hideIndicator(); + mUiOperator.hideUi(); return; } final RectF segmentStartCharRect = new RectF(lastCharRect); @@ -312,13 +331,13 @@ public class TextDecorator { if (!TextUtils.isEmpty(composingText)) { // This is an unexpected case. // TODO: Document this. - hideIndicator(); + mUiOperator.hideUi(); return; } // In MODE_ADD_TO_DICTIONARY, we cannot retrieve the character position at all because // of the lack of composing text. We will use the insertion marker position instead. if (info.isInsertionMarkerClipped()) { - hideIndicator(); + mUiOperator.hideUi(); return; } final float insertionMarkerHolizontal = info.getInsertionMarkerHorizontal(); diff --git a/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java b/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java index 0f2ba53d5..e83f494b3 100644 --- a/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java +++ b/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java @@ -649,7 +649,7 @@ public final class InputLogic { // message, this is called outside any batch edit. Potentially, this may result in some // janky flickering of the screen, although the display speed makes it unlikely in // the practice. - mConnection.setComposingText(textWithUnderline, 1); + setComposingTextInternal(textWithUnderline, 1); } } @@ -672,7 +672,7 @@ public final class InputLogic { inputTransaction.setDidAffectContents(); } if (mWordComposer.isComposingWord()) { - mConnection.setComposingText(mWordComposer.getTypedWord(), 1); + setComposingTextInternal(mWordComposer.getTypedWord(), 1); inputTransaction.setDidAffectContents(); inputTransaction.setRequiresUpdateSuggestions(); } @@ -908,8 +908,7 @@ public final class InputLogic { if (mWordComposer.isSingleLetter()) { mWordComposer.setCapitalizedModeAtStartComposingTime(inputTransaction.mShiftState); } - mConnection.setComposingText(getTextWithUnderline( - mWordComposer.getTypedWord()), 1); + setComposingTextInternal(getTextWithUnderline(mWordComposer.getTypedWord()), 1); } else { final boolean swapWeakSpace = tryStripSpaceAndReturnWhetherShouldSwapInstead(event, inputTransaction); @@ -1072,7 +1071,7 @@ public final class InputLogic { mWordComposer.applyProcessedEvent(event); } if (mWordComposer.isComposingWord()) { - mConnection.setComposingText(getTextWithUnderline(mWordComposer.getTypedWord()), 1); + setComposingTextInternal(getTextWithUnderline(mWordComposer.getTypedWord()), 1); } else { mConnection.commitText("", 1); } @@ -1640,7 +1639,7 @@ public final class InputLogic { final int[] codePoints = StringUtils.toCodePointArray(stringToCommit); mWordComposer.setComposingWord(codePoints, mLatinIME.getCoordinatesForCurrentKeyboard(codePoints)); - mConnection.setComposingText(textToCommit, 1); + setComposingTextInternal(textToCommit, 1); } // Don't restart suggestion yet. We'll restart if the user deletes the separator. mLastComposedWord = LastComposedWord.NOT_A_COMPOSED_WORD; @@ -1973,10 +1972,10 @@ public final class InputLogic { } final String lastWord = batchInputText.substring(indexOfLastSpace); mWordComposer.setBatchInputWord(lastWord); - mConnection.setComposingText(lastWord, 1); + setComposingTextInternal(lastWord, 1); } else { mWordComposer.setBatchInputWord(batchInputText); - mConnection.setComposingText(batchInputText, 1); + setComposingTextInternal(batchInputText, 1); } mConnection.endBatchEdit(); // Space state must be updated before calling updateShiftState @@ -2175,6 +2174,24 @@ public final class InputLogic { inputStyle, sequenceNumber, callback); } + /** + * Used as an injection point for each call of + * {@link RichInputConnection#setComposingText(CharSequence, int)}. + * + *

Currently using this method is optional and you can still directly call + * {@link RichInputConnection#setComposingText(CharSequence, int)}, but it is recommended to + * use this method whenever possible to optimize the behavior of {@link TextDecorator}.

+ *

TODO: Should we move this mechanism to {@link RichInputConnection}?

+ * + * @param newComposingText the composing text to be set + * @param newCursorPosition the new cursor position + */ + private void setComposingTextInternal(final CharSequence newComposingText, + final int newCursorPosition) { + mConnection.setComposingText(newComposingText, newCursorPosition); + mTextDecorator.hideIndicatorIfNecessary(newComposingText); + } + ////////////////////////////////////////////////////////////////////////////////////////////// // Following methods are tentatively placed in this class for the integration with // TextDecorator. From 29200b0abe1d65aa2f9ddefd247ab91563d666f8 Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Tue, 26 Aug 2014 21:32:25 -0700 Subject: [PATCH 2/2] Set the text bgcolor only when CursorAnchorInfo is available When CursorAnchorInfo is unavailable, we shouldn't try to show the commit indicator and set the text highlight color. With this CL, RichInputConnection can be used to track if the application responded that it does support CursorAnchorInfo or not. This result will be taken into consideration when InputLogic needs to determine whether the commit indicator should be displayed or not. Change-Id: I945d70eeb02a7a5f3d9b22459b23d7028508910f --- .../android/inputmethod/latin/LatinIME.java | 15 +++------ .../latin/RichInputConnection.java | 31 +++++++++++++++++++ .../latin/inputlogic/InputLogic.java | 21 +++++++++++-- 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/java/src/com/android/inputmethod/latin/LatinIME.java b/java/src/com/android/inputmethod/latin/LatinIME.java index dcafd83a5..2a7a5413d 100644 --- a/java/src/com/android/inputmethod/latin/LatinIME.java +++ b/java/src/com/android/inputmethod/latin/LatinIME.java @@ -57,7 +57,6 @@ import android.view.inputmethod.InputMethodSubtype; import com.android.inputmethod.accessibility.AccessibilityUtils; import com.android.inputmethod.annotations.UsedForTesting; import com.android.inputmethod.compat.CursorAnchorInfoCompatWrapper; -import com.android.inputmethod.compat.InputConnectionCompatUtils; import com.android.inputmethod.compat.InputMethodServiceCompatUtils; import com.android.inputmethod.dictionarypack.DictionaryPackConstants; import com.android.inputmethod.event.Event; @@ -777,20 +776,13 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen // Note that the calling sequence of onCreate() and onCurrentInputMethodSubtypeChanged() // is not guaranteed. It may even be called at the same time on a different thread. mSubtypeSwitcher.onSubtypeChanged(subtype); - mInputLogic.onSubtypeChanged(SubtypeLocaleUtils.getCombiningRulesExtraValue(subtype)); + mInputLogic.onSubtypeChanged(SubtypeLocaleUtils.getCombiningRulesExtraValue(subtype), + mSettings.getCurrent()); loadKeyboard(); } private void onStartInputInternal(final EditorInfo editorInfo, final boolean restarting) { super.onStartInput(editorInfo, restarting); - if (ProductionFlags.ENABLE_CURSOR_ANCHOR_INFO_CALLBACK) { - // AcceptTypedWord feature relies on CursorAnchorInfo. - if (mSettings.getCurrent().mShouldShowUiToAcceptTypedWord) { - InputConnectionCompatUtils.requestUpdateCursorAnchorInfo( - getCurrentInputConnection(), true /* enableMonitor */, - true /* requestImmediateCallback */); - } - } } @SuppressWarnings("deprecation") @@ -859,7 +851,8 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen // span, so we should reset our state unconditionally, even if restarting is true. // We also tell the input logic about the combining rules for the current subtype, so // it can adjust its combiners if needed. - mInputLogic.startInput(mSubtypeSwitcher.getCombiningRulesExtraValueOfCurrentSubtype()); + mInputLogic.startInput(mSubtypeSwitcher.getCombiningRulesExtraValueOfCurrentSubtype(), + currentSettingsValues); // Note: the following does a round-trip IPC on the main thread: be careful final Locale currentLocale = mSubtypeSwitcher.getCurrentSubtypeLocale(); diff --git a/java/src/com/android/inputmethod/latin/RichInputConnection.java b/java/src/com/android/inputmethod/latin/RichInputConnection.java index 035557610..497823aeb 100644 --- a/java/src/com/android/inputmethod/latin/RichInputConnection.java +++ b/java/src/com/android/inputmethod/latin/RichInputConnection.java @@ -30,7 +30,9 @@ import android.view.inputmethod.CorrectionInfo; import android.view.inputmethod.ExtractedText; import android.view.inputmethod.ExtractedTextRequest; import android.view.inputmethod.InputConnection; +import android.view.inputmethod.InputMethodManager; +import com.android.inputmethod.compat.InputConnectionCompatUtils; import com.android.inputmethod.latin.settings.SpacingAndPunctuations; import com.android.inputmethod.latin.utils.CapsModeUtils; import com.android.inputmethod.latin.utils.DebugLogUtils; @@ -906,4 +908,33 @@ public final class RichInputConnection { mIC.setSelection(mExpectedSelStart, mExpectedSelEnd); } } + + private boolean mCursorAnchorInfoMonitorEnabled = false; + + /** + * Requests the editor to call back {@link InputMethodManager#updateCursorAnchorInfo}. + * @param enableMonitor {@code true} to request the editor to call back the method whenever the + * cursor/anchor position is changed. + * @param requestImmediateCallback {@code true} to request the editor to call back the method + * as soon as possible to notify the current cursor/anchor position to the input method. + * @return {@code true} if the request is accepted. Returns {@code false} otherwise, which + * includes "not implemented" or "rejected" or "temporarily unavailable" or whatever which + * prevents the application from fulfilling the request. (TODO: Improve the API when it turns + * out that we actually need more detailed error codes) + */ + public boolean requestUpdateCursorAnchorInfo(final boolean enableMonitor, + final boolean requestImmediateCallback) { + final boolean scheduled = InputConnectionCompatUtils.requestUpdateCursorAnchorInfo(mIC, + enableMonitor, requestImmediateCallback); + mCursorAnchorInfoMonitorEnabled = (scheduled && enableMonitor); + return scheduled; + } + + /** + * @return {@code true} if the application reported that the monitor mode of + * {@link InputMethodService#onUpdateCursorAnchorInfo(CursorAnchorInfo)} is currently enabled. + */ + public boolean isCursorAnchorInfoMonitorEnabled() { + return mCursorAnchorInfoMonitorEnabled; + } } diff --git a/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java b/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java index e83f494b3..de62f9786 100644 --- a/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java +++ b/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java @@ -50,6 +50,7 @@ import com.android.inputmethod.latin.SuggestedWords; import com.android.inputmethod.latin.SuggestedWords.SuggestedWordInfo; import com.android.inputmethod.latin.WordComposer; import com.android.inputmethod.latin.define.DebugFlags; +import com.android.inputmethod.latin.define.ProductionFlags; import com.android.inputmethod.latin.settings.SettingsValues; import com.android.inputmethod.latin.settings.SettingsValuesForSuggestion; import com.android.inputmethod.latin.settings.SpacingAndPunctuations; @@ -140,8 +141,9 @@ public final class InputLogic { * Call this when input starts or restarts in some editor (typically, in onStartInputView). * * @param combiningSpec the combining spec string for this subtype + * @param settingsValues the current settings values */ - public void startInput(final String combiningSpec) { + public void startInput(final String combiningSpec, final SettingsValues settingsValues) { mEnteredText = null; mWordComposer.restartCombining(combiningSpec); resetComposingState(true /* alsoResetLastComposedWord */); @@ -159,15 +161,24 @@ public final class InputLogic { } else { mInputLogicHandler.reset(); } + + if (ProductionFlags.ENABLE_CURSOR_ANCHOR_INFO_CALLBACK) { + // AcceptTypedWord feature relies on CursorAnchorInfo. + if (settingsValues.mShouldShowUiToAcceptTypedWord) { + mConnection.requestUpdateCursorAnchorInfo(true /* enableMonitor */, + true /* requestImmediateCallback */); + } + } } /** * Call this when the subtype changes. * @param combiningSpec the spec string for the combining rules + * @param settingsValues the current settings values */ - public void onSubtypeChanged(final String combiningSpec) { + public void onSubtypeChanged(final String combiningSpec, final SettingsValues settingsValues) { finishInput(); - startInput(combiningSpec); + startInput(combiningSpec, settingsValues); } /** @@ -2238,6 +2249,10 @@ public final class InputLogic { */ private boolean shouldShowCommitIndicator(final SuggestedWords suggestedWords, final SettingsValues settingsValues) { + if (!mConnection.isCursorAnchorInfoMonitorEnabled()) { + // We cannot help in this case because we are heavily relying on this new API. + return false; + } if (!settingsValues.mShouldShowUiToAcceptTypedWord) { return false; }