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 platform #44

Open
i509VCB opened this issue Dec 21, 2022 · 8 comments · May be fixed by #130
Open

Android platform #44

i509VCB opened this issue Dec 21, 2022 · 8 comments · May be fixed by #130
Labels
Android NDK enhancement New feature or request help wanted Extra attention is needed

Comments

@i509VCB
Copy link
Contributor

i509VCB commented Dec 21, 2022

EDIT: A bit of discussion on this happened in rust-windowing/swbuf#8 (comment)

@john01dav john01dav transferred this issue from rust-windowing/swbuf Dec 22, 2022
@john01dav john01dav added enhancement New feature or request help wanted Extra attention is needed Android NDK labels Dec 23, 2022
@MarijnS95
Copy link
Member

The most likely way to implement this would be via ANativeWindow_lock(), whose API has been wrapped for the ndk crate at:

https://github.com/MarijnS95/ndk/compare/window-lock

We can merge this into the ndk crate after some slight cleanup.

@ids1024
Copy link
Member

ids1024 commented Apr 11, 2023

Using ANativeWindow_lock, is there a way to request that the window uses an BGRX buffer like SoftBuffer currently assumes? Or would we have to deal with a different pixel format? (And if that could vary, things become even more complicated...)

@MarijnS95
Copy link
Member

@ids1024 There is a way to change the format, see how the example has a (commented out) way to do this:

rust-mobile/ndk@master...MarijnS95:ndk:window-lock#diff-f302a34380ff35caef13c0c344aede7c4c0bf4ae4b957e066ad0bc73eb4cc418R10

Unfortunately there's no support for BGRX, only RGBX:

https://docs.rs/ndk/latest/ndk/hardware_buffer_format/enum.HardwareBufferFormat.html
https://developer.android.com/reference/android/hardware/HardwareBuffer

@ids1024
Copy link
Member

ids1024 commented Apr 11, 2023

Right. Hopefully we can at least expect RGBX to always work?

In comparison Wayland's wl_shm has a huge list of formats but only requires a compositor to support argb8888 and xrgb8888 (which a little endian, so the same as formats called BGRA/BGRX elsewhere).

It seems we can't expect to use the same pixel format everywhere while also getting performant no-copy presentation.

@MarijnS95
Copy link
Member

First draft pushed at https://github.com/MarijnS95/softbuffer/compare/android, untested but at least letting you folks know that this is being worked on :)

@MarijnS95
Copy link
Member

MarijnS95 commented Jun 16, 2023

Updated the branch, it is fully working now (but will need some cleanup before it is ready for a PR, though let me know if I should open it as draft already)!

A couple important notes:

  • Android only has R8G8B8A8_UNORM and R8G8B8X8_UNORM as 32-bits-per-pixel format;
  • Android (and/or the hardware) really values alignment, so consecutive scanlines of pixels are not contiguous: instead there's a stride parameter in pixel count;
    • For now I've exposed this value on Buffer::stride(), though we could also have pixel setters with x, y coords or similar.
  • None of the winit-based examples follow proper Resumed semantics: examples/winit: Implement proper Event::Resumed semantics #127;
  • Separately PR the README reformat too?
  • The example cannot compile as an executable and cdylib for Android concurrently. I should perhaps make a separate winit_android.rs for that using a shared .rs file with the business logic, or thanks to most of the CFG's in the code: an [[example]] path = "" to compile the same file twice;
  • ndk/native_window: Add lock() to blit raw pixel data rust-mobile/ndk#404 needs a review, merge and release as a preliminary: softbuffer is a neat little test-bed.

@ids1024
Copy link
Member

ids1024 commented Jun 26, 2023

Currently, I think #98 is the most important missing feature/API in softbuffer. It's awkward if some platforms only support RGBX/RGBA but others only support BGRA/BGRX. We'll want an API that helps with conversion, but for maximum performance an application would have to support both...

though let me know if I should open it as draft already

I generally think it's good to have a draft PR. That makes it a bit more visible that some work has been done, and easy to comment on. Relative to just linking a branch on the issue.

Exposing a Buffer::stride hopefully doesn't make the API too confusing. It isn't really any harder to deal with, it's just something users of the library have to deal with, but may be unfamiliar with if they haven't seen how this works at a low level before.

@MarijnS95 MarijnS95 linked a pull request Jun 26, 2023 that will close this issue
@MarijnS95
Copy link
Member

@ids1024 done in #130.

Depends on whether you want to go for performance, convenient/consistent/simple API across all platforms, or both.

In the NDK API I've added a helper function that returns an iterator yielding one mutable scanline/slice at a time without padding:

https://github.com/MarijnS95/ndk/blob/b520751834fc7d5a6c9759fa9c9ea8ce3d0f3213/ndk/src/native_window.rs#L214-L230
And when put to use: rust-mobile/rust-android-examples#7

Maybe this could help users? Or x/y getters/setters? The possibilities are (unfortunately) endless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Android NDK enhancement New feature or request help wanted Extra attention is needed
Development

Successfully merging a pull request may close this issue.

4 participants