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

Include rounded corner radius in android safe area implementation #6143

Merged
merged 3 commits into from Jan 21, 2024

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Jan 18, 2024

Android apparently has a separate API for "window cutouts", i.e. for cameras and the like, and a separate API for "rounded corners", i.e. devices where the rounded corners of the display will cause some areas of the theoretically-rectangular display to be obscured. Because there is no engineering like overengineering, right? Bonus points for them being usable at different API levels, too.

Disclaimer that this does absolutely nothing on my one test device that has a rounded display and a high enough API level, but honestly it may be a device issue since it's not even returning true for WindowInsets.IsRound. Maybe it'll help someone, no idea.

Android apparently has a separate API for "window cutouts", i.e.
for cameras and the like, and a separate API for "rounded corners",
i.e. devices where the rounded corners of the display will cause some
areas of the theoretically-rectangular display to be obscured.
Because there is no engineering like overengineering, right?
Bonus points for them being usable at different API levels, too.

Disclaimer that this does absolutely nothing on my one test device
that has a rounded display and a high enough API level, but honestly
it may be a device issue since it's not even returning `true`
for `WindowInsets.IsRound`. Maybe it'll help someone, no idea.
@LittleEndu
Copy link
Contributor

LittleEndu commented Jan 18, 2024

Reading the code, I'm guessing that the resulting safe area is less than optimal.

image

It would make way more sense if the code only cut away from the longest screen side (resulting in what I colored orange), or if optimizing for most play area only 1-sqrt(2)/2 of the radius should be cut away (resulting in what I colored green). Hopefully someone can test what the resulting screen actually looks like.

Also there's a TODO still left in.

Copy link
Member

@Susko3 Susko3 left a comment

Choose a reason for hiding this comment

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

I've yet to test this on a real device, but the native android stuff looks correct.

You should remove this todo about rounded corners:

https://github.com/ppy/osu-framework/blob/f67b4bfa910f0d23970a8d6a7d0a9afa262e4706/osu.Framework.Android/AndroidGameView.cs#L276C1-L276C1

Comment on lines 260 to 261
var radiusInsetArea = screenArea.Shrink(cornerInsetLeft, cornerInsetRight, cornerInsetTop, cornerInsetBottom);
usableScreenArea = usableScreenArea.Intersect(radiusInsetArea);
Copy link
Member

Choose a reason for hiding this comment

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

I think the math here is too conservative. See the illustration (green would be considered the safe area):

image

It should either apply top & bottom or left & right padding, not both at the same time.

@Susko3
Copy link
Member

Susko3 commented Jan 18, 2024

I'm more for the orange solution as it matched what other games do & doesn't complicate things with 4-side padding. The green solution is too simple and not a true optimal area, as it doesn't consider the width and height of the screen (if the screen is really tall, it would be optimal to push the corner point towards the orange solution).

@bdach
Copy link
Collaborator Author

bdach commented Jan 18, 2024

can anyone with an iOS device say what iOS does with rounded corners? @frenzibyte @peppy?

@Susko3
Copy link
Member

Susko3 commented Jan 18, 2024

I think this is accurate, except the bottom safe area edge is overriden locally.

safe_area

@frenzibyte
Copy link
Member

iPhone 12:

image

@bdach
Copy link
Collaborator Author

bdach commented Jan 18, 2024

that looks weird because it cuts out mostly at the long edge but also appears to cut out a bit on the short edge?

i dunno. think i'm just gonna make it cut at the long edge and hope everyone here can be satisfied with that.

int cornerInsetTop = Math.Max(topLeftCorner?.Radius ?? 0, topRightCorner?.Radius ?? 0);
int cornerInsetBottom = Math.Max(bottomLeftCorner?.Radius ?? 0, bottomRightCorner?.Radius ?? 0);

var radiusInsetArea = screenArea.Width >= screenArea.Height
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i hope i can be forgiven for probably imperfectly handling square screens

Copy link
Contributor

Choose a reason for hiding this comment

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

semi serious, safe area is a setting, right? This isn't going to break someone using osu-framework on a smart watch?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's not a setting, and i have absolutely no idea, can we skip that question?

@peppy
Copy link
Sponsor Member

peppy commented Jan 18, 2024

that looks weird because it cuts out mostly at the long edge but also appears to cut out a bit on the short edge?

that's just the bezel

Copy link
Member

@Susko3 Susko3 left a comment

Choose a reason for hiding this comment

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

Tested to work as expected when hard-coding corner radii.

@Joehuu Joehuu self-requested a review January 20, 2024 21:55
Copy link
Member

@Joehuu Joehuu left a comment

Choose a reason for hiding this comment

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

LGTM on Pixel 6a.

Copy link
Sponsor Member

@peppy peppy left a comment

Choose a reason for hiding this comment

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

Looks sane

@peppy peppy merged commit a5bfffb into ppy:master Jan 21, 2024
19 of 21 checks passed
@bdach bdach deleted the rounded-corners-android branch January 22, 2024 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants