koreader / android-luajit-launcher

Android NativeActivity based launcher for LuaJIT, implementing the main loop within Lua land via FFI
MIT License
132 stars 84 forks source link

android 13 cutout #415

Closed yparitcher closed 1 year ago

yparitcher commented 1 year ago

works for me on 13

Please test on <= 12

Fixes: koreader/koreader#9446


This change is Reviewable

pazos commented 1 year ago

Excellent, nicely done BTW :)

Please test on <= 12

It should work just fine. Unfortunately I don't have my dev machine with me until the weekend. Could you upload the APK somewhere?

works for me on 13

And should work for everybody :partying_face:

Could you test what happens on devices that require a view?

https://github.com/koreader/android-luajit-launcher/blob/master/app/src/main/java/org/koreader/launcher/MainActivity.kt#L115-L123

You just need to flip the conditions. IIRC it should work (ie: the NativeSurfaceView's surfaceChanged shouldn't override the activity's surfaceChanged) but I'm not sure anymore :p

Anyhow, none of the devices with NativeSurfaceView have a camera hole, so we're good nevertheless.

NiLuJe commented 1 year ago

I should be able to build/test tomorrow, FWIW ;).

yparitcher commented 1 year ago

Excellent, nicely done BTW :)

I just followed your instructions :)

https://user.fm/files/v2-ee3f4dfb7fe9f25004ddbcf525818773/koreader-android-arm-debug-v2023.04-26-gb4f453eb5_2023-05-07.apk

Could you test what happens on devices that require a view?

https://github.com/koreader/android-luajit-launcher/blob/master/app/src/main/java/org/koreader/launcher/MainActivity.kt#L115-L123

You just need to flip the conditions. IIRC it should work (ie: the NativeSurfaceView's surfaceChanged shouldn't override the activity's surfaceChanged) but I'm not sure anymore :p

Anyhow, none of the devices with NativeSurfaceView have a camera hole, so we're good nevertheless.

sorry, not sure exactly what you want here.

yparitcher commented 1 year ago

Please also test landscape, to make sure i did not mess that up.

pazos commented 1 year ago

sorry, not sure exactly what you want here.

  1. Flip the condition by adding a !, so it ends like
 val surfaceKind: String = if (!device.needsView) {
            view = NativeSurfaceView(this)
            window.takeSurface(null)
            view?.holder?.addCallback(this)
            setContentView(view)
            "SurfaceView"
        } else {
            "Native Content"
        }

so your device ends using "SurfaceView" instead of "Native Content".

  1. Rebuild the APK and test if you get the same results with both.

Again, not so important as there're no devices that require a view and have a cutout. So totally fine if you want to skip it. I can test it myself when I finally update my tablet to android 13

yparitcher commented 1 year ago

the cutout on SurfaceView works, however when i try to open the top menu i get an error:

05-10 21:02:04.064 18158 18190 E NativeThread: Failed to run script: ffi/util.lua:693: bad argument #1 to 'pairs' (table expected, got nil)
05-10 21:02:04.109 18158 18158 V NativeGlue: Pause: 0xe7beb0f0
05-10 21:02:04.162  1203  1488 W ActivityManager: setHasOverlayUi called on unknown pid: 18158
yparitcher commented 1 year ago

aarch64 refuses to build jni/luajit/luajit

NiLuJe commented 1 year ago

aarch64 refuses to build jni/luajit/luajit

It's a GCC issue on debug builds, a release one should go through just fine (but will crash in fun and interesting ways at runtime ;p).

NiLuJe commented 1 year ago

the cutout on SurfaceView works, however when i try to open the top menu i get an error:

05-10 21:02:04.064 18158 18190 E NativeThread: Failed to run script: ffi/util.lua:693: bad argument #1 to 'pairs' (table expected, got nil)
05-10 21:02:04.109 18158 18158 V NativeGlue: Pause: 0xe7beb0f0
05-10 21:02:04.162  1203  1488 W ActivityManager: setHasOverlayUi called on unknown pid: 18158

Do we get a Lua stacktrace somewhere in these cases?

yparitcher commented 1 year ago

aarch64 refuses to build jni/luajit/luajit

It's a GCC issue on debug builds, a release one should go through just fine (but will crash in fun and interesting ways at runtime ;p).

I thought we have a aarch64 build?

yparitcher commented 1 year ago

the cutout on SurfaceView works, however when i try to open the top menu i get an error:

05-10 21:02:04.064 18158 18190 E NativeThread: Failed to run script: ffi/util.lua:693: bad argument #1 to 'pairs' (table expected, got nil)
05-10 21:02:04.109 18158 18158 V NativeGlue: Pause: 0xe7beb0f0
05-10 21:02:04.162  1203  1488 W ActivityManager: setHasOverlayUi called on unknown pid: 18158

Do we get a Lua stacktrace somewhere in these cases?

That is all i got in the adb logs. The koreader bomb page is empty. This was using surfaceview instead of native context. Native context works fine.

NiLuJe commented 1 year ago

Okay, finally tested on Android 12 ;).

First boot was... wonky on several aspects (some of it unrelated: e.g., the statistics DB migration hung until the next input event), at which point it booted up... with the bottom cutoff ;).

Everything's fine after a restart, and Landscape behaves (as does switching from a Landscape book to the Portrait FM).

NiLuJe commented 1 year ago

The SurfaceView implementation also appears to behave over here... except for that first boot issue where it's also cutoff.

Funnily enough, I couldn't reproduce it the second time around when switching back to the Native Content imp.

(And I got to see the spinner on the splash screen, which I never did before, I think?).

Frenzie commented 1 year ago

You should generally see a spinner (as in 3 dots) the first time after any upgrade. It's about 2-3 seconds on my Realme 7 Pro with a Snapdragon 720G.

NiLuJe commented 1 year ago

Yeah, I'm not sure I did in the cases where the first boot was buggy (and the spinner is low, but I don't think it's low enough to fall off-screen in case the splash is affected by a similar issue?).

That said, it was late, I hadn't slept much, so it's entirely possible I just missed it (I was mainly looking at the logcat, not the device's screen) ;).

pazos commented 1 year ago

the cutout on SurfaceView works, however when i try to open the top menu i get an error:

sorry, I missed the notification :p

Who knows? My tablet is going to update to lineage20 soon, so I can try to reproduce there. It seems that android 13 introduced quite a bit of changes that mess with some of our hacks. Work for future us :p

Feel free to merge when you want. @yparitcher