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

Android backend #130

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Android backend #130

wants to merge 8 commits into from

Conversation

MarijnS95
Copy link
Member

@MarijnS95 MarijnS95 commented Jun 26, 2023

Fixes #44

Contains my hacks to further discussing Android support. There are two issues:

  • Format is RGBA or RGBX;
  • Buffers almost always have a stride() != width(), requiring us to deal with that (and IOSurface 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.

@ids1024
Copy link
Member

ids1024 commented Jun 27, 2023

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.

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.

@ids1024
Copy link
Member

ids1024 commented Jun 27, 2023

Adding a stride() is unnecessary for now, if it converts on present anyway. But we'll need that for this for IOSurface, or anything else where we write directly into memory in a format the GPU may handle with a stride requirement.

So that can be added to the API when #98 is implemented.

examples/winit.rs Outdated Show resolved Hide resolved
@MarijnS95
Copy link
Member Author

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.

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.

No fuss/hurry to get this merged before fleshing out the details.

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.

Sounds like a plan.

Adding a stride() is unnecessary for now, if it converts on present anyway. But we'll need that for this for IOSurface, or anything else where we write directly into memory in a format the GPU may handle with a stride requirement.

It's necessary unless we code in the conversion on present. And Android might again do a conversion after that.

@MarijnS95 MarijnS95 force-pushed the android branch 2 times, most recently from 3e41353 to 2c40bc2 Compare November 9, 2023 00:06
@MarijnS95 MarijnS95 force-pushed the android branch 3 times, most recently from 0a7f720 to d5cc95a Compare June 25, 2024 19:05
Copy link
Member Author

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.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure when that's used, seems a little odd.

Looks like that was added by @notgull in 0e6706c.

Copy link
Member

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

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 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)

Copy link
Member Author

Choose a reason for hiding this comment

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

You can definitely use modules 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.

@MarijnS95 MarijnS95 marked this pull request as ready for review June 25, 2024 19:08
@MarijnS95 MarijnS95 requested a review from john01dav as a code owner June 25, 2024 19:08
}

pub fn present_with_damage(self, _damage: &[Rect]) -> Result<(), SoftBufferError> {
Err(SoftBufferError::Unimplemented)
Copy link
Member

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.

}

pub fn age(&self) -> u8 {
todo!()
Copy link
Member

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.

Copy link
Member Author

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 HardwareBuffers (that we allocate ourselves!) to a surface 1. Here we could implement buffer aging correctly.

Footnotes

  1. Lots of caveats apply here:

    1. The API is still on a branch for the ndk crate, to be finished;
    2. NDK functions to retrieve a SurfaceControl for the root NativeWindow are broken right now and might only be fixed in Android 15 onwards;
    3. Requires API level 29.

@MarijnS95 MarijnS95 force-pushed the android branch 4 times, most recently from 1ed5a10 to 2e30674 Compare August 3, 2024 10:53
@MarijnS95 MarijnS95 force-pushed the android branch 2 times, most recently from cf2d262 to fd7f27e Compare November 29, 2024 21:40
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.
Copy link
Member

@notgull notgull left a 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Android platform
4 participants