adafruit / Adafruit_ILI9341

Library for Adafruit ILI9341 displays
410 stars 283 forks source link

setAddrWindow + pushColor on ESP32 not working properly #41

Closed TheMasterFX closed 5 years ago

TheMasterFX commented 6 years ago

I'm using the JpegDecoder lib (https://github.com/Bodmer/JPEGDecoder) for displaying jpegs on the ILI9341 display. All functions (including graphicstest example) are just working fine. But as soon I'm using the pushColor function in combination with setAddrWindow it doesn't work as expected (blank screen or misaligned pixel fragments). Since the drawPixel function is working fine (which calls writepixel which uses setAddrWindow(x,y,1,1) too) I tried to make it minimalistic as possible. So when I do something like this:

tft.setAddrWindow(0,0,32,32); 
i = 32*32;
while(i--) tft.pushColor(ILI9341_CYAN);

I expected to see a Cyan colored square with 32x32 pixels on the top left. But what I see is a Cyan Line all over the 240px width and 4 px height. So it seems like the controller doesn't limit the frame.

Board: ESP32 Devkit v1.0 Libs: Adafruit ILI9341 v1.2.0 + Adafruit GFX v1.2.9 Framework: ESP-IDF v3.1 (1.4.0)

With my ST7735 everything is working as expected.

Celppu commented 5 years ago

I have similar problem with ST7735s 80*160 display

ladyada commented 5 years ago

take a look at this repo - with me-no-dev's refactor a lot of stuff changed, it may be that library needs a bit of a refresh! https://github.com/adafruit/Adafruit_ImageReader/blob/master/Adafruit_ImageReader.cpp

Bodmer commented 5 years ago

Unfortunately Adafruit have changed the input parameters to the setAddrWindow(), this has broken compatibility with all legacy sketches that used the function and caused grief to library users. :-(

It used to be:

setAddrWindow(x_start, y_start, x_end, y_end); 

If it now:

setAddrWindow(x_start, y_start, width, height); 

You should also use startWrite() and endWrite() to bracket low level writes, this is essential for the ESP32 which uses a mutex to access the SPI hardware. The following change will work for the JPEGDecoder library examples:

    tft.startWrite(); // <<<<<<<<<<<< New

    // draw image MCU block only if it will fit on the screen
    if ( ( mcu_x + win_w) <= tft.width() && ( mcu_y + win_h) <= tft.height())
    {
      // Now set a MCU bounding window on the TFT to push pixels into (x, y, x + width - 1, y + height - 1)
      // tft.setAddrWindow(mcu_x, mcu_y, mcu_x + win_w - 1, mcu_y + win_h - 1); // Old line
      tft.setAddrWindow(mcu_x, mcu_y, win_w, win_h);  // <<<<<<<<<<<< new changed line
      // Write all MCU pixels to the TFT window
      while (mcu_pixels--) tft.pushColor(*pImg++);
    }

    else if ( ( mcu_y + win_h) >= tft.height()) JpegDec.abort();

    tft.endWrite();  // <<<<<<<<<<<< New

A user here also reports that drawRGBBitmap() can be used to paint the jpeg blocks to the screen.

ladyada commented 5 years ago

yep i believe this change came with the @me-no-dev refactor - which did a ton of speedup but broke some code - however if you can have that decoder update their code, we can't fix it here, we've already moved forward!

me-no-dev commented 5 years ago

https://github.com/espressif/WROVER_KIT_LCD/blob/master/src/WROVER_KIT_LCD.cpp#L556-L778

is this what is being discussed here? :) you can copy/paste that code in the ILI or any other such lib. I kept it from this repo as it is ESP32 specific

Bodmer commented 5 years ago

@me-no-dev No this is a different issue. The input parameters to setAddrWindow changed in March 2017 and this went un-noticed. It affects one of the ESP8266+ESP32+Arduino_Due legacy examples in the library here. It can easily be fixed in that library.

Thanks for your concern.

PaintYourDragon commented 5 years ago

setAddrWindow() does not provide its own SPI transaction...you either need to surround this with a tft.startWrite() and tft.endWrite(), like so:

tft.startWrite();
tft.setAddrWindow(0,0,32,32); 
tft.endWrite();
int i = 32*32;
while(i--) tft.pushColor(ILI9341_CYAN);

…but this is somewhat suboptimal in that each call to pushColor() has its own transaction start/end (the function is considered deprecated these days)…so the other option is to use one of the “write” functions to issue pixels without a per-pixel transaction, e.g.:

tft.startWrite();
tft.setAddrWindow(0,0,32,32);
int i = 32*32;
tft.writeColor(ILI9341_CYAN, i);
tft.endWrite();

Instead of writeColor(color, len), there’s also a writePixels(buffer, len) function, or you can SPI_WRITE16(color) each pixel in a loop, within the start/end write transaction.

Bodmer commented 5 years ago

Thanks, I have updated the JPEGDecoder example and used:

tft.drawRGBBitmap(x, y, pImg, w, h);

this works fine.