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

Pixel format API design #98

Open
ids1024 opened this issue Apr 11, 2023 · 17 comments
Open

Pixel format API design #98

ids1024 opened this issue Apr 11, 2023 · 17 comments

Comments

@ids1024
Copy link
Member

ids1024 commented Apr 11, 2023

We need an API to list available pixel formats, and select one. Some considerations:

  • We want a way to support transparency: Transparency #17
  • The web backend current converts BGRX to RGBA. It would be better if it could use RGBA directly, if an application can render that way.
  • It looks like for Android platform #44 we'd have to use RGBX, rather than BGRX.
  • For WIP: Attempt to use IOSurface on macOS #95, it seems BGRA is supported, but not BGRX. So we need to make sure to set alpha to 255.
  • Wayland's wl_shm can support many formats, but only two are required to be supported by compositors.

So some questions here:

  • What formats should be supported?
    • We want to offer a format for each platform that can be used efficiently without conversion, at least, and there doesn't seem to be one that can be assumed everywhere, unfortunately.
    • There are many formats that could be supported on at least some platforms.
  • Do we want to support formats that are not 4 byte per pixel? This will impact the API, which currently uses u32 values for pixels.
  • How to we communicate what formats are supported, and what formats are optimal?
  • We still want at least one format to work on all platforms, even if it requires conversion on some, right? What formats should be universally supported, then?
@notgull
Copy link
Member

notgull commented Apr 11, 2023

Here is an API I wrote a while back for an image crate I was writing (abandoned as of now). The idea is that is specifies RGBA channels, their sizes, their order and whether or not they are present. It's based on the setup that pixman uses. I think a similar type would be good for softbuffer.

What if we made it so, no matter what format the buffer is, it's transcoded to the proper format? However, the API would expose a "transcode-free formats" function (maybe an Iterator?) that indicates which formats aren't transcoded at all?

@ids1024
Copy link
Member Author

ids1024 commented Apr 11, 2023

Technically that doesn't cover every possible format. Wayland for instance supports things like YUV, indexed color, etc. But I doubt YUV is useful for softbuffer consumers, and indexed color (hopefully) isn't relevant these days, nor is it useful without knowing the palette. 😆

I guess that iterator would be returned by a method of Surface? And there would be a set_format method to set the format for the next buffer_mut? Unlike resize this call would be optional, and default to the current default.

But still, we have the fact Buffer derefs to &[u32], which is a problem if we want to support formats with more or less than 32 bits per pixel.

@ids1024
Copy link
Member Author

ids1024 commented Apr 11, 2023

The most obvious way to specify formats would be to provide an enum with every format supported by at least one backend. Regardless, backends likely use enums, and need to be mapped that way.

Of course we also need to handle conversion some way. Which is simple for variations on RGBA of the same bit depth but in different orders.

Transparency support is somewhat special. We can't really convert RGBA to RGBX and have that to work as expected (but can do the reverse). The application needs to know if transparency is supported. Also a particular backend might only support RGBA here, but have a hint to indicate that the surface is opaque. This seems to be the case with IOSurface an CALayer's isOpaque property (no idea if that will have any impact when using an IOSurface).

@daxpedda
Copy link
Member

Transparency support is somewhat special. We can't really convert RGBA to RGBX and have that to work as expected (but can do the reverse). The application needs to know if transparency is supported. Also a particular backend might only support RGBA here, but have a hint to indicate that the surface is opaque. This seems to be the case with IOSurface an CALayer's isOpaque property (no idea if that will have any impact when using an IOSurface).

I agree with the enum idea. Couldn't we encode alpha support into the texture format enum? So BGRX and BGRA could just be two different formats.

@ids1024
Copy link
Member Author

ids1024 commented Apr 13, 2023

The complication there is format conversion. And whether or not transparency is supported at all. BGRX can be converted to BGRA by setting the last component of each pixel to 255. But converting BGRA to BGRX isn't possible without losing the alpha channel, which the caller probably doesn't want. So do we not allow that conversion, and require an application to check for a format with alpha before trying to use transparency?

@daxpedda
Copy link
Member

The complication there is format conversion.

I'm in favor of doing no conversion at all. We take what the user supplies and that's it. The user has to supply it correctly.

So do we not allow that conversion, and require an application to check for a format with alpha before trying to use transparency?

One way I imagine we could "require to check the format" could be to make the format part of the buffer, basically:

