From c2261f46947b7c22153a10583e16e5802b883e22 Mon Sep 17 00:00:00 2001 From: Candice Lo Date: Thu, 13 Apr 2023 06:38:22 +0000 Subject: [PATCH 1/2] Fix seekbar flickers when changing the font size Delaying the time of updating the system font scale to guarantee that the seekbar status has been updated first. Bug: 277821437 Test: Manual - Attach video to the bug Test: atest FontScalingDialogTest Test: atest FontScalingTileTest Change-Id: I5f91a77677481efb434478d2b39112ad3c88948d --- .../fontscaling/FontScalingDialog.kt | 76 +++++++++++++++---- .../systemui/qs/tiles/FontScalingTile.kt | 13 +++- .../fontscaling/FontScalingDialogTest.kt | 49 +++++++++--- .../systemui/qs/tiles/FontScalingTileTest.kt | 10 ++- 4 files changed, 118 insertions(+), 30 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/accessibility/fontscaling/FontScalingDialog.kt b/packages/SystemUI/src/com/android/systemui/accessibility/fontscaling/FontScalingDialog.kt index c9579d5e13567..a5b7878f22aa0 100644 --- a/packages/SystemUI/src/com/android/systemui/accessibility/fontscaling/FontScalingDialog.kt +++ b/packages/SystemUI/src/com/android/systemui/accessibility/fontscaling/FontScalingDialog.kt @@ -15,11 +15,12 @@ */ package com.android.systemui.accessibility.fontscaling -import android.annotation.WorkerThread import android.content.Context import android.content.pm.ActivityInfo import android.content.res.Configuration +import android.database.ContentObserver import android.os.Bundle +import android.os.Handler import android.provider.Settings import android.util.TypedValue import android.view.LayoutInflater @@ -27,13 +28,18 @@ import android.widget.Button import android.widget.SeekBar import android.widget.SeekBar.OnSeekBarChangeListener import android.widget.TextView +import androidx.annotation.MainThread +import androidx.annotation.WorkerThread import com.android.systemui.R import com.android.systemui.common.ui.view.SeekBarWithIconButtonsView import com.android.systemui.dagger.qualifiers.Background +import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.statusbar.phone.SystemUIDialog +import com.android.systemui.util.concurrency.DelayableExecutor import com.android.systemui.util.settings.SecureSettings import com.android.systemui.util.settings.SystemSettings -import java.util.concurrent.Executor +import com.android.systemui.util.time.SystemClock +import java.util.concurrent.atomic.AtomicInteger import kotlin.math.roundToInt /** The Dialog that contains a seekbar for changing the font size. */ @@ -41,17 +47,31 @@ class FontScalingDialog( context: Context, private val systemSettings: SystemSettings, private val secureSettings: SecureSettings, - @Background private val backgroundExecutor: Executor + private val systemClock: SystemClock, + @Main mainHandler: Handler, + @Background private val backgroundDelayableExecutor: DelayableExecutor ) : SystemUIDialog(context) { + private val MIN_UPDATE_INTERVAL_MS: Long = 800 + private val CHANGE_BY_SEEKBAR_DELAY_MS: Long = 100 + private val CHANGE_BY_BUTTON_DELAY_MS: Long = 300 private val strEntryValues: Array = context.resources.getStringArray(com.android.settingslib.R.array.entryvalues_font_size) private lateinit var title: TextView private lateinit var doneButton: Button private lateinit var seekBarWithIconButtonsView: SeekBarWithIconButtonsView - private var lastProgress: Int = -1 + private var lastProgress: AtomicInteger = AtomicInteger(-1) + private var lastUpdateTime: Long = 0 + private var cancelUpdateFontScaleRunnable: Runnable? = null private val configuration: Configuration = Configuration(context.resources.configuration) + private val fontSizeObserver = + object : ContentObserver(mainHandler) { + override fun onChange(selfChange: Boolean) { + lastUpdateTime = systemClock.elapsedRealtime() + } + } + override fun onCreate(savedInstanceState: Bundle?) { setTitle(R.string.font_scaling_dialog_title) setView(LayoutInflater.from(context).inflate(R.layout.font_scaling_dialog, null)) @@ -79,8 +99,8 @@ class FontScalingDialog( seekBarWithIconButtonsView.setMax((strEntryValues).size - 1) val currentScale = systemSettings.getFloat(Settings.System.FONT_SCALE, 1.0f) - lastProgress = fontSizeValueToIndex(currentScale) - seekBarWithIconButtonsView.setProgress(lastProgress) + lastProgress.set(fontSizeValueToIndex(currentScale)) + seekBarWithIconButtonsView.setProgress(lastProgress.get()) seekBarWithIconButtonsView.setOnSeekBarChangeListener( object : OnSeekBarChangeListener { @@ -89,7 +109,7 @@ class FontScalingDialog( override fun onProgressChanged(seekBar: SeekBar, progress: Int, fromUser: Boolean) { if (!isTrackingTouch) { // The seekbar progress is changed by icon buttons - changeFontSize(progress) + changeFontSize(progress, CHANGE_BY_BUTTON_DELAY_MS) } else { // Provide preview configuration for text instead of changing the system // font scale before users release their finger from the seekbar. @@ -103,26 +123,50 @@ class FontScalingDialog( override fun onStopTrackingTouch(seekBar: SeekBar) { isTrackingTouch = false - changeFontSize(seekBar.progress) + changeFontSize(seekBar.progress, CHANGE_BY_SEEKBAR_DELAY_MS) } } ) doneButton.setOnClickListener { dismiss() } + systemSettings.registerContentObserver(Settings.System.FONT_SCALE, fontSizeObserver) } - private fun changeFontSize(progress: Int) { - if (progress != lastProgress) { + /** + * Avoid SeekBar flickers when changing font scale. See the description from Setting at {@link + * TextReadingPreviewController#postCommitDelayed} for the reasons of flickers. + */ + @MainThread + fun updateFontScaleDelayed(delayMsFromSource: Long) { + var delayMs = delayMsFromSource + if (systemClock.elapsedRealtime() - lastUpdateTime < MIN_UPDATE_INTERVAL_MS) { + delayMs += MIN_UPDATE_INTERVAL_MS + } + cancelUpdateFontScaleRunnable?.run() + cancelUpdateFontScaleRunnable = + backgroundDelayableExecutor.executeDelayed({ updateFontScale() }, delayMs) + } + + override fun stop() { + cancelUpdateFontScaleRunnable?.run() + cancelUpdateFontScaleRunnable = null + systemSettings.unregisterContentObserver(fontSizeObserver) + } + + @MainThread + private fun changeFontSize(progress: Int, changedWithDelay: Long) { + if (progress != lastProgress.get()) { + lastProgress.set(progress) + if (!fontSizeHasBeenChangedFromTile) { - backgroundExecutor.execute { updateSecureSettingsIfNeeded() } + backgroundDelayableExecutor.execute { updateSecureSettingsIfNeeded() } fontSizeHasBeenChangedFromTile = true } - backgroundExecutor.execute { updateFontScale(strEntryValues[progress]) } - - lastProgress = progress + updateFontScaleDelayed(changedWithDelay) } } + @WorkerThread private fun fontSizeValueToIndex(value: Float): Int { var lastValue = strEntryValues[0].toFloat() for (i in 1 until strEntryValues.size) { @@ -150,8 +194,8 @@ class FontScalingDialog( } @WorkerThread - fun updateFontScale(newScale: String) { - systemSettings.putString(Settings.System.FONT_SCALE, newScale) + fun updateFontScale() { + systemSettings.putString(Settings.System.FONT_SCALE, strEntryValues[lastProgress.get()]) } @WorkerThread diff --git a/packages/SystemUI/src/com/android/systemui/qs/tiles/FontScalingTile.kt b/packages/SystemUI/src/com/android/systemui/qs/tiles/FontScalingTile.kt index 12d98473ff076..c013486b83e4f 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/tiles/FontScalingTile.kt +++ b/packages/SystemUI/src/com/android/systemui/qs/tiles/FontScalingTile.kt @@ -17,7 +17,6 @@ package com.android.systemui.qs.tiles import android.content.Intent import android.os.Handler -import android.os.HandlerExecutor import android.os.Looper import android.provider.Settings import android.view.View @@ -40,8 +39,10 @@ import com.android.systemui.qs.QsEventLogger import com.android.systemui.qs.logging.QSLogger import com.android.systemui.qs.tileimpl.QSTileImpl import com.android.systemui.statusbar.phone.SystemUIDialog +import com.android.systemui.util.concurrency.DelayableExecutor import com.android.systemui.util.settings.SecureSettings import com.android.systemui.util.settings.SystemSettings +import com.android.systemui.util.time.SystemClock import javax.inject.Inject class FontScalingTile @@ -50,7 +51,7 @@ constructor( host: QSHost, uiEventLogger: QsEventLogger, @Background backgroundLooper: Looper, - @Main mainHandler: Handler, + @Main private val mainHandler: Handler, falsingManager: FalsingManager, metricsLogger: MetricsLogger, statusBarStateController: StatusBarStateController, @@ -59,7 +60,9 @@ constructor( private val dialogLaunchAnimator: DialogLaunchAnimator, private val systemSettings: SystemSettings, private val secureSettings: SecureSettings, - private val featureFlags: FeatureFlags + private val systemClock: SystemClock, + private val featureFlags: FeatureFlags, + @Background private val backgroundDelayableExecutor: DelayableExecutor ) : QSTileImpl( host, @@ -89,7 +92,9 @@ constructor( mContext, systemSettings, secureSettings, - HandlerExecutor(mHandler) + systemClock, + mainHandler, + backgroundDelayableExecutor ) if (view != null) { dialogLaunchAnimator.showFromView( diff --git a/packages/SystemUI/tests/src/com/android/systemui/accessibility/fontscaling/FontScalingDialogTest.kt b/packages/SystemUI/tests/src/com/android/systemui/accessibility/fontscaling/FontScalingDialogTest.kt index 353a7c370ab63..f10c21bc514ac 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/accessibility/fontscaling/FontScalingDialogTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/accessibility/fontscaling/FontScalingDialogTest.kt @@ -50,10 +50,14 @@ private const val OFF: Int = 0 @RunWith(AndroidTestingRunner::class) @TestableLooper.RunWithLooper(setAsMainLooper = true) class FontScalingDialogTest : SysuiTestCase() { + private val MIN_UPDATE_INTERVAL_MS: Long = 800 + private val CHANGE_BY_SEEKBAR_DELAY_MS: Long = 100 + private val CHANGE_BY_BUTTON_DELAY_MS: Long = 300 private lateinit var fontScalingDialog: FontScalingDialog private lateinit var systemSettings: SystemSettings private lateinit var secureSettings: SecureSettings - private lateinit var backgroundExecutor: FakeExecutor + private lateinit var systemClock: FakeSystemClock + private lateinit var backgroundDelayableExecutor: FakeExecutor private val fontSizeValueArray: Array = mContext .getResources() @@ -67,10 +71,20 @@ class FontScalingDialogTest : SysuiTestCase() { MockitoAnnotations.initMocks(this) val mainHandler = Handler(TestableLooper.get(this).getLooper()) systemSettings = FakeSettings() + // Guarantee that the systemSettings always starts with the default font scale. + systemSettings.putFloat(Settings.System.FONT_SCALE, 1.0f) secureSettings = FakeSettings() - backgroundExecutor = FakeExecutor(FakeSystemClock()) + systemClock = FakeSystemClock() + backgroundDelayableExecutor = FakeExecutor(systemClock) fontScalingDialog = - spy(FontScalingDialog(mContext, systemSettings, secureSettings, backgroundExecutor)) + FontScalingDialog( + mContext, + systemSettings, + secureSettings, + systemClock, + mainHandler, + backgroundDelayableExecutor + ) } @Test @@ -96,9 +110,14 @@ class FontScalingDialogTest : SysuiTestCase() { val seekBar: SeekBar = fontScalingDialog.findViewById(R.id.seekbar)!! seekBarWithIconButtonsView.setProgress(0) + backgroundDelayableExecutor.runAllReady() + backgroundDelayableExecutor.advanceClockToNext() + backgroundDelayableExecutor.runAllReady() iconEndFrame.performClick() - backgroundExecutor.runAllReady() + backgroundDelayableExecutor.runAllReady() + backgroundDelayableExecutor.advanceClockToNext() + backgroundDelayableExecutor.runAllReady() val currentScale = systemSettings.getFloat(Settings.System.FONT_SCALE, /* def= */ 1.0f) assertThat(seekBar.getProgress()).isEqualTo(1) @@ -117,9 +136,14 @@ class FontScalingDialogTest : SysuiTestCase() { val seekBar: SeekBar = fontScalingDialog.findViewById(R.id.seekbar)!! seekBarWithIconButtonsView.setProgress(fontSizeValueArray.size - 1) + backgroundDelayableExecutor.runAllReady() + backgroundDelayableExecutor.advanceClockToNext() + backgroundDelayableExecutor.runAllReady() iconStartFrame.performClick() - backgroundExecutor.runAllReady() + backgroundDelayableExecutor.runAllReady() + backgroundDelayableExecutor.advanceClockToNext() + backgroundDelayableExecutor.runAllReady() val currentScale = systemSettings.getFloat(Settings.System.FONT_SCALE, /* def= */ 1.0f) assertThat(seekBar.getProgress()).isEqualTo(fontSizeValueArray.size - 2) @@ -139,7 +163,7 @@ class FontScalingDialogTest : SysuiTestCase() { // Default seekbar progress for font size is 1, set it to another progress 0 seekBarWithIconButtonsView.setProgress(0) - backgroundExecutor.runAllReady() + backgroundDelayableExecutor.runAllReady() val currentSettings = secureSettings.getInt( @@ -153,6 +177,7 @@ class FontScalingDialogTest : SysuiTestCase() { @Test fun dragSeekbar_systemFontSizeSettingsDoesNotChange() { + fontScalingDialog = spy(fontScalingDialog) val slider: SeekBarWithIconButtonsView = spy(SeekBarWithIconButtonsView(mContext)) whenever( fontScalingDialog.findViewById(R.id.font_scaling_slider) @@ -169,7 +194,9 @@ class FontScalingDialogTest : SysuiTestCase() { // OnSeekBarChangeListener and the seekbar could get updated progress value // in onStopTrackingTouch. seekBar.progress = 0 - backgroundExecutor.runAllReady() + backgroundDelayableExecutor.runAllReady() + backgroundDelayableExecutor.advanceClockToNext() + backgroundDelayableExecutor.runAllReady() // Verify that the scale of font size remains the default value 1.0f. var systemScale = systemSettings.getFloat(Settings.System.FONT_SCALE, /* def= */ 1.0f) @@ -177,7 +204,9 @@ class FontScalingDialogTest : SysuiTestCase() { // Simulate releasing the finger from the seekbar. seekBarChangeCaptor.value.onStopTrackingTouch(seekBar) - backgroundExecutor.runAllReady() + backgroundDelayableExecutor.runAllReady() + backgroundDelayableExecutor.advanceClockToNext() + backgroundDelayableExecutor.runAllReady() // Verify that the scale of font size has been updated. systemScale = systemSettings.getFloat(Settings.System.FONT_SCALE, /* def= */ 1.0f) @@ -188,6 +217,7 @@ class FontScalingDialogTest : SysuiTestCase() { @Test fun dragSeekBar_createTextPreview() { + fontScalingDialog = spy(fontScalingDialog) val slider: SeekBarWithIconButtonsView = spy(SeekBarWithIconButtonsView(mContext)) whenever( fontScalingDialog.findViewById(R.id.font_scaling_slider) @@ -205,7 +235,8 @@ class FontScalingDialogTest : SysuiTestCase() { /* progress= */ 0, /* fromUser= */ false ) - backgroundExecutor.runAllReady() + backgroundDelayableExecutor.advanceClockToNext() + backgroundDelayableExecutor.runAllReady() verify(fontScalingDialog).createTextPreview(/* index= */ 0) fontScalingDialog.dismiss() diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/FontScalingTileTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/FontScalingTileTest.kt index 1d6f225dd0a30..ddbfca57e6880 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/FontScalingTileTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/FontScalingTileTest.kt @@ -33,10 +33,12 @@ import com.android.systemui.plugins.statusbar.StatusBarStateController import com.android.systemui.qs.QSHost import com.android.systemui.qs.QsEventLogger import com.android.systemui.qs.logging.QSLogger +import com.android.systemui.util.concurrency.FakeExecutor import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.eq import com.android.systemui.util.mockito.nullable import com.android.systemui.util.settings.FakeSettings +import com.android.systemui.util.time.FakeSystemClock import com.google.common.truth.Truth.assertThat import org.junit.After import org.junit.Before @@ -61,6 +63,8 @@ class FontScalingTileTest : SysuiTestCase() { @Mock private lateinit var uiEventLogger: QsEventLogger private lateinit var testableLooper: TestableLooper + private lateinit var systemClock: FakeSystemClock + private lateinit var backgroundDelayableExecutor: FakeExecutor private lateinit var fontScalingTile: FontScalingTile val featureFlags = FakeFeatureFlags() @@ -70,6 +74,8 @@ class FontScalingTileTest : SysuiTestCase() { MockitoAnnotations.initMocks(this) testableLooper = TestableLooper.get(this) `when`(qsHost.getContext()).thenReturn(mContext) + systemClock = FakeSystemClock() + backgroundDelayableExecutor = FakeExecutor(systemClock) fontScalingTile = FontScalingTile( @@ -85,7 +91,9 @@ class FontScalingTileTest : SysuiTestCase() { dialogLaunchAnimator, FakeSettings(), FakeSettings(), - featureFlags + FakeSystemClock(), + featureFlags, + backgroundDelayableExecutor, ) fontScalingTile.initialize() testableLooper.processAllMessages() From 12c0ea2c98245bb8ef74ea42d09a3058e2c0a2e4 Mon Sep 17 00:00:00 2001 From: Candice Lo Date: Tue, 25 Apr 2023 10:51:33 +0000 Subject: [PATCH 2/2] Provides text preview whenever the seekbar progress changes When the font scale is modified by the icon buttons, we also like to show the preview of text first to avoid the feeling of lag for users to wait for the system font size change. Bug: 279525588 Test: manually - attach the video to the bug Change-Id: I8da85eb9b264f331c4347a87e2107d52872f4274 --- .../accessibility/fontscaling/FontScalingDialog.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/accessibility/fontscaling/FontScalingDialog.kt b/packages/SystemUI/src/com/android/systemui/accessibility/fontscaling/FontScalingDialog.kt index a5b7878f22aa0..e79b3f4bf3ad3 100644 --- a/packages/SystemUI/src/com/android/systemui/accessibility/fontscaling/FontScalingDialog.kt +++ b/packages/SystemUI/src/com/android/systemui/accessibility/fontscaling/FontScalingDialog.kt @@ -107,13 +107,13 @@ class FontScalingDialog( var isTrackingTouch = false override fun onProgressChanged(seekBar: SeekBar, progress: Int, fromUser: Boolean) { + // Always provide preview configuration for text first when there is a change + // in the seekbar progress. + createTextPreview(progress) + if (!isTrackingTouch) { // The seekbar progress is changed by icon buttons changeFontSize(progress, CHANGE_BY_BUTTON_DELAY_MS) - } else { - // Provide preview configuration for text instead of changing the system - // font scale before users release their finger from the seekbar. - createTextPreview(progress) } }