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.

Sign up for GitHub

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

Jump to bottom

Reverted cutout and fix rotations #439

Merged
merged 8 commits into from Dec 22, 2023
Merged

Reverted cutout and fix rotations #439

Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7eba2bc
Revert "android 13 cutout"
hugleo Sep 24, 2023
ac84107
merge changes
hugleo Sep 24, 2023
475ca95
clean up
hugleo Sep 24, 2023
e263f96
missed one
hugleo Sep 24, 2023
9c57506
address nitpicks
hugleo Dec 15, 2023
9bb9154
remove leftovers from my personal build
hugleo Dec 15, 2023
25b2d11
Remove the possibly unneeded inset bottom
hugleo Dec 16, 2023
19adfe1
remove and deduplicated
hugleo Dec 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 74 additions & 4 deletions app/src/main/java/org/koreader/launcher/MainActivity.kt
View file
Open in desktop
Expand Up @@ -45,9 +45,13 @@ class MainActivity : NativeActivity(), LuaInterface,
private var lastImportedPath: String? = null

// Surface height & width determined at runtime to account for device cutout
// ONLY FOR ANDROID 13
private var surfaceHeight: Int? = null
private var surfaceWidth: Int? = null

// Device cutout - only used on API 28+
private var topInsetHeight: Int = 0

// Fullscreen - only used on API levels 16-18
private var fullscreen: Boolean = true

Expand Down Expand Up @@ -165,12 +169,46 @@ class MainActivity : NativeActivity(), LuaInterface,
"surface changed {\n width: %d\n height: %d\n format: %s\n}",
width, height, pixelFormatName(format))
)
surfaceWidth = width
surfaceHeight = height

// ONLY FOR ANDROID 13
if (android.os.Build.VERSION.SDK_INT == 33) {
surfaceWidth = width
surfaceHeight = height
}

super.surfaceChanged(holder, format, width, height)
drawSplashScreen(holder)
}

override fun onAttachedToWindow() {
Log.d(TAG_SURFACE, "onAttachedToWindow()")
super.onAttachedToWindow()

// ONLY FOR ANDROID 13
if (android.os.Build.VERSION.SDK_INT == 33) {
// nothing to do
}
// ONLY FOR ANDROID 14
else if (android.os.Build.VERSION.SDK_INT == 34) {
val cut = windowManager.defaultDisplay.cutout
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this chunk of code works for android 9 - 12?

It would be wonderful to have a single codepath for all versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this chunk of code works for android 9 - 12?

It would be wonderful to have a single codepath for all versions.

That's what I would like to show you!

for android <= 12 as you say so works.
val cut: DisplayCutout? = window.decorView.rootWindowInsets.displayCutout

for android 13 this seems to be the secret:
windowManager.defaultDisplay.cutout
Following the doc this method should work for >= P

For now I've tested on devices without the hole on some devices < 13 and works.
But I can test some more on the emulator enabling holes in the dev options.

Did you remember to try the method windowManager.defaultDisplay.cutout before?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you remember to try the method windowManager.defaultDisplay.cutout before?

I actually don't remember. I think I didn't try anything. Just read the documentation, find something useful, went with that and pushed an APK for others to test. Since the test were ok I never bothered again :)

So please go with that for everything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you remember to try the method windowManager.defaultDisplay.cutout before?

I actually don't remember. I think I didn't try anything. Just read the documentation, find something useful, went with that and pushed an APK for others to test. Since the test were ok I never bothered again :)

So please go with that for everything.

Tested on emulator and doesn't work for Android 9. Need Android >= 10

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks!.

Then from 9 up to 12 the legacy method and from 13 onwards the new one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks!.

Then from 9 up to 12 the legacy method and from 13 onwards the new one.

Already there ;-)

if (cut != null && cut.boundingRects.isNotEmpty()) {
// TODO: we can handle various kinds of cutouts: getSafeInsetLeft, getSafeInsetRight, getSafeInsetTop, getSafeInsetBottom
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of random TODO lists.

Do you have something in mind to do with those cutouts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of random TODO lists.

Do you have something in mind to do with those cutouts?

Yes. Today we only deal with topInsetHeight when the hole is at the top.
On the emulator, we can enable holes in any direction: top, bottom, left, or right.
When you guys changed surfaceWidth = width; surfaceHeight = height inside the surfaceChanged method, it enabled support for all kinds of holes.
When I reverted the changes I realized that it was not implemented in the old version ;-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so remove the TODO and add a NOTE instead.
Somebody else that reads the code in the future might find that useful.

Not so with a TODO.

topInsetHeight = cut.safeInsetTop
}
}
else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
val cut: DisplayCutout? = window.decorView.rootWindowInsets.displayCutout
if (cut != null) {
val cutPixels = cut.safeInsetTop
if (topInsetHeight != cutPixels) {
Log.v(TAG_SURFACE,
"top $cutPixels pixels are not available, reason: window inset")
topInsetHeight = cutPixels
}
}
}
}

/* Called just before the activity is resumed by an intent */
override fun onNewIntent(intent: Intent) {
val scheme = intent.scheme
Expand Down Expand Up @@ -424,7 +462,23 @@ class MainActivity : NativeActivity(), LuaInterface,
}

override fun getScreenHeight(): Int {
return surfaceHeight ?: getHeight()
// ONLY FOR ANDROID 13
if (android.os.Build.VERSION.SDK_INT == 33) {
return surfaceHeight ?: getHeight()
}

return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
// We need to handle the notch in Portrait
// NOTE: getScreenAvailableHeight does it automatically, but it also excludes the nav bar, when there's one :/
if (getOrientationCompat(screenIsLandscape).and(1) == 0) {
// getScreenOrientation returns LinuxFB rotation constants, Portrait rotations are always even
getHeight() - topInsetHeight
} else {
getHeight()
}
} else {
getHeight()
}
}

override fun getScreenMaxBrightness(): Int {
Expand Down Expand Up @@ -452,7 +506,23 @@ class MainActivity : NativeActivity(), LuaInterface,
}

override fun getScreenWidth(): Int {
return surfaceWidth ?: getWidth()
// ONLY FOR ANDROID 13
if (android.os.Build.VERSION.SDK_INT == 33) {
return surfaceWidth ?: getWidth()
}

return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
// We need to handle the notch in Landscape
// NOTE: getScreenAvailableWidth does it automatically, but it also excludes the nav bar, when there's one :/
if (getOrientationCompat(screenIsLandscape).and(1) == 1) {
// getScreenOrientation returns LinuxFB rotation constants, Landscape rotations are always odd
getWidth() - topInsetHeight
} else {
getWidth()
}
} else {
getWidth()
}
}

override fun getStatusBarHeight(): Int {
Expand Down