Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Matrix.setValues doesn't accept value array with size >9 #8369

Open
zach-klippenstein opened this issue Jul 25, 2023 · 11 comments · Fixed by #8371
Open

Matrix.setValues doesn't accept value array with size >9 #8369

zach-klippenstein opened this issue Jul 25, 2023 · 11 comments · Fixed by #8371

Comments

@zach-klippenstein
Copy link
Contributor

zach-klippenstein commented Jul 25, 2023

Description

If you call android.graphics.Matrix.setValues with an array of more than 9 elements, it throws an ArrayIndexOutOfBoundsException. This does not happen on Android, where it throws for less than 9 values, but accepts >9 values.

This prevents an optimization in Jetpack Compose where we re-use a 4x4 array when converting between Compose's Matrix type and the system one.

Steps to Reproduce

Create an android.graphics.Matrix, a 16-element array, and pass it to setValues.

Robolectric & Android Version

Robolectric: 4.9.2, 4.10.3
Android: Every version Jetpack Compose tests on: 21, 26, 28, 30, 33.

Link to a public git repo demonstrating the problem:

https://r.android.com/2647564

@zach-klippenstein
Copy link
Contributor Author

Workaround in https://r.android.com/2675858.

@zach-klippenstein zach-klippenstein changed the title Matrix.setValues doesn't accept > 9 values Matrix.setValues doesn't accept value array with size >9 Jul 25, 2023
@utzcoz
Copy link
Member

utzcoz commented Jul 26, 2023

@zach-klippenstein Thanks for filing this issue. I think it is caused by

if (values.length != 9) {
throw new ArrayIndexOutOfBoundsException();
}
. I think I can fix it this weekend or @hoisie or other folks can help it if they have time resources this week.

@utzcoz
Copy link
Member

utzcoz commented Jul 26, 2023

@zach-klippenstein If you can ensure RobolectricComposeTest.kt only run on macOS and Linux, you can enable native graphics with Config to use realistic implementation of Matrix(RNG use graphics libraries built from AOSP source code).

@zach-klippenstein
Copy link
Contributor Author

Thanks for the suggestion, TIL! That could solve my immediate problem, but I also don't want to break downstream users of Compose who are running their own Robolectric tests and might be using Windows.

@zach-klippenstein
Copy link
Contributor Author

I tried applying the annotation @GraphicsMode(GraphicsMode.Mode.NATIVE) to the test class, but it's still hitting the legacy matrix code when running on macOS. Do I need to pass something to my @Config annotation as well?

zach-klippenstein added a commit to zach-klippenstein/robolectric that referenced this issue Jul 26, 2023
If the array is <9, and ArrayIndexOutOfBoundsException is still thrown.

Tested by new tests in `ShadowMatrixTest`.

Fixes robolectric#8369
zach-klippenstein added a commit to zach-klippenstein/robolectric that referenced this issue Jul 26, 2023
If the array is <9, and ArrayIndexOutOfBoundsException is still thrown.

Tested by new tests in `ShadowMatrixTest`.

Fixes robolectric#8369
utzcoz pushed a commit that referenced this issue Jul 27, 2023
If the array is <9, and ArrayIndexOutOfBoundsException is still thrown.

Tested by new tests in `ShadowMatrixTest`.

Fixes #8369
@utzcoz
Copy link
Member

utzcoz commented Jul 27, 2023

@hoisie Could we have a 4.10.4 minor patch release? We can cherry-pick this one and other important commits to this release. In AndroidX part, the 4.10.3 integration has been reverted by https://android-review.googlesource.com/c/platform/frameworks/support/+/2623613 because of flaky crashes. I don't know whether related issues have been fixed or it is related to Robolectric only. @zach-klippenstein Maybe you can ask Aurimas for extra informations if you want the Robolectic 4.10.4 before 4.11.

@utzcoz utzcoz reopened this Jul 27, 2023
@utzcoz
Copy link
Member

utzcoz commented Jul 27, 2023

Reopen for later integration discussion.

@zach-klippenstein
Copy link
Contributor Author

Just talked to Aurimas, we're finally on 4.10.3. It's not super urgent to have this fix available, we just need it for part of the implementation of the new Compose text field rewrite. Do you know roughly when 4.11 is targeting release? If it's some time in the next couple months we can probably wait for that.

@utzcoz
Copy link
Member

utzcoz commented Jul 31, 2023

@zach-klippenstein I think 4.11 will be released after AOSP 14 source code released and the Android 14 support is integrated into Robolectric.

@zach-klippenstein
Copy link
Contributor Author

I believe Android 14 source has been released. Do you have a rough ETA for when Robolectric might integrate it?

@hoisie
Copy link
Contributor

hoisie commented Oct 5, 2023

We are in the process of releasing support for Android 14 (U).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants