-
-
Notifications
You must be signed in to change notification settings - Fork 10
rudimentary support for BMPs with alpha masks #43
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
base: main
Are you sure you want to change the base?
Conversation
"rudimentary" in the sense that the format will be allowed and parsed, but the alpha channel is thrown away when drawing and getting pixels.
I wouldn't mind adding this hack, as long as it is documented how ARGB images are handled. I couldn't find information about whether BMP uses premultiplied alpha or not. If it doesn't, drawing a not entirely opaque image would lead to an ugly output. In this case it would be better to apply some simple alpha blending with black to fix the semi transparent pixels:
It was planned to have some alpha blending inside of e-g. But alpha blending would have only ever worked for a subset of |
Cool, thanks for the thoughtful (and speedy!) feedback :)
Yeah, the output I'm seeing isn't gonna win any beauty contests, that's for sure :) For my particular use case, I'm just barfing out the splash BMPs that are embedded in the UKIs for a phone bootloader I'm hacking on. In this particular situation, I have a 720x1280 framebuffer to play with, and the image (the Arch splash logo that ships with a core OS package) is only a small portion of the screen, so it doesn't actually look too awful.
I thought about this, and decided not to explore until I took a temperature test from upstream. Since you seem receptive, let's talk some more ... I was thinking that it could be useful to provide the "background" color that the image should blend with. That is, you'd call something like Further, any pixels that have 0 alpha would actually get skipped. So this |
That sounds great, I had thought about suggesting something like this, but had decided against it to prevent feature creep beyond your use case. But being able to set the background color is a much better solution and I support the idea of implementing it this way.
I don't think this is a good idea, because it would treat pixels with alpha 0 and 1 very differently (0 would draw nothing and 1 would draw a pixel in glorious hot pink). In some cases this can also have worse performance, because the image can no longer be drawn via |
👍 I've updated the PR with the blending. Let me know what you think. I must say, it's looking fabulous: Let me know how I should document this. Also, I'm not sure if it's possible to expose the alpha channel via |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know how I should document this.
My suggestion is to add a new "Supported formats" or "Limitations" section to the lib.rs
docs, above the examples. This would also be a good place to later add the description of the limitations in the recently added RLE support.
Also, I'm not sure if it's possible to expose the alpha channel via Pixels? Since e-g doesn't have core color types with alpha. I didn't think very hard about it, though.
It is possible by implementing our own custom Argb8888
type in tinybmp
, but this type should ideally be included in e-g to make it usable in different crates. I'm inclined to not implement this at the moment and wait until an e-g type becomes available (unlikely) or if someone requests this feature.
/// If this image contains transparent pixels (pixels with an alpha channel), then blend these | ||
/// pixels with the provided color. Note that this will only be used when drawing to a target. | ||
/// It will not be applied when querying pixels from the image. | ||
pub fn with_alpha_bg<BG: Into<Rgb888>>(mut self, alpha_bg: BG) -> Self { | ||
self.alpha_bg = alpha_bg.into(); | ||
self | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be a regular setter (set_alpha_bg<...>(&mut self, alpha_bg: BG)
) instead of this builder style method. Using a mutable reference instead of having to transfer ownership makes it more flexible.
An even better solution would be to add a wrapper type around the Bmp
type, that sets the background color, but this would be a lot more work:
pub fn with_alpha_bg<BG: Into<Rgb888>>(&self, alpha_bg: BG) -> BmpWithAlphaBg<'_, C> {
BmpWithAlphaBg {
bmp: self,
alpha_bg: alpha_bg.into(),
}
}
This would allow the Bmp
object to be drawn with any background color without needing a mutable reference or even ownership of the object. You could then easily implement something like:
fn draw_button<D: DrawTarget>(target: &mut D, text: &str, icon: &Bmp<'_, D::Color>, background: D::Color) -> Result<(), D::Error> {
....
}
This would still kind of work with a mutable reference to the icon, but wouldn't work with the original transfer of ownership in the with_alpha_bg
method.
alpha += 1; | ||
if alpha == 0 { | ||
// pixel is completely transparent, use bg color | ||
self.alpha_bg | ||
} else if alpha == 255 { | ||
// pixel is completely opaque, just use its color | ||
Rgb888::from(RawU24::new(v >> 8)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't have time to thoroughly check the blending code, but the checking for alpha == 0
and alpha == 255
must happen before 1
is added.
"rudimentary" in the sense that the format will be allowed and parsed, but the alpha channel is thrown away when drawing and getting pixels.
No hard feelings if you don't want to accept this patch. It's kinda silly. The better solution is to not use BMPs with alpha masks, of course. It's just that I found myself in a situation where I want to render some BMPs that I don't necessarily have control over. I figured this hacky solution is better than nothing since it's unlikely embedded-graphics will ever get alpha blending support.