-
Notifications
You must be signed in to change notification settings - Fork 36
Description
It is presently impossible to render a PNG with transparency. For example, shading a button with an image to show that it's disabled.
There is a bug in the image resizing function.
rmkit/src/rmkit/util/image.cpy
Lines 17 to 21 in fc372b2
| for (int i=0; i < resize_len; i++): | |
| rgba_buf[j++] = resize_buffer[i] | |
| rgba_buf[j++] = resize_buffer[i] | |
| rgba_buf[j++] = resize_buffer[i] | |
| rgba_buf[j++] = resize_buffer[i] |
All four bytes are set to the grayscale value. This means that if you enable alpha when drawing the bitmap, fully black pixels will be fully transparent (and are skipped)
Lines 355 to 357 in fc372b2
| // 4th bit is alpha -- if it's 0, skip drawing | |
| if ((char*)src)[i*image.channels+3] != 0: | |
| self._set_pixel(&ptr[i], i, j, pack_pixel((char *) src, i*image.channels)) |
This function should probably set the 4th byte to 0xFF instead of the color value.
Confoundingly, Pixmap::alpha is also broken. The alpha value is a remarkable_color(uint16), which makes no sense when passed in as the pseudo alpha. In draw_bitmap, we compare the full uint32 pixel to a uint16, which means it's impossible to set the pseudo alpha to white (0xFFFFFFFF). Additionally the default value of 97 doesn't make sense to me. I would expect it to be WHITE if not disabled by default.
Line 87 in fc372b2
| remarkable_color alpha = 97 |
If I fix both Pixmap::alpha and util::resize_image I can get the above result by setting the alpha to WHITE. However, this is obviously not true alpha, if you zoom in all the way you can see some pixels around the edge that are lighter than the background.
Forcibly converting all images to a 1-channel grayscale and then back up to a 4-channel RGBA doesn't make a ton of sense. If you want to convert all images down to grayscale before resizing, that's fine, but why bother with converting it to RGBA? render_bitmap has logic for 1-channel bitmaps.
One more thing:
Lines 313 to 317 in fc372b2
| inline void grayscale_to_rgb32(uint8_t src, char *dst): | |
| uint32_t color = (src * 0x00010101); | |
| dst[0] = color & 0x00FF | |
| dst[1] = color & 0x0000FF | |
| dst[2] = color & 0x000000FF |
While this technically still produces the correct result (and likely gets optimized away), the code is not doing what you appear to have intended.
I could make a PR just fixing pseudo alpha on Pixmaps, but that feels like a hack on top of some structural issues in bitmap rendering. I think that CachedIcons and Pixmap rendering need to be rethought. Alpha blending does not appear to be supported at all in the render process, so why are we forcing all icons into RGBA? There are two separate pseudo alpha calculations happening here, which gives you a situation where both full white and full black are transparent.
My recommendation is to rework cached icons to either explicitly convert to grayscale or RGB, and ensuring bitmap rendering works for 1 and 3 channel bitmaps. Since alpha is not actually supported, we just shouldn't support RGBA bitmaps at all. In that case, a simple pseudo alpha is fine.
While it would be extremely nice to properly support alpha blending, I think that might be a much bigger change.

