adafruit / Adafruit-ST7735-Library

This is a library for the Adafruit 1.8" SPI display http://www.adafruit.com/products/358 and http://www.adafruit.com/products/618
https://learn.adafruit.com/1-8-tft-display
565 stars 305 forks source link

ST7789 240x135 display wondering about usage of WIDTH and HEIGHT in setRotation #119

Closed KurtE closed 4 years ago

KurtE commented 4 years ago

@ladyada - Sorry in advance if I am missing something obvious...

But finally picked up a couple of these displays from Digikey and figured out we are not properly supporting this display in our Teensy specific library ST7735_t3 (ST7789_T3) and started to fix it.

The first part was obvious, in the init function needing to add check for this width and height and that made my test program appear to work find in Rotation 0 and Rotation 1, but still have issues with Rotation 2 (and probably 3).

So looked what you are doing in your library (this one). and I see:

  case 2:
    madctl = ST77XX_MADCTL_RGB;
    if ((WIDTH == 135) && (HEIGHT == 240)) {
      _xstart = _colstart - 1;
      _ystart = _rowstart;
    } else {
      _xstart = 0;
      _ystart = 0;
    }
    _width = windowWidth;
    _height = windowHeight;

(similar for case 3) Which maybe logically makes sense, but I am wondering how WIDTH is ever set to 135. The only two references to WIDTH are contained within this project are these two.

I understand that this comes from the base class (GFX), but as far as I know this is only set within the constructor, and I believe that all three constructors for this class look like:

Adafruit_ST7789::Adafruit_ST7789(int8_t cs, int8_t dc, int8_t mosi, int8_t sclk,
                                 int8_t rst)
    : Adafruit_ST77xx(240, 320, cs, dc, mosi, sclk, rst) {}

Again I know I am missing something obvious and in our own library, but thought I would mention this in case it is a real issue.

ladyada commented 4 years ago

@KurtE thanks - it would be great if the _T3 gfx libraries were re-merged to mainline - we have DMA support for a few chips now so you can def have speedy updates.

KurtE commented 4 years ago

@ladyada - Yes at some point would be great to merge some of this stuff back into your main line versions. One problem is that with many of our libraries we have extended them as well.

Things like with many of the libraries we added the support for a logical frame buffer. Where we can do all of the changes in memory and then do an updateScreen. Also added the DMA updates.

But also added features, like we now do opaque text output of the GFX fonts. In may not be 100% correct in all cases, but pretty close. Also added ability to set a clip rectangle and an offset. It also supporte the ILI9341 like fonts in addition to the GFX fonts.

Also borrowed from RA8875 SUMOTOY version the ability to set text cursor to logical CENTER of X and/or y and if you then do a print, it calculates size and centers...

Again may be interesting to see how much of this makes sense to you to add into GFX.

Also just finished the PR back to our version to work with your smaller display.

Kurt

P.S. - I hope you are all doing OK during these days!

ladyada commented 4 years ago

hi we would accept changes like those to GFX as long as the API did not get broken for the thousands of people using the code now :) we already have logical frame buffer support they are called Canvas https://github.com/adafruit/Adafruit-GFX-Library/blob/master/Adafruit_GFX.cpp#L1719

makermelissa commented 4 years ago

@KurtE, you were correct in that this was a bug for rotation 2 and 3. Thanks.