Confusion about pixel byte order

[e00E]

PixMap::data says the byte order is RGBA. PixMap::pixels returns a cast of the internal u8 slice into PremultipliedColorU8, which says the byte order is ABGR. RGBA != ABGR so it seems like one of the comments is wrong.

I'm further confused how PremultipliedColorU8 can guarantee a byte order when it internally uses u32's native byte order. To create the struct it does u32::from_ne_bytes([r, g, b, a]) and to return the red channel self.0.to_ne_bytes()[0]. This works if users use the red accessor. But if users use the raw u32 or a bytemuck cast then doesn't the resulting order depend on the native endianness?

[RazrFalcon]

It should be RGBA. Will update the docs.

But if users use the raw u32 or a bytemuck cast then doesn't the resulting order depend on the native endianness?

Maybe. Idk. Internally we use either RGBA or Color getters.

PS: I don't really care about big-endian targets, so whatever.

[e00E]

If you don't care about big endian then you could use little endian everywhere (from_ne_bytes -> from_le_bytes etc). This guarantees a consistent byte order and has no performance overhead on little endian targets. Big endian targets will be slower but still work.

Another alternative is to leave the internal layout unspecified, forcing users to always use getters.

[RazrFalcon]

Does it really matter? It seems to be working just fine. And we do test on big-endian.

[e00E]

Correctness matters. If the documentation says the byte order is guaranteed to be RGBA, then it matters whether that is true. If it isn't true, then it shouldn't be documented that way. Hence one solution being that the byte order is documented to be unspecified.

I can see a couple of reasons for why big endian tests still pass:

• My statement about about the byte order is wrong.
• My statement is true but all the tests use PremultipliedColorU8 in a byte order independent way.
• My statement is true but the test doesn't actually run with big endian.

I don't have a big endian machine but maybe I'll try to write a test for it that runs on Github CI.

Edit: This isn't a problem. Since the u32 is always initialized through native byte order, red will always be the first byte, regardless of endianness.

[e00E]

Looking more at the code, I don't think tiny-skia ever uses the u32 directly. I also realize now that there is an even better solution: change struct ColorU8(u32) to struct ColorU8([u8; 4]) . That guarantees the byte order. I don't see any advantage of u32 over the array. Do you remember why you went with u32?

[RazrFalcon]

Will take a look.

Some people suggest using Vec<u32> to begin with (https://github.com/RazrFalcon/tiny-skia/issues/70) to eliminate alignment issues.

It's a low priority for me.

[e00E]

That is a real issue too. Pixmap storesVec<u8> but is casts into Vec<u32>, which has stricter alignment requirements. Would also be fixed by using [u8; 4].

I don't understand why Pixmap stores Vec<u8> when it is always treated as PremultipliedColorU8. It should store Vec<PremultipliedColorU8>. Casting &[T] into &[u8] is always fine because &[u8] has alignment 1 but the other way around isn't. Is there any reason for storing Vec<u8>?

[RazrFalcon]

Is there any reason for storing Vec?

The only reason is that it wasn't an issue until recently. As long as any other non-u8 value doesn't affect performance - I'm fine with it.

[e00E]

I'm willing to make a PR to clean these issues up. I don't think there would be performance impact but not super confident. I see there are benchmarking instructions so I can compare benchmarks on my machine at least.

[e00E]

This isn't a problem. Since the u32 is always initialized through native byte order, red will always be the first byte, regardless of endianness.