From 6caeeb1aebb2231942581ce63fbca535db10f06c Mon Sep 17 00:00:00 2001 From: Jordan Demeulenaere Date: Mon, 7 Aug 2023 10:09:24 +0200 Subject: [PATCH 1/2] Convert PeopleSpaceActivity to Kotlin This CL converts the PeopleSpaceActivity to Kotlin. This is so that ag/24308773 can call coroutines extension functions, which look extremely bad when called in Java. This is a pure conversion with no change in logic. Bug: 238993727 Test: Manual, built and started the PeopleSpaceActivity by adding a Conversation widget to the launcher. Change-Id: Id5e94ce517111d503a7e48576891d7b7680835e3 Change-Id: Id8eeac2b5dd52849b85f9e92c7e052987507451c --- .../systemui/people/PeopleSpaceActivity.java | 98 ------------------- .../systemui/people/PeopleSpaceActivity.kt | 85 ++++++++++++++++ 2 files changed, 85 insertions(+), 98 deletions(-) delete mode 100644 packages/SystemUI/src/com/android/systemui/people/PeopleSpaceActivity.java create mode 100644 packages/SystemUI/src/com/android/systemui/people/PeopleSpaceActivity.kt diff --git a/packages/SystemUI/src/com/android/systemui/people/PeopleSpaceActivity.java b/packages/SystemUI/src/com/android/systemui/people/PeopleSpaceActivity.java deleted file mode 100644 index d1d3e3de39f0d..0000000000000 --- a/packages/SystemUI/src/com/android/systemui/people/PeopleSpaceActivity.java +++ /dev/null @@ -1,98 +0,0 @@ -/* - * Copyright (C) 2020 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.systemui.people; - -import static android.appwidget.AppWidgetManager.EXTRA_APPWIDGET_ID; -import static android.appwidget.AppWidgetManager.INVALID_APPWIDGET_ID; - -import android.content.Intent; -import android.os.Bundle; -import android.util.Log; -import android.view.ViewGroup; - -import androidx.activity.ComponentActivity; -import androidx.lifecycle.ViewModelProvider; - -import com.android.systemui.compose.ComposeFacade; -import com.android.systemui.flags.FeatureFlags; -import com.android.systemui.flags.Flags; -import com.android.systemui.people.ui.view.PeopleViewBinder; -import com.android.systemui.people.ui.viewmodel.PeopleViewModel; - -import javax.inject.Inject; - -import kotlin.Unit; -import kotlin.jvm.functions.Function1; - -/** People Tile Widget configuration activity that shows the user their conversation tiles. */ -public class PeopleSpaceActivity extends ComponentActivity { - - private static final String TAG = "PeopleSpaceActivity"; - private static final boolean DEBUG = PeopleSpaceUtils.DEBUG; - - private final PeopleViewModel.Factory mViewModelFactory; - private final FeatureFlags mFeatureFlags; - - @Inject - public PeopleSpaceActivity(PeopleViewModel.Factory viewModelFactory, - FeatureFlags featureFlags) { - super(); - mViewModelFactory = viewModelFactory; - mFeatureFlags = featureFlags; - } - - @Override - protected void onCreate(Bundle savedInstanceState) { - super.onCreate(savedInstanceState); - setResult(RESULT_CANCELED); - - PeopleViewModel viewModel = new ViewModelProvider(this, mViewModelFactory).get( - PeopleViewModel.class); - - // Update the widget ID coming from the intent. - int widgetId = getIntent().getIntExtra(EXTRA_APPWIDGET_ID, INVALID_APPWIDGET_ID); - viewModel.onWidgetIdChanged(widgetId); - - Function1 onResult = (result) -> { - finishActivity(result); - return null; - }; - - if (mFeatureFlags.isEnabled(Flags.COMPOSE_PEOPLE_SPACE) - && ComposeFacade.INSTANCE.isComposeAvailable()) { - Log.d(TAG, "Using the Compose implementation of the PeopleSpaceActivity"); - ComposeFacade.INSTANCE.setPeopleSpaceActivityContent(this, viewModel, onResult); - } else { - Log.d(TAG, "Using the View implementation of the PeopleSpaceActivity"); - ViewGroup view = PeopleViewBinder.create(this); - PeopleViewBinder.bind(view, viewModel, /* lifecycleOwner= */ this, onResult); - setContentView(view); - } - } - - private void finishActivity(PeopleViewModel.Result result) { - if (result instanceof PeopleViewModel.Result.Success) { - if (DEBUG) Log.d(TAG, "Widget added!"); - Intent data = ((PeopleViewModel.Result.Success) result).getData(); - setResult(RESULT_OK, data); - } else { - if (DEBUG) Log.d(TAG, "Activity dismissed with no widgets added!"); - setResult(RESULT_CANCELED); - } - finish(); - } -} diff --git a/packages/SystemUI/src/com/android/systemui/people/PeopleSpaceActivity.kt b/packages/SystemUI/src/com/android/systemui/people/PeopleSpaceActivity.kt new file mode 100644 index 0000000000000..30e84368ac0b3 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/people/PeopleSpaceActivity.kt @@ -0,0 +1,85 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.people + +import android.appwidget.AppWidgetManager +import android.os.Bundle +import android.util.Log +import androidx.activity.ComponentActivity +import androidx.lifecycle.ViewModelProvider +import com.android.systemui.compose.ComposeFacade.isComposeAvailable +import com.android.systemui.compose.ComposeFacade.setPeopleSpaceActivityContent +import com.android.systemui.flags.FeatureFlags +import com.android.systemui.flags.Flags +import com.android.systemui.people.ui.view.PeopleViewBinder +import com.android.systemui.people.ui.view.PeopleViewBinder.bind +import com.android.systemui.people.ui.viewmodel.PeopleViewModel +import javax.inject.Inject + +/** People Tile Widget configuration activity that shows the user their conversation tiles. */ +class PeopleSpaceActivity +@Inject +constructor( + private val viewModelFactory: PeopleViewModel.Factory, + private val featureFlags: FeatureFlags, +) : ComponentActivity() { + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + setResult(RESULT_CANCELED) + + // Update the widget ID coming from the intent. + val viewModel = ViewModelProvider(this, viewModelFactory)[PeopleViewModel::class.java] + val widgetId = + intent.getIntExtra( + AppWidgetManager.EXTRA_APPWIDGET_ID, + AppWidgetManager.INVALID_APPWIDGET_ID, + ) + viewModel.onWidgetIdChanged(widgetId) + + // Set the content of the activity, using either the View or Compose implementation. + if (featureFlags.isEnabled(Flags.COMPOSE_PEOPLE_SPACE) && isComposeAvailable()) { + Log.d(TAG, "Using the Compose implementation of the PeopleSpaceActivity") + setPeopleSpaceActivityContent( + activity = this, + viewModel, + onResult = { finishActivity(it) }, + ) + } else { + Log.d(TAG, "Using the View implementation of the PeopleSpaceActivity") + val view = PeopleViewBinder.create(this) + bind(view, viewModel, lifecycleOwner = this, onResult = { finishActivity(it) }) + setContentView(view) + } + } + + private fun finishActivity(result: PeopleViewModel.Result) { + if (result is PeopleViewModel.Result.Success) { + if (DEBUG) Log.d(TAG, "Widget added!") + setResult(RESULT_OK, result.data) + } else { + if (DEBUG) Log.d(TAG, "Activity dismissed with no widgets added!") + setResult(RESULT_CANCELED) + } + + finish() + } + + companion object { + private const val TAG = "PeopleSpaceActivity" + private const val DEBUG = PeopleSpaceUtils.DEBUG + } +} From fc76d7bb005b9822a7ad4356b343ad80bc69bc9c Mon Sep 17 00:00:00 2001 From: Jordan Demeulenaere Date: Thu, 3 Aug 2023 15:35:52 +0200 Subject: [PATCH 2/2] Refresh PeopleSpace tiles earlier to avoid recomposition This CL moves the logic used to refresh the PeopleSpace tiles on RESUME events so that the first time it is triggered happens before our first composition, to avoid composing and redrawing this screen twice, which reduces GPU consumption. See b/276871425 for details. Test: See go/sysui-memory-comparison Bug: 276871425 Change-Id: I0feec96dc526b38f5bec0a7da29bd2fbe4dc2d7e --- .../systemui/people/ui/compose/PeopleScreen.kt | 12 ------------ .../android/systemui/people/PeopleSpaceActivity.kt | 14 ++++++++++++++ .../systemui/people/ui/view/PeopleViewBinder.kt | 8 -------- 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/packages/SystemUI/compose/features/src/com/android/systemui/people/ui/compose/PeopleScreen.kt b/packages/SystemUI/compose/features/src/com/android/systemui/people/ui/compose/PeopleScreen.kt index d84e67620177f..68f010e1c50d8 100644 --- a/packages/SystemUI/compose/features/src/com/android/systemui/people/ui/compose/PeopleScreen.kt +++ b/packages/SystemUI/compose/features/src/com/android/systemui/people/ui/compose/PeopleScreen.kt @@ -42,13 +42,10 @@ import androidx.compose.runtime.key import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.asImageBitmap -import androidx.compose.ui.platform.LocalLifecycleOwner import androidx.compose.ui.res.dimensionResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp -import androidx.lifecycle.Lifecycle -import androidx.lifecycle.repeatOnLifecycle import com.android.compose.theme.LocalAndroidColorScheme import com.android.systemui.R import com.android.systemui.compose.modifiers.sysuiResTag @@ -70,15 +67,6 @@ fun PeopleScreen( val priorityTiles by viewModel.priorityTiles.collectAsState() val recentTiles by viewModel.recentTiles.collectAsState() - // Make sure to refresh the tiles/conversations when the lifecycle is resumed, so that it - // updates them when going back to the Activity after leaving it. - val lifecycleOwner = LocalLifecycleOwner.current - LaunchedEffect(lifecycleOwner, viewModel) { - lifecycleOwner.repeatOnLifecycle(Lifecycle.State.RESUMED) { - viewModel.onTileRefreshRequested() - } - } - // Call [onResult] this activity when the ViewModel tells us so. LaunchedEffect(viewModel.result) { viewModel.result.collect { result -> diff --git a/packages/SystemUI/src/com/android/systemui/people/PeopleSpaceActivity.kt b/packages/SystemUI/src/com/android/systemui/people/PeopleSpaceActivity.kt index 30e84368ac0b3..5b7eb454597c1 100644 --- a/packages/SystemUI/src/com/android/systemui/people/PeopleSpaceActivity.kt +++ b/packages/SystemUI/src/com/android/systemui/people/PeopleSpaceActivity.kt @@ -20,7 +20,10 @@ import android.appwidget.AppWidgetManager import android.os.Bundle import android.util.Log import androidx.activity.ComponentActivity +import androidx.lifecycle.Lifecycle import androidx.lifecycle.ViewModelProvider +import androidx.lifecycle.lifecycleScope +import androidx.lifecycle.repeatOnLifecycle import com.android.systemui.compose.ComposeFacade.isComposeAvailable import com.android.systemui.compose.ComposeFacade.setPeopleSpaceActivityContent import com.android.systemui.flags.FeatureFlags @@ -29,6 +32,7 @@ import com.android.systemui.people.ui.view.PeopleViewBinder import com.android.systemui.people.ui.view.PeopleViewBinder.bind import com.android.systemui.people.ui.viewmodel.PeopleViewModel import javax.inject.Inject +import kotlinx.coroutines.launch /** People Tile Widget configuration activity that shows the user their conversation tiles. */ class PeopleSpaceActivity @@ -50,6 +54,16 @@ constructor( ) viewModel.onWidgetIdChanged(widgetId) + // Make sure to refresh the tiles/conversations when the lifecycle is resumed, so that it + // updates them when going back to the Activity after leaving it. + // Note that we do this here instead of inside an effect in the PeopleScreen() composable + // because otherwise onTileRefreshRequested() will be called after the first composition, + // which will trigger a new recomposition and redraw, affecting the GPU memory (see + // b/276871425). + lifecycleScope.launch { + repeatOnLifecycle(Lifecycle.State.RESUMED) { viewModel.onTileRefreshRequested() } + } + // Set the content of the activity, using either the View or Compose implementation. if (featureFlags.isEnabled(Flags.COMPOSE_PEOPLE_SPACE) && isComposeAvailable()) { Log.d(TAG, "Using the Compose implementation of the PeopleSpaceActivity") diff --git a/packages/SystemUI/src/com/android/systemui/people/ui/view/PeopleViewBinder.kt b/packages/SystemUI/src/com/android/systemui/people/ui/view/PeopleViewBinder.kt index d8a429e5bb1a1..5f338c30c9666 100644 --- a/packages/SystemUI/src/com/android/systemui/people/ui/view/PeopleViewBinder.kt +++ b/packages/SystemUI/src/com/android/systemui/people/ui/view/PeopleViewBinder.kt @@ -109,14 +109,6 @@ object PeopleViewBinder { } } } - - // Make sure to refresh the tiles/conversations when the Activity is resumed, so that it - // updates them when going back to the Activity after leaving it. - lifecycleOwner.lifecycleScope.launch { - lifecycleOwner.repeatOnLifecycle(Lifecycle.State.RESUMED) { - viewModel.onTileRefreshRequested() - } - } } private fun setNoConversationsContent(view: ViewGroup, onGotItClicked: () -> Unit) {