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

Trying to get rid of the black area #430

Closed hugleo closed 9 months ago

hugleo commented 1 year ago

"Following r-1's suggestion on https://github.com/koreader/koreader/issues/10591#issuecomment-1637220961, I got rid of the variables surfaceHeight and surfaceWidth. I don't know if it will break something, but at least it is rendering normally on my devices."


This change is Reviewable

NiLuJe commented 1 year ago

Err, yes, you've just broken notches everywhere ;o).

hugleo commented 1 year ago

Somehow, it seems to be working 100% here. I'm not a detail-oriented guy.

pazos commented 1 year ago

It works on your device because it doesn't have a hole. It screws all devices with holes. Both Android 13 and below.

We can't accept a PR like that

Frenzie commented 1 year ago

If I understand correctly that means the device is reporting bogus values? Since it doesn't have a cutout and all that?

pazos commented 1 year ago

Hi, the latest commit doesn't fix anything. New variables are null so you're returning the same as before.

Please, if you want to fix the issue, ask first. We can give you hints to achieve what you want.

hugleo commented 1 year ago

Just some crazy tests for now.

pazos commented 1 year ago

No problem :)

Always good to see people tinkering in this repo.

In case you would like to do another attempt we could:

  1. revert the changes in https://github.com/koreader/android-luajit-launcher/commit/a9d36a04f4f5a7bddcd46dae5d5cd67031fc7f85 (working for android 4 9 up to android 12)
  2. reapply the changes just for android 13
  3. Based on this comment: https://github.com/koreader/koreader/issues/10591#issuecomment-1620776325 you tell me what we need to do on android 14 :). (either behave like in 9-12, the thing in 13 or that obscure flag window.attributes.layoutInDisplayCutoutMode = WindowManager.LayoutParams.LAYOUT_IN_DISPLAY_CUTOUT_MODE_SHORT_EDGES if this is the new thing to do:))
hugleo commented 1 year ago

Debugging

hugleo commented 1 year ago

I've discovered through debugging that the surfaceChanged function is executing some milliseconds after Koreader captures the event C.APP_CMD_CONFIG_CHANGED. This happens more often on older devices. I believe that eventually, this issue could also happens on Android 13, leading to a black area as Koreader draws with the inverted orientation. I reverted the changes and managed to simulate the notch problem on my device with Android 14. I'll attempt to debug this further.

hugleo commented 1 year ago

More debugging reveals that the problem is even worse for older devices, such as my Android 6 device:

08-07 22:33:52.797 1824 1837 I KOReader: setting zoom mode to manual 08-07 22:33:53.974 1824 1824 V Surface : surface changed { 08-07 22:33:53.974 1824 1824 V Surface : width: 1872 08-07 22:33:53.974 1824 1824 V Surface : height: 1404 08-07 22:33:53.974 1824 1824 V Surface : format: OPAQUE 08-07 22:33:53.974 1824 1824 V Surface : } 08-07 22:33:54.199 1824 1837 V MainActivity: requesting epd update, type: EPD_FULL

08-07 22:35:01.789 548 1848 I ActivityManager: Config changes=480 {1.3 dualscreenflag=DISABLE ?mcc?mnc en_US ldltr sw702dp w702dp h891dp 320dpi lrg port finger -keyb/v/h -nav/h s.25} 08-07 22:35:02.637 1824 1837 V MainActivity: requesting epd update, type: EPD_FULL 08-07 22:37:43.316 1824 1837 V MainActivity: requesting epd update, type: EPD_FULL 08-07 22:38:48.198 548 1486 I ActivityManager: Config changes=480 {1.3 dualscreenflag=DISABLE ?mcc?mnc en_US ldltr sw702dp w936dp h657dp 320dpi lrg land finger -keyb/v/h -nav/h s.26} 08-07 22:38:49.406 1824 1837 V MainActivity: requesting epd update, type: EPD_FULL 08-07 22:41:46.110 548 669 I ActivityManager: Config changes=480 {1.3 dualscreenflag=DISABLE ?mcc?mnc en_US ldltr sw702dp w702dp h891dp 320dpi lrg port finger -keyb/v/h -nav/h s.27}

Do you notice the recurring line "Config changes=480"? This occurs when I rotate my device to the opposite orientation. We should observe lines similar to:

