[SnapHelper] Always respect padding when calculating the center of RecyclerView [139452422]

Fixed

Bug

Status Update

No update yet.

Description

je...@google.com

created issue #1

Aug 15, 2019 02:50PM

When PagerSnapHelper calculates the distance between the center of a View and the center of RecyclerView, it only respects RecyclerView's padding when clipToPadding is set to true. However, even when clipToPadding is set to true, the center should be in the middle between the padded edges, not in the middle between the actual edges.

The reasoning is that the padding defines the content rectangle, and whether or not children's pixels are clipped to that rectangle or not doesn't change the semantics of that rectangle.

Consider for example a horizontal ViewPager2 that has unequal padding on the left and the right. When it is in idle state, a page exactly fills the rectangle described by the inner edge of the padding, so the page looks to be off-center by design. When snapping kicks in (e.g., because of a swipe), the page to which it is snapping should end up in that same "off-center" position, exactly matching the rectangle described by the inner edge of the padding, regardless of the value of clipToPadding.

Comments

je...@google.com <je...@google.com> #2Aug 15, 2019 02:51PM

A couple of questions:

1. Have you saw crash in real device or only in simulators?
2. Do you use dynamic feature for language ID?

sh...@google.com <sh...@google.com> #3Aug 15, 2019 11:39PM

Tested on Android 12 Emulator with custom executor, but cannot repro this issue.

je...@google.com <je...@google.com> #4Aug 16, 2019 04:57PM

Reassigned to je...@google.com.

Second crash in the description is from a real device. Experienced it myself on two different Xiaomi phones, plus lots of crashes from users in the Google Play console.
Dynamic features are not used in the application.

As a wild guess, I have downgraded build tools from 31.0.0 to 30.0.3, compileSdk from 31 to 30, and moved all work with Language ID to the service in a separate process (just to be sure that crash can kill secondary process instead of main). This combination is in beta for 2 days by now and I don't see any SIGSEGV crashes.

je...@google.com <je...@google.com> Aug 20, 2019 02:53PM

Accepted by je...@google.com.

ap...@google.com <ap...@google.com> #5Aug 21, 2019 11:57PM

Hmm, I feel the crash might be something related to separate/secondary process.

I also changed compileSdk and targetSDK to 31 but still cannot repro this issue.

je...@google.com <je...@google.com> #6Aug 22, 2019 10:00AM

Status: Assigned (reopened)

On the contrary, there was no separate process before, when crashes started.

In the new build (with the aforementioned changes) I can see SIGSEGV crash, but only one instead of dozens and it has a bit different backtrace:

signal 11 (SIGSEGV), code 1 (SEGV_MAPERR)
liblanguage_id_jni.so (offset 0x11e000)

backtrace:
  #00  pc 000000000003c7c0  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/split_config.arm64_v8a.apk!lib/arm64-v8a/liblanguage_id_jni.so (offset 0x11e000)
  #00  pc 000000000003b960  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/split_config.arm64_v8a.apk!lib/arm64-v8a/liblanguage_id_jni.so (offset 0x11e000)
  #00  pc 000000000003bb48  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/split_config.arm64_v8a.apk!lib/arm64-v8a/liblanguage_id_jni.so (offset 0x11e000)
  #00  pc 000000000003bafc  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/split_config.arm64_v8a.apk!lib/arm64-v8a/liblanguage_id_jni.so (offset 0x11e000)
  #00  pc 0000000000036c98  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/split_config.arm64_v8a.apk!lib/arm64-v8a/liblanguage_id_jni.so (offset 0x11e000)
  #00  pc 0000000000032714  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/split_config.arm64_v8a.apk!lib/arm64-v8a/liblanguage_id_jni.so (offset 0x11e000)
  #00  pc 0000000000031cac  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/split_config.arm64_v8a.apk!lib/arm64-v8a/liblanguage_id_jni.so (offset 0x11e000)
  #00  pc 0000000000057438  /data/app/azagroup.reedy-mF7zTu2bv_ELlbFArwNgqA==/oat/arm64/base.odex (offset 0x57000)

ap...@google.com <ap...@google.com> #7Aug 22, 2019 04:54PM

FYI, ML Kit launched a new language ID SDK in the latest release, which uses a new language ID model. https://developers.google.com/ml-kit/release-notes#august_11_2021

Could you try the new SDK version(17.0.0) to check if you can still repro this native crash? Thanks!

je...@google.com <je...@google.com> Aug 27, 2019 09:54AM

Marked as fixed.