embedded-graphics: Images don't work correctly between color formats?

Version of embedded-graphics in use (if applicable): 0.6.2
Target: QEMU 5.0.0
If using real hardware: Custom

Description of the problem/feature request/other

Okay, so first some background:

I’m using this crate for graphics in QEMU OVMF UEFI, which only knows the pixel format at runtime, so I’ve implemented DrawTarget generically over Into<Bgr888> and just use Rgb888::from if need be.(blt and pixel bitmask currently unsupported). So far so good, the circle and some text example colors are correct, despite the code using Rgb565.

And onto the issue:

Then I wanted to display images, and found that Image<Tga, Rgb888> results in the colors being wrong. It compiles, it should be converting through Into<Bgr888>, yet the colors are wrong?

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 25 (13 by maintainers)

Most upvoted comments

But to_ne_bytes does nothing except directly transmute to an array.(through a union for const) It does not change the order of anything?

Rgb888::new(0x11, 0x22, 0x33).into_storage() results in an u32 with a value of 0x00112233. When you convert that value into an array by using to_ne_bytes on an x86_64 machine you will get: [0x33, 0x22, 0x11, 0x00].

Changing it to to_be_bytes, which I guess would “un-reverse” it? is substantially worse. And to_le_bytes would of course be a noop on x86

The resulting array in this case would be [0x00, 0x11, 0x22, 0x33], so you will need to remove the first element and not the last element of the array when copying it to the framebuffer.

rfuest on Aug 12, 2020

Maybe the color types should provide a way to get an array in the appropriate format so they don’t depend on endianness like this?

That’s a good idea. I’ve implemented this in PR #429.

Seems weird that Rgb888 isn’t actually laid out as Rgb888 and all.

That depends on the endianness of the machine and I agree that it can get weird.

With the problem with this figured out and #424 for dynamic images, I guess this can be closed?

I’m glad that it works now and if you don’t have any additional questions feel free to close the issue.

rfuest on Aug 12, 2020