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

WIP: Attempt to use IOSurface on macOS #95

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Apr 10, 2023

It looks like BytesPerRow (the stride) has some kind of alignment requirement (testing on M1), that doesn't let us just use the width. So for this to work, softbuffer needs to expose the stride, and the user needs to deal with that...

This doesn't seem to be showing the right thing even when width matches stride. Not sure why.

It looks like `BytesPerRow` (the stride) has some kind of alignment requirement (testing on M1), that doesn't let us just use the width. So for this to work, softbuffer needs to expose the stride, and the user needs to deal with that...

This doesn't seem to be showing the right thing even when width matches stride. Not sure why.
@ids1024
Copy link
Member Author

ids1024 commented Apr 10, 2023

This doesn't seem to be showing the right thing even when width matches stride. Not sure why.

Ah, so this part is due to the alpha channel. https://developer.apple.com/documentation/corevideo/cvpixelformatdescription/1563591-pixel_format_identifiers doesn't list an BGRX format or similar, so it looks like we need to set the alpha of each pixel to 255 for it to display properly.

@ids1024
Copy link
Member Author

ids1024 commented Apr 11, 2023

We'll probably need to expose a stride to efficiently implement #44 using https://developer.android.com/ndk/reference/struct/a-native-window-buffer#struct_a_native_window___buffer

So I think we'll ultimately want an API change like this.

let blue = (x * y) % 255;

buffer[index as usize] = blue | (green << 8) | (red << 16);
let stride = buffer.stride();
Copy link
Member

Choose a reason for hiding this comment

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

Interesting, Android needs the same!

@LoganDark
Copy link

We'll probably need to expose a stride to efficiently implement #44 using https://developer.android.com/ndk/reference/struct/a-native-window-buffer#struct_a_native_window___buffer

So I think we'll ultimately want an API change like this.

I would highly recommend looking into imgref for this.

@ids1024
Copy link
Member Author

ids1024 commented Aug 29, 2023

Is imgref currently used in any crates that would likely be used with Softbuffer for rendering?

We need the Buffer type to provide a present method, and on some backends to release certain resources on drop. So it's natural enough to provide a stride() method on that. But we could add a method returning ImgRefMut if that will be useful to users of softbuffer.

@MarijnS95
Copy link
Member

Should we move this discussion over to #98 which is now open to discuss the API considering requirements of various platforms, not just limiting it to macOS here?

FWIW such a crate could be provided behind a [feature] gate if desired. I've open-coded a similar thing when adding the necessary bindings to the NDK to facilitate support in softbuffer (#130): https://docs.rs/ndk/0.8.0-beta.0/ndk/native_window/struct.NativeWindowBufferLockGuard.html#method.lines

@LoganDark
Copy link

Is imgref currently used in any crates that would likely be used with Softbuffer for rendering?

We need the Buffer type to provide a present method, and on some backends to release certain resources on drop. So it's natural enough to provide a stride() method on that. But we could add a method returning ImgRefMut if that will be useful to users of softbuffer.

The Buffer should deref to ImgRefMut instead of &[u32]. That's what I mean. Then the ImgRefMut itself would know its own stride.

Bonus if you do ImgRefMut<ARGB> instead of ImgRefMut<u32> but either way is fine.

@ids1024
Copy link
Member Author

ids1024 commented Aug 30, 2023

Bonus if you do ImgRefMut instead of ImgRefMut but either way is fine.

Yeah, that part gets back to #98. Unfortunately it seems we need to use RGBA on some platforms for no-copy presentation....

So I think we've settled on this solution for macOS, it's just that it requires breaking API changes, and we may want to do that along with #98.

@LoganDark
Copy link

LoganDark commented Aug 30, 2023

Unfortunately it seems we need to use RGBA on some platforms for no-copy presentation

Well right now you say u32. Using the dedicated structs from rgb crate would be better and it's even what I use internally before converting to u32. So in a way you just proven my point

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

Successfully merging this pull request may close these issues.

3 participants