search

almindor / st7789

Rust library for displays using the ST7735 driver
https://docs.rs/st7735-lcd
MIT License
49 stars 25 forks source link

rpi pico/waveshare 1.14in: x,y origin offset by -40,-53 #21

Open cloneable opened 2 years ago

cloneable commented 2 years ago

Hi!

I'm using your crate to drive a Waveshare 1.14in LCD module (240x135 65kRGB) (board with ST7789) connected an RPi Pico. So far everything works, except the pixel origin 0,0 is offset by -40,-53 (x,y; landscape unswapped). I have to translate objects by 40x53 to get them into correct position. I checked Display::init commands and compared them to the reference driver by Waveshare (haven't used), but nothing stands out.

Would you happen to have an idea what could be the reason for this? You have some hardcoded values for a different LCD in the code, but they shouldn't affect this, I believe.

almindor commented 2 years ago

Hey, this seems really specific. How did you figure the offset, just trial and error?

Out of interest could you try using the mipidsi crate? It's a generic driver I wrote after this one and has ST7789 support too. Just to see if this is consistent.

You're using embedded-graphics for drawing right?

cloneable commented 2 years ago

Hey, this seems really specific. How did you figure the offset, just trial and error?

Yes, I'm drawing a triangle the size of the screen and moved it until its points are in the corners.

Out of interest could you try using the mipidsi crate? It's a generic driver I wrote after this one and has ST7789 support too. Just to see if this is consistent.

OK, will do.

You're using embedded-graphics for drawing right?

Yes.

cloneable commented 2 years ago

I noticed the hardcoded framebuffer size of 320x240. My LCD's size is 240x135 and if I add 40px left and right and 53px above and below I get 320x241. Doesn't seem like a coincidence. I'll look into this.

almindor commented 2 years ago

Hmm, so the ST7789 AFAIK was always framebuffer size 320x240 with display size 240x240, that's why I hardcoded it. Do you have a link to your specific display specs/sell page?

We can def. make this into a configurable value but I need to understand what variants are there.

cloneable commented 2 years ago

Hmm, so the ST7789 AFAIK was always framebuffer size 320x240 with display size 240x240, that's why I hardcoded it. Do you have a link to your specific display specs/sell page?

Sure! It's this little guy: Product: https://www.waveshare.com/pico-lcd-1.14.htm Specs: https://www.waveshare.com/wiki/Pico-LCD-1.14

We can def. make this into a configurable value but I need to understand what variants are there.

That would be nice. I'll see if I can make it work with your new driver and show you the changes.

almindor commented 2 years ago

Huh interesting, they do mention the driver is ST7789. I always thought ST7789 was forced into 320x240. Could you please test with this driver and mipidsi as well to see if all works fine if you fix the framebuffer size to match?

If so I'll make changes to both to allow changing the default size.

cloneable commented 2 years ago

OK, I tried your new driver with _sized, but no luck. So I went over the reference driver again and only now I noticed what the SetWindows function does at the top: it translates the origin by 40,53 in landscape (or 52,40 in portrait [yes, 52]) when sending the CASET+RASET instructions. I hacked Display::set_address_window() to simply do the same and everything works, of course.

Sorry about this! So it seems Waveshare simply put the LCD there and drivers have to know about this quirk.

Now I'm not sure how to cleanly make the origin offset configurable when portrait and landscape have different values. Maybe another abstraction layer is best for codifying the properties of ST7789 based modules. For now, I'll do some more experiments with mipidsi.

cloneable commented 2 years ago

And thanks for helping! I think the _sized PR is still useful to expose the actual display size. The framebuffer should always be 240,320, right?

(I noticed that Display::clear clears the entire framebuffer, so maybe the driver itself doesn't need the display size.)

almindor commented 2 years ago

And thanks for helping! I think the _sized PR is still useful to expose the actual display size. The framebuffer should always be 240,320, right?

(I noticed that Display::clear clears the entire framebuffer, so maybe the driver itself doesn't need the display size.)

Yes so the idea behind the framebuffer being > than display size is hw scrolling. Since clear needs to clear the whole thing it needs to use the FB.

I think the best solution to this would be to introduce a waveshare model separate from ST7789 into mipidsi. Would you be willing to do this? it's basically just copy & paste + change in the sizes and the window function call in init.

The other option of doing it would be to make a custom constructor for the ST7789 that does that (and adjusts the sizes similar to my PR).