enum Buffer<'a> {
	Bgrx(BgrxBuffer<'a>),
	Bgra(BgraBuffer<'a>),
	Rgba(RgbaBuffer<'a>),
}

I'm don't think I'm in favor of this API, because the API of each of these types would be exactly the same. Having a format function that returns the an enum with the format would make more sense to me.

In any case, I'm in favor of not doing any conversions at all.

@ids1024
Copy link
Member Author

ids1024 commented Apr 13, 2023

Not doing conversions makes things simpler, definitely. The trouble there is if there is no format that is supported on all backends. If we don't do conversion, does that mean the web backend would only advertise RGBA, and an application could only work on that backend if it can render to that (which may or may not be practical), or does the conversion itself?

So this compromises the goal of making softbuffer an easy way to put pixels on the screen across platforms. And we'd want to document the minimum set of formats a portable application needs to handle.

We could also provide helpers or a wrapper to do format conversion. At least we'd need to make the examples demonstrate what is needed for a portable application.

@daxpedda
Copy link
Member

daxpedda commented Apr 13, 2023

If we don't do conversion, does that mean the web backend would only advertise RGBA, and an application [..] does the conversion itself?

Yes, that is what I would be in favor of.

So this compromises the goal of making softbuffer an easy way to put pixels on the screen across platforms. And we'd want to document the minimum set of formats a portable application needs to handle.

We could also provide helpers or a wrapper to do format conversion. At least we'd need to make the examples demonstrate what is needed for a portable application.

Maybe I'm thinking about this wrong. The goal is to create a single portable API. But the reason we are here discussing different formats, is because not all backends work the same and there are differences that we can't bridge without introducing overhead. That is the reason why I want to have conversions not happen in softbuffer, but by the user, to enable a way to not pay the overhead in return for a non-portable API.

So my thinking now was instead of introducing helpers, maybe it would make more sense indeed to do the conversion in softbuffer, but introduce extension traits to circumvent that conversion by having to supply specific color formats.

Alternatively, we could also introduce from_rgba(), from_bgrx(), from_bgra() and have a function returning the format which does no overhead.


To summarize my braindump here:

  • (non-portable) Return an enum Buffer type with a type for each format (Pixel format API design #98 (comment)).
  • (non-portable) Provide a format() function telling the user which format needs to be supplied to the buffer, doing no conversions on the softbuffer-side. Optionally provide helper functions to do the conversion on the user side.
  • (portable) Provide from_rgba(), from_bgrx(), from_bgra() methods, necessary conversions are done on the softbuffer side. If the user wants to circumvent the conversion overhead, a format() function can inform the user which format is native.

All APIs suggested here would be available on all platforms, non-portable just means the user has to handle multiple ways to draw the image to handle different formats for different platforms.

Currently I'm in favor of the last one.

@daxpedda
Copy link
Member

daxpedda commented Apr 13, 2023

#98 (comment)

What if we made it so, no matter what format the buffer is, it's transcoded to the proper format? However, the API would expose a "transcode-free formats" function (maybe an Iterator?) that indicates which formats aren't transcoded at all?

Basically is the conclusion I came to after a bunch of back and forth here (just an enum instead of an Iterator).

Which basically still leaves open the two problems posed by @ids1024.

  • How to handle non &[u32] formats.
  • How to handle transparency conversions.

I think the first problem can be addressed by providing conversion free methods. Alternatively the "multiple Buffer types" suggestion would address this too.

Transparency conversion is problematic, but we could apply the "pre-multiplied alpha" strategy, which is the best I can think of. Alternatively, again, could be addressed with the "multiple Buffer types" suggestion.

@ids1024
Copy link
Member Author

ids1024 commented Apr 13, 2023

Making Buffer an enum makes a certain amount of sense. But I think we want an API that lets the caller choose between multiple different formats out of those that are available on the platform, so it would just be a bit redundant. (If I'm setting the format, I'd match on the Buffer with a unreachable!() for types other than the one I selected.)

Having the core API always use a native format, and then providing some sort of helper for conversion, seems better now that I think of it. "Explicit is better than implicit", and done well that should still provide an easy to use API.

Though that still needs a good API. A from_rgba() method would probably still involve copying even if no conversion is needed, right? It would be good for it to still be no-copy where possible. I guess an API could wrap Buffer, also be derefable, and if conversion is needed, either allocate a vec! for the format converted from, or convert in place on present? (In-place conversion would clash with #90).

@ids1024
Copy link
Member Author

ids1024 commented Apr 13, 2023

If we're unsure about non-u32 formats, we can leave those out for now, and update the ABI for that later (possibly in a breaking way).

I'd definitely say the priorities are:

  • Being able to use a native format without conversion on all platforms, and without copying.
  • Support for transparent windows where possible.

I'd like to fix those and get them in a tagged release. Those seem like the main limitations of softbuffer currently. #39 and #90 could be added with non-breaking changes, as an enhancement.

For most purposes on current platforms, you don't need pixel formats smaller than 32-bit. And higher bit depth (for things like HDR) is still niche and probably not something that you'd use a software renderer for.

@daxpedda
Copy link
Member

Though that still needs a good API. A from_rgba() method would probably still involve copying even if no conversion is needed, right?

True, I didn't think this through.

I guess an API could wrap Buffer, also be derefable, and if conversion is needed, either allocate a vec! for the format converted from, or convert in place on present? (In-place conversion would clash with #90).

I'm not sure I properly understand how #90 plays a role here, could you elaborate? My first thought was to do in-place conversion on present.

I was also just thinking that reading pixels will be pretty weird as well, because it might not be the format users are using to write.


Maybe I was thinking about this wrongly, but it might be better to decide on the texture format on Surface creation, conversion would then happen in present if the format is not native and the buffer was changed. This would make reading and writing pixels use the same format as well. In addition, whatever API we will come up with to let the user use a desired format on the Buffer, it won't make a whole lot of sense, the most common use-case is to use a single texture format, native or not, it's unlikely anybody wants to switch formats between frames.

@ids1024
Copy link
Member Author

ids1024 commented Apr 17, 2023

I'm not sure I properly understand how #90 plays a role here, could you elaborate?

The goal of buffer age is to allow redrawing only part of the buffer, because the buffer matches what you presented age frames ago. If it were converted in place, the components would be mixed up, unless buffer_mut also converts the buffer back.

So I guess the format conversion wrapper could provide an age that passes through to the backend when conversion isn't needed, but always returns None/0 for converted formats if conversion is done in place, or returns 1 if a separate vec were used.

I was also just thinking that reading pixels will be pretty weird as well, because it might not be the format users are using to write.

Basically the same issue as buffer age. So if we want to support it, it would have to convert back. Which couldn't be done in-place for a front buffer that the display server is also accessing.

Maybe I was thinking about this wrongly, but it might be better to decide on the texture format on Surface creation

Can we assume the formats supported by the display are the same for each surface, in all backends? Then softbuffer::Context could have a method to get a list of supported formats. Otherwise we need a RawWindowHandle or a softbuffer::Surface to get the supported formats, which is important for an application that could handle multiple formats depending on what the backend supports.

@daxpedda
Copy link
Member

daxpedda commented Apr 17, 2023

Can we assume the formats supported by the display are the same for each surface, in all backends? Then softbuffer::Context could have a method to get a list of supported formats. Otherwise we need a RawWindowHandle or a softbuffer::Surface to get the supported formats, which is important for an application that could handle multiple formats depending on what the backend supports.

We have access to the Surface already when creating Context, right? So this shouldn't be a problem, but I don't know any of the details here. On Web there is only one supported format 😆.

But yeah, this seems like a viable solution to me.

@ids1024
Copy link
Member Author

ids1024 commented Apr 17, 2023

Creating a Context requires a RawDisplayHandle, while creating a Surface requires a RawWindowHandle.

So on Wayland the RawDisplayHandle represent a particular connection to a Wayland server, while the RawWindowHandle represents a particular window or surface.

With wl_shm the formats available aren't related to the window. If we wanted to use a dma_buf though the preferred formats can depend on the surface, and can change dynamically: https://wayland.app/protocols/linux-dmabuf-unstable-v1#zwp_linux_dmabuf_v1:request:get_surface_feedback

@daxpedda
Copy link
Member

daxpedda commented Apr 17, 2023

Creating a Context requires a RawDisplayHandle, while creating a Surface requires a RawWindowHandle.

Ah, I mixed it up, we have access to the Context when creating a Surface. So what you are saying is that on Wayland we need the RawWindowHandle to get the preferred format.

That shouldn't really stop us, but makes the API more awkward, to get the preferred format for a surface we need to have RawWindowHandle, which we then need to pass again when creating the Surface from the Context.

Maybe the solution here could be to create a SurfaceBuilder with RawWindowHandle, then decide on the format, which then creates the actual Surface.

[..], and can change dynamically: https://wayland.app/protocols/linux-dmabuf-unstable-v1#zwp_linux_dmabuf_v1:request:get_surface_feedback

Uhm, that sounds very weird. Maybe we just want to document that and not handle it otherwise? Or is this a common scenario?

@ids1024
Copy link
Member Author

ids1024 commented Apr 17, 2023

We're not currently using that API for the Wayland backend, we're currently using wl_shm where there are no differences between surfaces. This would be if we wanted to use a dmabuf referencing an on-GPU buffer instead. Which might perform better at least under certain circumstances (presumably it's better on a unified memory architecture, at least, saving a copy?).

So I don't know that this is an issue with any current platform, but it's at least a potential concern, that could limit changes to backends or future backends.

Yeah, a SurfaceBuilder might be a good idea. I was thinking of having a default format, and a way to change it, but if the conversion is done by a separate helper that wouldn't really work so well since the default would be different on different platforms.

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

No branches or pull requests

3 participants