-
Notifications
You must be signed in to change notification settings - Fork 50
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
Android backend #130
base: master
Are you sure you want to change the base?
Android backend #130
Conversation
Currently the web backend converts to RGBA on present. We can do the same here, if we want to merge this before we figure out #98. Ultimately we'll want that to provide an API that lets a client enumerate supported formats (some backends may support only BGRA, only RGBA, or both, etc.). And either use a native format or provide an easy way to convert when needed. |
Adding a So that can be added to the API when #98 is implemented. |
No fuss/hurry to get this merged before fleshing out the details.
Sounds like a plan.
It's necessary unless we code in the conversion on present. And Android might again do a conversion after that. |
3e41353
to
2c40bc2
Compare
0a7f720
to
d5cc95a
Compare
examples/utils/winit_app.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any clue why this file used an include!()
hack? Making it a module makes sure cargo fmt
tackles it, and probably makes it easier to have jump to symbol and friends working properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't use modules in examples, I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can if you put it directly in the examples folder and exclude it as an example via cargo targets.
(would have tried it out, but am on my phone right now)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can definitely use mod
ules in examples. The only thing you have to do, which is already done here and as linked "somewhat" by @daxpedda, is to make sure that the utilities library doesn't get picked up by autoexamples = true
. This is already done, because the only files that get picked up are examples/<example name>.rs
and examples/<example name>/main.rs
. As there's no main.rs
inside the utils/
folder, there's no utils
example target.
src/backends/android.rs
Outdated
} | ||
|
||
pub fn present_with_damage(self, _damage: &[Rect]) -> Result<(), SoftBufferError> { | ||
Err(SoftBufferError::Unimplemented) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can just do the same thing as present()
and ignore the damage. Which is only an optimization.
src/backends/android.rs
Outdated
} | ||
|
||
pub fn age(&self) -> u8 { | ||
todo!() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be valid to use 0
so the caller should assume every buffer is new and needs to be filled completely.
Not sure if ANativeWindow_Buffer
involves some kind of double/triple/etc. buffering. Presumably it would, though I don't see an API exposing the "age". I guess Android is handling this a bit different with the inOutDirtyBounds
argument of ANativeWindow_lock
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Android works with buffer pools and buffer queues, to implement multi-buffering. The underlying function is defined as:
Lock the window's next drawing surface for writing.
Previous data is probably undefined, if a different handle is returned most of the time. However, I've implemented a Vec
with a copy of the data, which we might use for this (and to implement the new fn fetch()
?).
In a quick test I just kept track of the raw pointers returned from this function. After 3 calls the pointer repeats, and always in the same order. If I don't write to this pointer the old data is preserved.
I have no clue if this is something we can rely on however, since Android doesn't specify the lifetime of this CPU handle. And depending on the phone hardware, display presentation unit and UMA, may or may not have to copy this CPU visible memory (and discard it right after!) to make it presentable.
Allow me to go on a tangent and quickly check dumpsys SurfaceFlinger
:
---------------------------------------------------------------------------------------------------------------------------------------------------------------
rust.example.winit_android/android.app.NativeActivity#39518
rel 0 | 1 | 0 | DEVICE | 0 | 0 0 1096 2560 | 0.0 0.0 1096.0 2560.0 | [*]
...
HWC2 display_id: 0
layer: 11369 name: VRI[NativeActivity]#0(BLAST Consumer)0 z: 0 composition: Device/Device alpha: 255 format: RGBX_8888 dataspace:0x08810000 transform: 0/0/0 buffer_id: 0xb400007871ca9ab0 secure: 0
LayerBuffer: width=[1152], height=[2560], unaligned_width=[1096], unaligned_height=[2560]
LayerBufferFlags: secure=[0], interlace=[0], hdr=[0]
ColorMetaData: colorPrimaries=[1], range=[1], transfer=[1], matrixCoefficients=[0](matrixCoefficients is not used for SDR layer)
...
|-----|---------------|-----------|------|-------------|--------------------------|---------------------|---------------------|----|------------|-----------|----|-----|----|
| Idx | Comp Type | Split | Pipe | W x H | Format | Src Rect (L T R B) | Dst Rect (L T R B) | Z | Pipe Flags | Deci(HxV) | CS | Rng | Tr |
|-----|---------------|-----------|------|-------------|--------------------------|---------------------|---------------------|----|------------|-----------|----|-----|----|
| 0 | SDE | Pipe-1 | 139 | 1152 x 2560 | RGBX_8888 | 0 0 548 2560 | 0 0 548 2560 | 0 | 0x00002001 | 0 x 0 | 1 | 1 | 1 |
| | | Pipe-2 | 136 | 1152 x 2560 | RGBX_8888 | 548 0 1096 2560 | 548 0 1096 2560 | 0 | 0x00002001 | 0 x 0 | 1 | 1 | 1 |
If you ignore the fact that my phone is a dualpipe (two layer mixers and two encoders, each getting half of the plane, to drive a 4k@120Hz screen), you can clearly see that the "composition type" of this RGBX_8888
buffer of 1152x2560
pixels is "Snapdragon Display Engine" instead of some kind of software (or GPU) composition.
Completely separate from all this there's a more low-level API to dispatch HardwareBuffer
s (that we allocate ourselves!) to a surface 1. Here we could implement buffer aging correctly.
Footnotes
-
Lots of caveats apply here:
- The API is still on a branch for the
ndk
crate, to be finished; - NDK functions to retrieve a
SurfaceControl
for the rootNativeWindow
are broken right now and might only be fixed in Android 15 onwards; - Requires API level 29.
- The API is still on a branch for the
1ed5a10
to
2e30674
Compare
cf2d262
to
fd7f27e
Compare
After originally seeing complaints on `nightly`, we now see them on `stable` (but no longer on `nightly`): either some regression slipped in, or `rustc` understands that any relative path references inside content from `include_str!()` are supposed to be relative to the file that is being included, not the file that it is being included in (i.e. in this case relative to `./README.md`, not to `src/lib.rs` which includes `../README.md`).
On Android the backing buffer (`NativeWindow`) disappears when the application is not focussed and/or the screen is locked. Winit handles this by requiring apps to create their `raw_window_handle()` consumers _after_ `Event::Resumed` and to clean it up _before_ returning from `Event::Suspended`. For consistency Winit also sends `Resumed` on all other platforms during init.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for now, although we really should do something about pixel formats
Fixes #44
Contains my hacks to further discussing Android support. There are two issues:
stride() != width()
, requiring us to deal with that (andIOSurface
on Mac seems to have the same?).These could either be solved by exposing this information to the user (more work and documentation for them, but also more performant) or kept hidden by maintaining the current API, but requiring an extra framebuffer allocation that we convert when they submit the frame.