add rgb666 support

[almindor]

Thank you for helping out with embedded-graphics development! Please:

• Check that you've added passing tests and documentation
• Add a CHANGELOG.md entry in the Unreleased section under the appropriate heading (Added, Fixed, etc) if your changes affect the public API
• Run rustfmt on the project
• Run just build (Linux/macOS only) and make sure it passes. If you use Windows, check that CI passes once you've opened the PR.

PR description

Adds Rgb666 support for displays such as the ILI9486

[jamwaffles]

Hey, thanks for the PR! I'm moving house at the moment but I'll get to this when I've settled in :)

[almindor]

@jamwaffles ping? Also question, does it make any sense to create something like RawU18 for this?

[jamwaffles]

Thanks for the nudge!

does it make any sense to create something like RawU18 for this?

Yes, I think it does. The other colour types represent their exact storage so doing so with Rgb666 keeps things consistent.

[jamwaffles]

p.s. please rebase from master - the CI should start passing after #619 was merged.

[almindor]

p.s. please rebase from master - the CI should start passing after #619 was merged.

Seems there one more issue still in the CI for MSRV?

[almindor]

I'm also unclear how to handle ToBytesfor RawU18 since padding is necessary for any kind of endianness swap logic to work.

Right now I do LE MCU to BE display this way:

        let mut iter = colors.into_iter().flat_map(|c| {
            let s = c.into_storage(); // bit-packed 18bits
                                      // we need to un-pack and pad with 2 bits each into 3 bytes of 6bit info
            let red = ((s & 0x3F) as u8) << 2;
            let green = (((s >> 6) & 0x3F) as u8) << 2;
            let blue = (((s >> 12) & 0x3F) as u8) << 2;
            [red, green, blue]
        });

[almindor]

This should be good to go, the RawU18 representation is a bit funky tho when it comes to the endianness conversions, but then again it never really makes sense.

[bugadani]

Could you please update the PR with a display that uses rgb666? Right now this looks like "yet another random color format". (I read your use case on matrix but IMO it would make sense to include it in the OP.)

[jamwaffles]

LGTM, thanks for your patience while I got round to this. I agree the ToBytes impl is a bit odd for 18 bit colours - it certainly makes more sense for multiples of 8 bits - but it might be useful for somebody, and it's consistent with the other RGB colours.

[jamwaffles]

Right now I do LE MCU to BE display this way:

Because Rgb666 implements RgbColor you should be able to call c.r(), c.g() and c.b() to clean that code up a bit, FWIW.

[jamwaffles]

This is released in e-g-core 0.3.3. e-g itself should pick the changes up with a cargo update.