search

nvanbenschoten / motion

An Android library allowing images to exhibit a parallax effect that reacts to the device's tilt
Apache License 2.0
776 stars 89 forks source link

IllegalArgumentException: R array length must be 3 or 4 #16

Open PatrickDattilio opened 9 years ago

PatrickDattilio commented 9 years ago

java.lang.IllegalArgumentException: R array length must be 3 or 4 at android.hardware.SensorManager.getRotationMatrixFromVector(SensorManager.java:1336) at com.nvanbenschoten.motion.SensorInterpreter.setTargetVector(SensorInterpreter.java:134) at com.nvanbenschoten.motion.SensorInterpreter.interpretSensorEvent(SensorInterpreter.java:74) at com.nvanbenschoten.motion.ParallaxImageView.onSensorChanged(ParallaxImageView.java:119) at android.hardware.SystemSensorManager$SensorEventQueue.dispatchSensorEvent(SystemSensorManager.java:463) at android.os.MessageQueue.nativePollOnce(MessageQueue.java) at android.os.MessageQueue.next(MessageQueue.java:132) at android.os.Looper.loop(Looper.java:124) at android.app.ActivityThread.main(ActivityThread.java:5414) at java.lang.reflect.Method.invokeNative(Method.java) at java.lang.reflect.Method.invoke(Method.java:525) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1187) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003) at dalvik.system.NativeStart.main(NativeStart.java)

darwind commented 9 years ago

I didn't see that someone else opened an issue about this exact thing a few hours before me. See my issue: https://github.com/nvanbenschoten/motion/issues/17

Are you experiencing this on all different devices or is also related to Samsung devices and Android 4.3 too?

PatrickDattilio commented 9 years ago

100% on Samsung devices, running Android 4.3 as well.

nvanbenschoten commented 9 years ago

Hey guys, thanks for bringing this to my attention. At this point, I'd rather not just throw a try-catch around the getRotationMatrixFromVector() call, because it is in such a hot code path that the performance hit from continuously throwing exceptions on machines with this issue could be devastating. Because of that, I'm looking into alternatives of potentially trying to detect the issue on initialization and reacting accordingly.

Do either of you know a way that this can be reproduced so that I can test either manually or through unit tests? Worst case, I'm thinking we could write a unit test that mocks the exception throwing conditions.

darwind commented 9 years ago

@nvanbenschoten - I haven't been able to reproduce it on a Samsung S3 or a S4 mini, but what we see in our crashlogs in Crashlytics is that it's only Samsung S4 and Note 3 that crashes on Android 4.3 - I don't have any of those devices available I'm afraid.

We've tried to reproduce on a Samsung S3 with stock Android 4.3 (Samsung stock rom), but we didn't manage to reproduce.

In my link to a Google group the last poster links to the Chromium project: https://src.chromium.org/viewvc/chrome/trunk/src/content/public/android/java/src/org/chromium/content/browser/DeviceMotionAndOrientation.java?pathrev=246398

Where this issue was actually fixed by adding this check in the onSensorChanged method from the SensorEventListener:

194 if (values.length > 4) { 195 // On some Samsung devices SensorManager.getRotationMatrixFromVector 196 // appears to throw an exception if rotation vector has length > 4. 197 // For the purposes of this class the first 4 values of the 198 // rotation vector are sufficient (see crbug.com/335298 for details). 199 if (mTruncatedRotationVector == null) { 200 mTruncatedRotationVector = new float[4]; 201 } 202 System.arraycopy(values, 0, mTruncatedRotationVector, 0, 4); 203 getOrientationFromRotationVector(mTruncatedRotationVector); 204 } else { 205 getOrientationFromRotationVector(values); 206 }

What is most annoying is that I can't test or reproduce the issue and hence can't fix it. I would be happy to try this "fix" out in our production app - it can't be worse than it is I suppose ;-)

On a side-note: I think to be able to mock this you will need Samsungs official Android 4.3 rom to reproduce. It seems like they're throwing an exception where the original code from Google is just throwing away the extra floats in the array.

nvanbenschoten commented 9 years ago

@PatrickDattilio @darwind I just proposed a fix for this issue using a similar approach to Chrome. Let me know what you think.

Because it sounds like none of us are in a good situation to reproduce the issue on demand, I'm wondering what the best way to make sure this completely fixes it. @darwind would you be willing to monitor this fix on your production app and report back if it looks like it has fixed the issue?

darwind commented 9 years ago

Hey @nvanbenschoten I got my hands on a Samsung Galaxy Note 3 and will try to downgrade it tomorrow to Android 4.3 and see if I can reproduce and then test your fix.

We have a scheduled release tomorrow so the fix won't make it into our production app until a few weeks from today I'm afraid, but anything else fails I'll put the fix in the production app and "test" it on our users :-)

I'll keep you posted, how it goes tomorrow...

And thanks for the fast response ;-)

darwind commented 9 years ago

@nvanbenschoten apparently it's not that easy to downgrade a Note 3 from Lollipop to Jellybean... so I didn't manage to test this fix on it.

Either I get a device working with Jellybean or the fix ends up in production in next release. I'll report back when I have any progress.

nvanbenschoten commented 9 years ago

@darwind Do you have any updates about this fix? Is the crash still being reported even with the patch in place?

darwind commented 9 years ago

Hi @nvanbenschoten sorry for not responding. We have a release within the next few weeks if everything goes well.

I'll try to "sneak" the fix in ;-) Can I include it from Gradle or do I need to use the JAR file?