Conversation
src/lib.rs
Outdated
| /// - macOS/iOS: Supported, but currently doesn't work with additive values (maybe only as the | ||
| /// root layer?). Make sure that components are `<= alpha` if you want to be cross-platform. |
There was a problem hiding this comment.
It looks like this currently:
I suspect that this may be an intentional limitation of transparency in the "root" / that goes through the top-level layer, perhaps as a form of security measure to avoid users reading contents of contents below the window in a shader or smth? But I'll need to investigate this further, might be possible to fix with IOSurface.
There was a problem hiding this comment.
I tested with #329, that fixes it. It's probably because CGImage/CGContext or whatever is postmultiplied internally (which prevents certain kinds of alpha compositing).
| /// The non-alpha channels are not expected to already be multiplied by the alpha channel; | ||
| /// instead, the compositor will multiply the non-alpha channels by the alpha channel during | ||
| /// compositing. | ||
| /// | ||
| /// Also known as "straight alpha". | ||
| /// | ||
| /// ## Platform Dependent Behavior | ||
| /// | ||
| /// - Web and macOS/iOS: Supported. | ||
| /// - Android, KMS/DRM, Orbital, Windows, X11: Not yet supported. | ||
| #[doc(alias = "Straight")] | ||
| #[doc(alias = "Unassociated")] | ||
| Postmultiplied, |
There was a problem hiding this comment.
What should we call this? Wikipedia uses "straight", wgpu uses PostMultiplied.
There was a problem hiding this comment.
PostMultiplied seems a bit clearer, and if in doubt I'm inclined to make softbuffer APIs match wgpu.
There was a problem hiding this comment.
One problem with going with the wgpu choice here is that PostMultiplied is not in the WebGPU spec anywhere (and the spec actually uses the term "unpremultiplied alpha").
I traced the value back to gfx-rs/gfx#2509, which seems based on https://docs.vulkan.org/refpages/latest/refpages/source/VkCompositeAlphaFlagBitsKHR.html, so I guess we'd be basing it on the Vulkan spec.
There was a problem hiding this comment.
Actually, that link has an AlphaMode::Inherit, I guess we could use that approach instead of the AlphaMode::Opaque hack?
So instead of:
AlphaMode::Opaque= "require alpha channel 1.0"AlphaMode::Ignored= "alpha channel ignored".
We could do:
AlphaMode::Opaque= "alpha channel ignored" (to match WGPU'sCompositeAlphaMode::Opaque)AlphaMode::Inherit= "whatever the platform wants to default to, which usually means you should set the alpha channel to 1.0 to be cross-platform".
The difference between the first AlphaMode::Opaque and the second AlphaMode::Inherit would be that we probably wouldn't need to debug-assert that the alpha mode is as we expect in fn present if we use AlphaMode::Inherit.
There was a problem hiding this comment.
Inherit doesn't seem like the right term for that. That Vulkan spec says "the application is responsible for setting the composite alpha blending mode using native window system commands". But that wouldn't be true in softbuffer, at least not on all platforms. So there's nothing to inherit the alpha mode from.
The difference between the first AlphaMode::Opaque and the second AlphaMode::Inherit would be that we probably wouldn't need to debug-assert that the alpha mode is as we expect in fn present if we use AlphaMode::Inherit.
On the other hand, the caller also doesn't benefit from that debug assert, which helps ensure they're doing things in a correct cross-platform way.
AlphaMode::Opaque is a bit of a hack, but it seems handy enough for someone wanting to put some pixels on the screen and be sure it will work across platforms.
There was a problem hiding this comment.
https://docs.rs/wgpu/28.0.0/wgpu/enum.CompositeAlphaMode.html
I guess this idea is more like wgpu's CompositeAlphaMode::Auto, which is either Inherit or Opaque depending on the platform.
There was a problem hiding this comment.
Perhaps. Do you think we should use the same naming scheme then? I'm leaning towards no because "auto" doesn't really imply opaqueness. Maybe DefaultOpaque? Or AutoOpaque?
Hmm.
Thinking about things again, I wrote the following comment explaining why we even have AlphaMode::Opaque vs. folding it under AlphaMode::default():
We use a separate enum value here instead of just making a platform-specific default, because even though we'll want to effectively use a
*multipliedinternally on some platforms, we'll still want to hint to the compositor that the surface is opaque.
But maybe that's wrong?
The platforms we'll want to hint on is macOS/iOS (with CALayer.setOpaque(true)) and Web (with { alpha: false } in canvas.getContext), but for macOS/iOS, it should probably be Winit's responsibility to set in Window::set_transparent, and I've seen claims that setting the alpha mode doesn't actually do much on Web (but I don't have a source for that).
If that is the case, then the only value that AlphaMode::Opaque has vs. a platform-specific AlphaMode::default() would be the debug assertion. Is that enough of an argument for having it?
There was a problem hiding this comment.
If so, once we do #317, would it make sense to similarly have a PixelFormat::PlatformDefault variant to allow issuing debug assertions when using Buffer::pixel* methods with other variants?
| /// root layer?). Make sure that components are `<= alpha` if you want to be cross-platform. | ||
| /// - Android, Orbital, Web, Windows and X11: Not yet supported. | ||
| #[doc(alias = "Associated")] | ||
| Premultiplied, |
There was a problem hiding this comment.
I elected to call this Premultiplied instead of PreMultiplied (not camel casing), because most of the references I could find on the interwebs seemed to prefer to write it in one word instead of hyphenating as "pre-multiplied".
b636a35 to
21e4a90
Compare
4dd27ed to
304f431
Compare
9677e98 to
e9ce02e
Compare
dc30b3c to
fe20944
Compare
fe20944 to
693551c
Compare
| // TODO: Set opaque-ness on root layer too? Is that our responsibility, or Winit's? | ||
| // self.root_layer.setOpaque(opaque); |
There was a problem hiding this comment.
That would be https://docs.rs/winit/0.30.12/winit/window/struct.WindowAttributes.html#method.with_transparent, right?
So something that's left to winit.
There was a problem hiding this comment.
Or looking at this differently: if Vulkan, OpenGL, or wgpu won't set this property, then it makes sense that softbuffer won't either.
There was a problem hiding this comment.
I think wgpu calls setOpaque on the layer they're working with, which might be the root layer if it was created from CAMetalLayer, and otherwise it is a sublayer.
In any case, Winit doesn't currently set this property on its content view/layer, only on the window itself.
|
Other than some very minor details, this seems pretty good. Though I have less ability to comment on backends other than Wayland. It seems like a good way to handle alpha and a step towards more complete support for handling different formats. |
693551c to
61ec6fc
Compare
61ec6fc to
2263a76
Compare
Add:
This fixes #17 and prepares for #98 / #317.
As noted in the transparency issue, it is important to make a distinction between straight and premultiplied alpha. The former can be easier to work with, but the latter is often what's actually supported by compositors.
One mode that is a bit odd here is
Opaque, but it's necessary for the Web backend, because that platform doesn't support zero-copy RGBX, the alpha channel is always read (at least from what I could figure out). Adding this mode (and thus requiring that alpha channel to be 255) fixes #207.The implementation of these modes for each platform is as follows (I have tested all these):
OpaqueandPremultipliedwithIOSurface: UseIOSurfaceon macOS/iOS #329.Opaque,Ignored,Premultiplied.OpaqueandPostmultiplied.Premultipliedcan be supported in the future withImageBitmapIIUC.Premultiplied, but I couldn't get it to work properly, so only did part of it, I'm not too familiar with Windows. I suspect it might also need something like fixed window transparency winit#2503.Premultipliedcould be supported.We could fairly easily implement a conversion step to support postmultiplied alpha when premultiplied is supported by the backend, but I'd like to migrate towards a more efficient design where each backend always do zero-copying, so I haven't done that.
Expected behaviour
I've created an example
transparency.rs, which renders a few different shades of orange and yellow. The expected result are as follows:Opaque/IgnoredPremultipliedPostmultiplied