HorizontalPager : beyondBoundsPageCount not working properly [289088847]

Fixed

Bug

Status Update

No update yet.

Description

ad...@decathlon.com

created issue #1

Jun 28, 2023 12:49PM

Jetpack Compose version: androidx.compose:compose-bom:2023.06.01

Jetpack Compose component(s) used: HorizontalPager

Android Studio Build: Build #AI-222.4459.24.2221.10121639, built on May 12, 2023

Kotlin version: 1.8.21

Steps to Reproduce or Code Sample to Reproduce:

Surface( modifier = Modifier.fillMaxSize(), color = MaterialTheme.colorScheme.background ) { // Display 10 items val pagerState = rememberPagerState() HorizontalPager( state = pagerState, pageCount = 5, beyondBoundsPageCount = 0 ) { page -> // Our page content Column(Modifier.fillMaxSize()) { Log.d("TOTO", "Page load :${page + 1}") Text( text = "Page: $page", modifier = Modifier.fillMaxWidth() ) } } }

On load, only first page is displayed -> OK.

first wrong behavior :

Try to scroll to second page, the pager calls page 2 and 3
Try to scroll to third page, the page calls page 4
etc..

second wrong behavior

go to page 2
If you try to scroll just a little to page 1 and release (stay in page 2) page 1(ok) and 4(nok) are loaded

Comments

lp...@google.com <lp...@google.com> Jun 28, 2023 03:47PM

Reassigned to le...@google.com.

le...@google.com <le...@google.com> #2Jul 4, 2023 03:52PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

ad...@decathlon.com <ad...@decathlon.com> #3Jul 5, 2023 06:25AM

Hello, I see but, in the documentation, it's written : By default, the pager only loads the visible pages on-screen. To load more pages offscreen, set beyondBoundsPageCount to a value higher than zero.
So the page 4 shouldn't be loaded.

I work on a huge application with lot of legacy code (XML, fragment etc..) and I use Jetpack compose interoperability. In my use case, each page is an AndroidView composable which contains fragments with viewModels. I have some network calls on each fragments.
For instance, in my case, we call an unique API more than 100 million calls a month. So, I need to optimise the number of calls, and I don't want to preload screens because of useless calls if user doesn't go to these screens

I have a workaround for this but this is tricky and I was expected that beyondBoundsPageCount works with scroll and not only with page selection (pagerState.scrollToPage(tabs.indexOf(it)) works well and beyondBoundsPageCount has good behavior)

Here is my workaround

HorizontalPager(
pageCount = pageCount,
state = pagerState,
) { index ->
if (pagerState.currentPage == index) { <=== THIS DO THE TRICK / ONLY LOAD ACTIVE PAGE
Box(
modifier = Modifier
.fillMaxWidth()
.fillMaxHeight(),
) {
FragmentContainer(
modifier = Modifier
.fillMaxSize(),
fragmentManager = fragmentManager,
factory = { getFragment(index) },
)
}
}
}

Like I said, FragmentContainer is an AndroidView with FragmentContainerView

le...@google.com <le...@google.com> #4Jul 5, 2023 10:09AM

Hey, thanks for reporting. Yeah, I agree that the documentation for beyondBoundsPageCount is a bit confusing and it doesn't make it clear that some pages may be composed due to another mechanism (the prefetcher), we'll update that.

You can think of beyondBoundsPageCount as a guarantee that the given number of pages will be composed on both sides of the active page no matter if the pager is scrolling or stopped. In the meantime, the prefetching mechanism will do prefetching only on scroll (either gesture or animated) on the direction of the scroll.

Unfortunately there isn't an API yet for changing the way the prefetcher works, but we'll consider your use case and maybe we can improve how we're doing it right now. The first issue you reported is expected and it's similar to how ViewPager2 works, if you're at page 0 and start scrolling towards page 1, you'll see page 2 being composed; the second issue however is not.

ad...@decathlon.com <ad...@decathlon.com> #5Jul 5, 2023 12:22PM

Thanks for your answer !
You're right for the first issue behavior, it works like ViewPager2, and I hopped that Compose will fixed this 😅 ..

le...@google.com <le...@google.com> Jul 12, 2023 02:50PM

Reassigned to sh...@google.com.

ap...@google.com <ap...@google.com> #6Jul 24, 2023 12:59PM

Project: platform/frameworks/support
Branch: androidx-main

commit 8c5fbba1fe78ac9d1e7752eae7839d4e661be075
Author: Shahd AbuDaghash <shahddaghash@google.com>
Date: Fri Jul 14 16:14:07 2023

Update behavior of prefetcher.

Changed the behavior of the prefetcher not to perform prefetching as the page bounces back when the scrolling operation does not exceed the threshold to move to the next or previous screens. The condition is if the animator scrolling direction is the same as the direction of the pointer gesture, only then prefetching is performed.

Bug: 289088847
Test: Added test to verify behavior.
Change-Id: Ic742d3276d443948b76874ae04671b9b024609ad

A compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerGestureTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerPrefetcherTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/PagerState.kt

https://android-review.googlesource.com/2657245

ap...@google.com <ap...@google.com> #7Aug 4, 2023 03:45PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit 62b37f3a7b291b9fe74d72f025788671a2dbadb6
Author: Shahd AbuDaghash <shahddaghash@google.com>
Date: Fri Jul 28 17:30:44 2023

Update Pager prefetching behavior to match ViewPager2

The number of pages retained when using ViewPager2 was different from Pager in Compose. Therefore, the number of prefetched pages in Pager was updated to match the behavior in Views. Also, the documentation was fixed to make it clear that there will be pages automatically pre-fetched more than specified in beyondBoundsPageCount.

Fixes: 289088847
Test: Fixed previous tests that were affected by the change.
Relnote: Fixed an issue where the prefetching in Pager did not match the behavior in Views.
Change-Id: I93352e6de29fa8bc23b3af7516872d3e2da7ce0e

M compose/foundation/foundation/api/current.txt
M compose/foundation/foundation/api/restricted_current.txt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/BasePagerTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PageSizeTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerAccessibilityTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerOffscreenPageLimitPlacingTest.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/pager/PagerPrefetcherTest.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/LazyLayoutPager.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/Pager.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/PagerLayoutInfo.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/PagerMeasure.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/PagerMeasureResult.kt
M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/PagerState.kt

https://android-review.googlesource.com/2671298

na...@google.com <na...@google.com> #8Aug 10, 2023 03:47PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.compose.foundation:foundation:1.6.0-alpha03
androidx.compose.foundation:foundation-android:1.6.0-alpha03