08-07 22:33:53.974 1824 1824 V Surface : surface changed {...

Since the "surfaceChanged" function is not being called, we are not receiving the new dimensions.

hugleo commented 1 year ago

I've reverted commit a9d36a0 and have been testing. It appears that the window.decorView.rootWindowInsets.displayCutout method is not being called in full-screen mode. I have now implemented a new method to obtain the cutout information.

One thing I've noticed is that sometimes when we first install Koreader it requests all file permissions on the Android system screen. If we attempt to open Koreader after this, it get the weights before the onAttachedToWindow method is called, resulting in an incorrect cutout value. I still haven't tried to tackle this issue.

Here is a log demonstrating this behavior:

08-13 11:19:10.718 4421 4421 V Surface : Using Native Content implementation 08-13 11:19:10.735 4421 4483 D Surface : Height notch setted 08-13 11:19:15.742 4421 4421 D Surface : onAttachedToWindow() 08-13 11:19:15.755 4421 4421 V Surface : surface changed { 08-13 11:19:15.755 4421 4421 V Surface : width: 1080 08-13 11:19:15.755 4421 4421 V Surface : height: 1776 08-13 11:19:15.755 4421 4421 V Surface : format: RGB_565 08-13 11:19:15.755 4421 4421 V Surface : }

After closing Koreader and reopening it, the problem appears to be resolved for subsequent instances.

08-13 11:20:38.959 4499 4499 V Surface : Using Native Content implementation 08-13 11:20:39.019 4499 4499 D Surface : onAttachedToWindow() 08-13 11:20:39.151 4499 4499 V Surface : surface changed { 08-13 11:20:39.151 4499 4499 V Surface : width: 1080 08-13 11:20:39.151 4499 4499 V Surface : height: 1776 08-13 11:20:39.151 4499 4499 V Surface : format: RGB_565 08-13 11:20:39.151 4499 4499 V Surface : } 08-13 11:20:39.239 4499 4519 D Surface : Height notch setted

NiLuJe commented 1 year ago

One thing I've noticed is that sometimes when we first install Koreader it requests all file permissions on the Android system screen. If we attempt to open Koreader after this, it get the weights before the onAttachedToWindow method is called, resulting in an incorrect cutout value. I still haven't tried to tackle this issue.

Yup, I definitely noticed that behavior since the PR ;).

hugleo commented 1 year ago

I also took a shot at a different approach by making a few changes to the current code, seems to be working and it may be cleaner than this pull request. You can take a look at the changes in this branch: https://github.com/hugleo/android-luajit-launcher/tree/hugleo-fix-cutout

But now, there's a new issue that popped up yesterday (https://github.com/koreader/koreader/issues/10808), and I'm thinking about whether he is already using the fixed version from commit a9d36a0 and he is using Android 13, which is supposed work at least with the controls, right?

Edit: Learning some github stuffs :P ... and Moved 'hugleo-fix-cutout' to this PR since I believe it's a better. The old method is still available in the branch hugleo-fix-cutout-old-alternative. It's been running on my devices for one month but it still needs testing from others.

NiLuJe commented 1 year ago

Oh, I missed that #10808 was on A13. There may be some Pixel-specific weirdness involved, as I vaguely remember other reports of weirdness on Pixels...

NiLuJe commented 11 months ago

For future reference, what are you testing on?

hugleo commented 11 months ago

For future reference, what are you testing on?

Just some trouble with github from first time user ;-) First renamed hugleo-patch-1 branch and the PR autoclose and chain all of that. Sorry for the boring notifications.

NiLuJe commented 11 months ago

Oh, not that ;p. That was a genuine question, what are you testing the changes on? ;o).

hugleo commented 11 months ago

Oh, not that ;p. That was a genuine question, what are you testing the changes on? ;o).

I tested it some time ago in the Android 13 emulator, both with and without a cutout. I rotated it multiple times and everything looks fine now I don't see the notches anymore. Since then, I've been using it on my mobile phone with a cutout (Android 14) as well as on e-readers Likebook Mars (Android 6) and the Onyx Ultra (Android 11). However it needs to be tested by others in case I missed something.

pazos commented 11 months ago

Hey, I'm unsuscribing to this ticket because it causes a bit of noise on my inbox.

Feel free to play and do crazy tests all you want. If some day this is ready for a review please ping me.

hugleo commented 11 months ago

Hi @pazos

Is my last change. When you have the time to review and test, please share your thoughts. You can go ahead. I'll leave this as is until you're able to do it.

hugleo commented 11 months ago

I really don't understand the intention of the change.

Doing #430 (comment) gets us the proper behaviour on android 4-12 and android 13. If that's not enough for android 14 please review the developers documentation about what changed.

If you don't find anything relevant then feel free to address the oddities guarded by that specific android version.

It was another method that I tried (The intention was just keep the code clean and avoid the checks on getScreenWidth/getScreenHeight). I also implemented reverting the changes in the branch:

https://github.com/hugleo/android-luajit-launcher/tree/hugleo-fix-cutout-old-alternative

I didn't know which one was better and choose one myself. Didn't know how to work to github workflow so didn't put that to review first. Sorry for the trouble.

Can I replace the changes here for review with that instead?

pazos commented 11 months ago

@hugleo:

first see my two comments above.

Can I replace the changes here for review with that instead?

Yes, or open a new PR. Better if you can do it on two different commits.

One that reverts https://github.com/koreader/android-luajit-launcher/commit/a9d36a04f4f5a7bddcd46dae5d5cd67031fc7f85 and other that reapplies the changes there for android 13.

We most likely want to keep the changes here for a few weeks, since any stuff that reaches the main repo will cause a lot of noise on bug reports.

In a couple of days I'll have my dev machine ready to poke a bit with android, something that I didn't seriously for the last couple of years.

pazos commented 9 months ago

superseeded by #439