CODAR747
CODAR747

Reputation: 174

Why my code is causing unnecessary recompositions in jetpack compose?

I have written some code that shows the images and videos. I'm using viewModel but in the code below MediaScreenBody() composable is recomposing unneccesarly even there is no change in the data. Also it causing its child composables to recompose as same time or even more than the count of MediaSceenBpody() . I tried many ways like using by with collectAsState but those were causing even 10x recompositons. At the first time when app is launched there is only one composition of every single composable. And then if i interact with app e.g. swiping or scrolling then it recomposes the MediaSceenBpody() and its child content as well. It also causing lagging behavior to the scrolling and swiping due to recompositions. I did somereseach and i found that the pagerState may be causing this issue. I tried setting the pagerState to diffenrt number and recompositions are also changed to that number. But please keep inmind that somtimes it causes more than that. Its not neccessary there that it will recompose only to the number of pageState. I have shared this just for your reference.

@Composable
fun MediaScreen(onMediaItemClick: () -> Unit) {
    val mediaViewModel: MediaViewModel = viewModel(
        factory = MediaViewModel.Factory
    )
    val photoUIState = mediaViewModel.photoUIState.collectAsState()
    val videoUIState = mediaViewModel.videoUIState.collectAsState()

    ScreenContent(
        mediaViewModel = mediaViewModel,
        photoUIState = photoUIState,
        videoUIState = videoUIState,
        onMediaItemClick = onMediaItemClick
    )


}


@Composable
fun ScreenContent(
    modifier: Modifier = Modifier,
    mediaViewModel: MediaViewModel,
    photoUIState: State<PhotoUIState>,
    videoUIState: State<VideoUIState>,
    onMediaItemClick: () -> Unit
) {
    val topBarScrollBehavior = TopAppBarDefaults.enterAlwaysScrollBehavior()
    Scaffold(
        modifier = modifier
            .nestedScroll(topBarScrollBehavior.nestedScrollConnection),
        topBar = {
            TopAppBarContent(topBarScrollBehavior)
        }
    ) { contentPadding ->
        MediaScreenBody(
            contentPadding = contentPadding,
            mediaViewModel = mediaViewModel,
            photoUIState = photoUIState,
            videoUIState = videoUIState,
            onMediaItemClick = onMediaItemClick
        )

    }
}


@Composable
fun TopAppBarContent(scrollBehavior: TopAppBarScrollBehavior? = null) {
    TopAppBar(
        title = {
            Text(
                text = stringResource(id = R.string.app_name)
            )
        },
        colors = TopAppBarDefaults.topAppBarColors(
            containerColor = if (isSystemInDarkTheme()) cs_theme_dark_topBar else cs_theme_light_topBar,
            titleContentColor = Color.White
        ),
        scrollBehavior = scrollBehavior
    )
}

@Composable
fun MediaScreenBody(
    modifier: Modifier = Modifier,
    mediaViewModel: MediaViewModel,
    contentPadding: PaddingValues,
    photoUIState: State<PhotoUIState>,
    videoUIState: State<VideoUIState>,
    onMediaItemClick: () -> Unit,
) {

    val pagerState = rememberPagerState {
        3
    }

    var selectedTabIndex by rememberSaveable {
        mutableIntStateOf(0)
    }

    Column(
        modifier = Modifier
            .padding(contentPadding)
            .fillMaxSize()
    ) {
        TabRow(
            selectedTabIndex = selectedTabIndex,
            containerColor = if (isSystemInDarkTheme()) cs_theme_dark_topBar else cs_theme_light_topBar,
            contentColor = Color.White
        ) {
            tabs.forEachIndexed { index, tabData ->
                Tab(
                    selected = index == selectedTabIndex,
                    onClick = { selectedTabIndex = index },
                    text = { Text(text = tabData.title) }
                )
            }
        }

        HorizontalPager(
            modifier = modifier.fillMaxSize(),
            state = pagerState
        ) { currentPage ->
            when (currentPage) {
                0 -> {
                    when (photoUIState.value.mediaPhotos) {
                        is PhotoLoadingState.Loading -> LoadingScreen()
                        is PhotoLoadingState.Success -> SuccessScreen(
                            mediaList = (photoUIState.value.mediaPhotos as PhotoLoadingState.Success).photos,
                            onDownloadClick = {
                                val isSaved = mediaViewModel.saveMedia(it)
                            },
                            onMediaItemClick = onMediaItemClick
                        )

                        is PhotoLoadingState.Error -> ErrorScreen(onRetry = { mediaViewModel.loadPhotos() })
                    }
                }

                1 -> {
                    when (videoUIState.value.mediaVideos) {
                        is VideoLoadingState.Loading -> LoadingScreen()
                        is VideoLoadingState.Success -> SuccessScreen(
                            mediaList = (videoUIState.value.mediaVideos as VideoLoadingState.Success).videos,
                            onDownloadClick = {},
                            onMediaItemClick = onMediaItemClick
                        )

                        is VideoLoadingState.Error -> ErrorScreen(onRetry = { mediaViewModel.loadVideos() })
                    }
                }

                else -> {
                    val context = LocalContext.current
                    Button(onClick = {
                        Toast.makeText(
                            context,
                            "I'm working m third pager",
                            Toast.LENGTH_LONG
                        ).show()
                    }) {
                        Text(text = "Hello")
                    }
                }
            }
        }
    }
}

Edit - As there were many suggestions about not passing the state or viemodel as parameter. I tried calling it direclty within the MediaScreenBody() where HorizontalPager is used. But it is causing more than 40-50 recompositon when content is scrolled its got more worse. This is how i tried moving the code to MediaScreenBody() -

val mediaViewModel: MediaViewModel = viewModel(
        factory = MediaViewModel.Factory
    )
    val photoUIState = mediaViewModel.photoUIState.collectAsState()
    val videoUIState = mediaViewModel.videoUIState.collectAsState()

Upvotes: 0

Views: 467

Answers (2)

Semanticer
Semanticer

Reputation: 1982

This is a known bug in jetpack compose material library, and it has been finally fixed and released in

androidx.compose.material3:material3-*:1.4.0-alpha03

See release notes

Upvotes: 0

Ajay Chandran
Ajay Chandran

Reputation: 254

The problem for the unnecessary recompositions is because of ViewModel. A viewmodel is always considered unstable in a composable method. To avoid such recompositions, you can consider the below two ways:

  1. Never pass the viewmodel to your child composables. Have a callback to intimate the event and use your viewmodel in that callback.

In your case, you can avoid passing viewmodel to the MediaScreenBody() method and use viewmodel in ScreenContent() method itself.Something like this:

fun MediaScreenBody(
modifier: Modifier = Modifier,
contentPadding: PaddingValues,
onMediaItemClick: () -> Unit,
onPhotoError: () -> Unit,
onVideoError: () -> Unit
)

You can use the last two callbacks to load from viewmodel again.

  1. Declare a lambda outside the method call where you tend to use viewmodel. Sometimes using the viewmodel in the callback directly can lead to unnecessary recompositions. A sample is given below:

    val onClickLambda = remember<() -> Unit> { { // viewmodel access } }

    @Composable fun Test() { Button(onClick = onClickLambda) {} }

Upvotes: 0

Related Questions