google / accompanist

A collection of extension libraries for Jetpack Compose
https://google.github.io/accompanist
Apache License 2.0
7.34k stars 593 forks source link

[Pager] HorizontalPager not working as expected with Paging3 #1289

Closed mehul4795 closed 1 year ago

mehul4795 commented 1 year ago

Description

There are two screens in my app. The first screen shows the list of images on the device by using the Paging3 library in a vertical grid. Now, when the user clicks on an image, I am passing the click position to the second screen where I am using HorizontalPager to show the images in full screen. Both the screens share the same ViewModel to fetch images using Paging3.

When the second screen is opened, the clicked image is shown in full screen as expected. However, when the user swipes for the next image, instead of loading the next image, the image with the index 50 and so on gets loaded as the user swipes further.

The code to show the images in HorizontalPager is shown below.

val images: LazyPagingItems<MediaStoreImage> =
    viewModel.getImages(initialLoadSize = args.currentImagePosition + 1, pageSize = 50)
        .collectAsLazyPagingItems()

val pagerState = rememberPagerState(initialPage = currentImagePosition)

Box(modifier = modifier) {
    HorizontalPager(
        count = images.itemCount,
        state = pagerState,
        itemSpacing = 16.dp
    ) { page ->
        ZoomableImage(
            modifier = modifier,
            imageUri = images[page]?.contentUri
        )
    }
}

Expected behavior

When the user swipes, the next image in the list should be loaded.

Additional context

In the above code, currentImagePosition is the index of the image clicked on the first screen. I am setting the initialLoadSize to currentImagePosition + 1 which makes sure that the clicked image to be shown is already fetched by the paging library.

andkulikov commented 1 year ago

Is you getImages() always returning a new flow? It could be the issue as when this composable is rerun you just recreate a flow and we collect it again and so on. The returned Flow should be remembered as composable functions could be reexecuted

mehul4795 commented 1 year ago

The getImages() function has the code shown below.

fun getImages(initialLoadSize: Int = 50): Flow<PagingData<MediaStoreImage>> {
        return Pager(
            config = PagingConfig(
                pageSize = 50,
                initialLoadSize = initialLoadSize,
                enablePlaceholders = true
            )
        ) {
            repository.getImagesPagingSource()
        }.flow.cachedIn(viewModelScope)
    }
andkulikov commented 1 year ago

try to write your code like this

val images: LazyPagingItems<MediaStoreImage> =
    remember { viewModel.getImages(initialLoadSize = args.currentImagePosition + 1, pageSize = 50) }
        .collectAsLazyPagingItems()

curious if it will help

mehul4795 commented 1 year ago

The above code did solve the problem and the clicked image is shown and the next images are shown as the user swipes as expected. However, although there is total of 76 images to be fetched, only the next page is fetched and when I reach the end of the next page, a new page isn't fetched. One more thing to note, although I have set the page size to 50, the next page only loads 27 items. I have attached the screenshots of the log for reference.

Screenshot 2022-09-09 at 8 30 36 PM

The above log is for the case when I click on the first image on the first screen.

Screenshot 2022-09-09 at 8 31 07 PM

The above log is for the case when I click on the eleventh image on the first screen.

Note

For both cases, I swiped till the end of the second page and no more images were fetched. Also, Current Image Pos in the log is the position of the image clicked on the first screen.

mehul4795 commented 1 year ago

In case you want to have a look at the remaining code, I have added it to the StackOverflow question here.

andkulikov commented 1 year ago

This seems like an issue with Paging now, not Pager. Could you please try to reproduce it without Pager(by using LazyRow instead of HorizontalPager, for example) and file a bug in the androidx tracker

mehul4795 commented 1 year ago

I think I have solved the issue. Whenever the second screen was opened, the getImages() function of the shared ViewModel was called which created a new instance of Pager. This new instance of the Pager was different from the one used on the first screen. Referring to this answer on StackOverflow, I created the Pager in the init block of the ViewModel as shown below.

    val images: Flow<PagingData<MediaStoreImage>>

    init {
        images = Pager(
            config = PagingConfig(
                pageSize = 50,
                initialLoadSize = 50,
                enablePlaceholders = true
            )
        ) {
            repository.getImagesPagingSource()
        }.flow.cachedIn(viewModelScope)
    }

On the second screen, I collected the paging items as shown below.

val lazyImages: LazyPagingItems<MediaStoreImage> = viewModel.images.collectAsLazyPagingItems()

Now, I didn't have to pass the initialLoadSize as the Pager created in the first screen was being reused which had already loaded the items.

andkulikov commented 1 year ago

Great, thanks for confirming. It works as expected