Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setAddrWindow + pushColor on ESP32 not working properly #41

Closed
TheMasterFX opened this issue Oct 22, 2018 · 8 comments
Closed

setAddrWindow + pushColor on ESP32 not working properly #41

TheMasterFX opened this issue Oct 22, 2018 · 8 comments

Comments

@TheMasterFX
Copy link

@TheMasterFX TheMasterFX commented Oct 22, 2018

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

This comment has been minimized.

Copy link

@Celppu Celppu commented Nov 29, 2018

I have similar problem with ST7735s 80*160 display

@ladyada

This comment has been minimized.

Copy link
Member

@ladyada ladyada commented Nov 29, 2018

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

This comment has been minimized.

Copy link

@Bodmer Bodmer commented Jan 21, 2019

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

This comment has been minimized.

Copy link
Member

@ladyada ladyada commented Jan 21, 2019

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

This comment has been minimized.

Copy link
Contributor

@me-no-dev me-no-dev commented Jan 22, 2019

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

This comment has been minimized.

Copy link

@Bodmer Bodmer commented Jan 22, 2019

@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

This comment has been minimized.

Copy link
Member

@PaintYourDragon PaintYourDragon commented Mar 22, 2019

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

This comment has been minimized.

Copy link

@Bodmer Bodmer commented Mar 22, 2019

Thanks, I have updated the JPEGDecoder example and used:

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

this works fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.