red/blue swapped

[brianmay]

~This is kind of weird:~

• ~white is correct~
• ~red is correct~
• ~blue is correct~
• ~green is correct~
• ~magenta comes correct (red + blue)~
• ~yellow comes out correct (red + green) ~
• blue comes out correct from arduino app

~But with this library:~

• ~blue comes out as red.~
• ~cyan comes out as yellow (green + blue).~

LATE: Sorry, disregard most of the above. I tested again, red comes out as blue and blue comes out as red. It does work correctly with arduino.

This seems to be a clear case of red and blue being swapped. But green remains the same.

It is consistent. Filling screen fills it entirely with the same colour. Happens for all draw operations.

With this change:

diff --git a/Cargo.toml b/Cargo.toml
index 8026834..9e1d54e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -15,6 +15,7 @@ display-interface = "0.4.1"
 embedded-graphics-core = "0.3.3"
 embedded-hal = "0.2.6"
 nb = "1.0.0"
+log = "0.4"

 [dependencies.heapless]
 optional = true
diff --git a/src/models/ili9486.rs b/src/models/ili9486.rs
index 4c1475b..cbdb2ae 100644
--- a/src/models/ili9486.rs
+++ b/src/models/ili9486.rs
@@ -96,9 +96,11 @@ impl Model for ILI9486Rgb666 {
     {
         write_command(di, Instruction::RAMWR, &[])?;
         let mut iter = colors.into_iter().flat_map(|c| {
+            use log::*;
             let red = c.r() << 2;
             let green = c.g() << 2;
             let blue = c.b() << 2;
+            error!("ddd {red:?} {green:?} {blue:?}");
             [red, green, blue]
         });

I get a lot of:

 1970-01-01T00:06:11.158Z ERROR mipidsi::models::ili9486                > ddd 0 0 252

This looks like the correct BLUE colour to me. But this was displayed as RED.

So no idea what is going on here. But it does that something weird is going on with the colours.

So maybe the hardware has swapped the red and blue lines? But strange the arduino app works.

[brianmay]

Swapping red/blue in /src/models/ili9486.rs fixes the problem. But still weird. As Green is OK unchanged, and the standard clearly states that the colour order is red, green, and blue.

https://www.elecrow.com/download/ILI9488%20Data%20Sheet_100.pdf section 4.7.1.2. SPI Data for 18-bit/pixel (RGB 6-6-6 Bits Input), 262K-color, on page 120.

[almindor]

This sounds like RGB vs BGR. I wonder if it's similar issue to what this commit supposedly fixed for the ST7789 model.

I remember reading somewhere that the color invert was required when it shouldn't be on some variants of displays but it was specific to the ST7789

[brianmay]

Yes, does sound like RGB vs BGR. I don't see anything in that commit that would help though, unless I missed something.

[KerryRJ]

I am able to reproduce this with the st7735s.

[almindor]

Do we know if this is specific variants only or all cases? We could expose the INVON init somehow.

[brianmay]

For me, this sample arduino app works fine: https://github.com/Makerfabs/Project_Touch-Screen-Camera/tree/master/example/touch_draw_v2

Which uses this library, I think: https://github.com/lovyan03/LovyanGFX

So key question would be, what does that library do differently.

Oh, this is the device I have: https://www.makerfabs.com/esp32-3.5-inch-tft-touch-capacitive-with-camera.html

Quick glance, I see this line: https://github.com/lovyan03/LovyanGFX/blob/c8b09ac1cbf2f9183de432172134470dbd29eb71/src/lgfx/v1/panel/Panel_LCD.cpp#L156

writeData(getMadCtl(_internal_rotation) | (_cfg.rgb_order ? MAD_RGB : MAD_BGR), 1);

This rgb_order is provided by the application: https://github.com/Makerfabs/Project_Touch-Screen-Camera/blob/master/example/touch_draw_v2/touch_draw_v2.ino#L202-L204

// パネルの色順がを設定します。  RGB=true / BGR=false
// 省略時はfalse。赤と青が入れ替わっている場合は設定を変更してください。
panel.rgb_order = false;

Oops. Not sure how I missed seeing this before. I think this makes it pretty clear we have to use BGR. Wonder if it is just a matter of the hardware having the wires connected differently.

[pyaillet]

It seems to confirm the need to expose this configuration somehow.

[brianmay]

I don't think INVON 21h command is going to help here. From the standard:

This command makes no change of the content of the frame memory. Every bit is inverted from the frame memory to the display.

i.e. white becomes black and black becomes white. As per example picture also. Not what we are looking for.

[KerryRJ]

PR #13 fixes this for the st3375s built in to the Waveshare device I have available.

[almindor]

If I understand this correctly, using this MADCTL setting is all that's required here. Is the C app always sending the actual pixel data in RGB format tho?

We can definitely expose this, either as a "model wrapper" with a constructor, or just a runtime choice.

I'm out of workable MCUs atm. waiting for a replacement, so I can't test anything with real hw, but if this is all that's needed I can create a PR and we can discuss and test there.

[KerryRJ]

The st7735s Waveshare C code swaps the MSB and LSB before sending it over the wire BE. I think this will suffer the same problem where half the 6 G bits are also swapped and are no longer the colour that it is meant to be.

Color = ((Color<<8)&0xff00)|(Color>>8);

I have had to do this to make sure that R and B are swapped and G does not have half its bits swapped. I get the correct colours for a background and text.

    let r = c.r();
    let g = c.g();
    let b = c.b();
    (b as u16) << 11 | (g as u16) << 5 | r as u16

The MADCTRL RGB bit has no effect either way, when using the st7735s. I was wondering if this is just a Waveshare st7735s firmware issue if it even has one.

[brianmay]

If I understand this correctly, using this MADCTL setting is all that's required here. Is the C app always sending the actual pixel data in RGB format tho?

I think we concluded that the MADCTL setting has no affect. At least when I tested it. And when @KerryRJ tested it.

Don't understand why there is an RGB/BGR MADCTL setting though when it doesn't appear to do anything.

[KerryRJ]

I think I got to the bottom of MADCTL RGB bit not working. The set_orientation method is overwriting the RGB bit with 0, from the Orientation enum when it is used.

    ///
    /// Sets display [Orientation]
    ///
    pub fn set_orientation(&mut self, orientation: Orientation) -> Result<(), Error<RST::Error>> {
        self.write_command(Instruction::MADCTL)?;
        self.write_data(&[orientation as u8])?;
        self.orientation = orientation;
        Ok(())
    }

Ideally I would like to read MADCTL before updating it in the set_orientation method, but the SPIInterface trait does not support reading data.

The other option is to implement the set_orientation method in each of the models where Orientation and the RGB bit can be set correctly.

Is there a better way to handle this?

[almindor]

Created https://github.com/almindor/mipidsi/pull/14 as first step to fixing this and the orientation issues. @KerryRJ and @brianmay I'd love your input on the PR before I pull it in if you have the time.

NOTE: This won't fix the colors yet, but it should fix the orientation override. I cannot test atm, still out of MCU

[brianmay]

I think I got to the bottom of MADCTL RGB bit not working. The set_orientation method is overwriting the RGB bit with 0, from the Orientation enum when it is used.

Doh. I should have thought of that. I already encountered a similar problem before. Yes, you are absolutely correct. I setup the registers correctly, and it swapped red/blue.

Will look at and comment on #14.

[KerryRJ]

Colours are sorted with #14.

[brianmay]

Not quite finished yet though, we still need to make the initial RGB/BGR value configurable somehow.