Ported writeRect to write 16bpp pixmaps from PaulStoffregen/ILI9341_t3 as well as DemoSauce examples #1

marcmerlin · 2017-03-04T05:18:44Z

This is not my code, it was ported from
https://github.com/PaulStoffregen/ILI9341_t3

Tested on ESP32, allows writing 16bpp pixmaps defined in a C file.

This is not my code, it was ported from https://github.com/PaulStoffregen/ILI9341_t3

requires this patch: espressif/WROVER_KIT_LCD#1

me-no-dev · 2017-03-04T10:26:56Z

Adafruit_ILI9341.cpp

+
+// This code was ported/adapted from https://github.com/PaulStoffregen/ILI9341_t3
+// by Marc MERLIN
+void Adafruit_ILI9341::setAddr(uint16_t x0, uint16_t y0, uint16_t x1, uint16_t y1) {


this method already exists ;) it's setAddressWindow

me-no-dev · 2017-03-04T10:28:20Z

Adafruit_ILI9341.cpp

+void Adafruit_ILI9341::writeRect(int16_t x, int16_t y, int16_t w, int16_t h, const uint16_t *pcolors) {
+        startWrite();
+	setAddr(x, y, x+w-1, y+h-1);
+	// setaddrwindow does not work if x!=1 (the picture gets offset) but setAddr works


I'm not sure I get this comment?

read setAddressWindow ;) setAddr(x, y, x+w-1, y+h-1); == setAddessWindow(x,y,w,h)

me-no-dev · 2017-03-04T10:29:04Z

Adafruit_ILI9341.cpp

+	setAddr(x, y, x+w-1, y+h-1);
+	// setaddrwindow does not work if x!=1 (the picture gets offset) but setAddr works
+	//setAddrWindow(x, y, x+w-1, y+h-1);
+	for(y=h; y>0; y--) {


this is equal to writePixels(pcolors, w*h)

me-no-dev · 2017-03-04T10:33:02Z

Adafruit_ILI9341.cpp

+        startWrite();
+	setAddr(x, y, x+w-1, y+h-1);
+	// setaddrwindow does not work if x!=1 (the picture gets offset) but setAddr works
+	//setAddrWindow(x, y, x+w-1, y+h-1);


calling it wrong in this comment

me-no-dev · 2017-03-04T10:34:19Z

Adafruit_ILI9341.cpp

+		}
+		SPI_WRITE16(*pcolors++);
+	}
+        endWrite();


since this method calls startWrite and endWrite, it should be called drawRect not writeRect

me-no-dev · 2017-03-04T10:36:33Z

void Adafruit_ILI9341::drawRect(int16_t x, int16_t y, int16_t w, int16_t h, const uint16_t *pcolors) {
  startWrite();
  setAddressWindow(x,y,w,h);
  writePixels(pcolors, w*h);
  endWrite();
}

- removed redundant setAddr - writeRect renamed to drawRect as recommended by me-no-dev

marcmerlin · 2017-03-04T14:54:25Z

Thank you for your review, clearly it was a bit too late last night for me to get rid of setAddr from the other code branch.
How does this cleaned up version look?

The original suggestion of using drawRect was a bad idea in the end since it conflicts with another totally different drawRect method in the T3 lib. drawBitmap is a better name to say what this does anyway.

Note that this requires espressif/WROVER_KIT_LCD#1 for the graphics support missing from the stock Adafruit library. - changed baud rate to 115200 - on my iotuz board, some demos crash the board, likely due to lack of power from the VREG. I've enabled DEBUG_ANIM = true Please try setting it to false to see if the other demos work in sequence. - analogWrite does not exist on ESP32, change to digitalWrite. - disable font support (not ported/impleneted)

…ith lib. - added missing setScroll and alternate interface for drawRect - modified DemoSauce to work with new Adafruit_ILI9341 lib. Awesome demo! Please try to reset 'DEBUG_ANIM = false' in DemoSauce.

marcmerlin · 2017-03-05T05:18:17Z

Ok, I used the time to finish porting the methods from https://github.com/PaulStoffregen/ILI9341_t3
that were missing for the included 10 screen animation demo, to work.
See a few screenshots and videos: https://goo.gl/photos/bMVqdiAxDkEppuN88

This should be a great stress test for your library. Sadly, on my board some of the demos crash, but this may due to the weak voltage regulator on my board.
Could you try integrating this patch, and running the demo on your board to see if all 10 demos work?
(reset 'DEBUG_ANIM = false' in DemoSauce)

me-no-dev · 2017-03-05T07:44:50Z

Adafruit_ILI9341.cpp

+void Adafruit_ILI9341::drawBitmap(int16_t x, int16_t y, int16_t w, int16_t h, const uint16_t *pcolors) {
+        startWrite();
+	setAddrWindow(x, y, w, h);
+	for(y=h; y>0; y--) {


as I said. those two for loops equal writePixels(pcolors, w*h)

My apologies, I missed that bit in your comment. Fixed, tested, and it works fine with writePixels

me-no-dev · 2017-03-05T07:45:25Z

Adafruit_ILI9341.cpp

+        endWrite();
+}
+
+void Adafruit_ILI9341::setScroll(uint16_t offset)


this is already in the library ;) scrollTo ;)

First, thank you for having a look at all this, and you are of course totally correct. I got mislead by the fact that the ILI9341_VSCRSADD define was missing and assumed the method was too.
I've updated scrollTo to use ILI9341_VSCRSADD

@me-no-dev so scrollTo wsa buggy and didn't work, but I just fixed it with this last uploaded commit.
I'm now able to display a bitmap and scroll it around.

me-no-dev · 2017-03-05T07:46:32Z

Adafruit_ILI9341.h

@@ -155,6 +156,19 @@ class Adafruit_ILI9341 : public Adafruit_GFX {
        void      drawFastHLine(int16_t x, int16_t y, int16_t w, uint16_t color);
        void      fillRect(int16_t x, int16_t y, int16_t w, int16_t h, uint16_t color);

+	// Compat with ILI9341_T3 lib


we need to figure out if the names for the methods make sense with the GFX library, or if we need to move them there...

Understood, this is your call. It's kind of a shame that Paul had a fork for so long and nothing got merged, and now we have diverging names.
If you want to remove the #defines and instead change the code to use the 'native' names, I'd understand.

me-no-dev · 2017-03-05T11:12:16Z

there are many many places where you call multiple draws in a loop of some sort. writing and not drawing could make things faster. See my other comments and look for math issues where the examples do not run.

Added #define setScroll scrollTo to make the lib compatible with Paul's T3 library.

My apologies, this was already in my tree, but I messed up copying this over and those were missing from this PR.

marcmerlin · 2017-03-05T14:46:30Z

I tried to address all your comments (thanks again for the feedback), but about this one: "there are many many places where you call multiple draws in a loop of some sort. writing and not drawing could make things faster. See my other comments and look for math issues where the examples do not run."
Are you talking about code in demosauce? If so, that's not my code, I didn't look at it any longer than it took me to get it to run on my board.
If you mean something else I missed, please point me to what I should fix, and I'll have a look.

marcmerlin · 2017-03-05T15:05:39Z

Ok, I fixed up the pictureEmbed.ino code a bit, and added a scrollTo example in it, which also validates the now fixed and working library code.
If you don't want to keep all the bitmaps or prune this down, feel free, I won't take offence :)

Also, I think I'm done with the changes/improvement I needed on my side, so feel free to pull my changes, modify them in whichever way you see fit, including removing things you don't like if any, and I'll resync with you after that.
Thanks again for your time and looking at all this.

marcmerlin · 2017-03-05T15:18:28Z

Actually commit ae3517d isn't quite good.
Your suggestion to use writePixels the pixels are shifted in packs of 2 I think.
Try it out, but we may have to revert this one CL since the bitmap pixel order is reversed I think (need to do more testing, but I need to head out now)

me-no-dev · 2017-03-05T17:53:10Z

about writePixels, no it does the same exact thing that the for loops did. you increment the input address with each loop. And if you see the inner for loop, it counts down to 1 and not 0, but then it writes another pixel, which is the same exact thing.

As for scroll to, let me see what I have here (cuz I use it)... Yes damned... I must have translated it wrong when I was moving from C to C++

marcmerlin · 2017-03-06T00:02:18Z

@me-no-dev I understand they should be the same, but they are not.
The picture is visibly offset 2 pixels at a time when I use
writePixels((uint16_t*) pcolors, w*h);
See

When I revert to
for(y=h; y>0; y--) {
for(x=w; x>1; x--) {
SPI_WRITE16(*pcolors++);
}
SPI_WRITE16(*pcolors++);
}
endWrite();
the picture looks good again, see

So, clearly the picture looks damaged with
writePixels((uint16_t*) pcolors, w*h);
Are you able to duplicate and see the difference too?

…her column.

me-no-dev · 2017-03-06T01:28:47Z

I would rather get writePixels to work ;) if it is as you say, it's unusable

Code change suggestion from me-no-dev

marcmerlin · 2017-03-06T02:04:22Z

@me-no-dev
size_t pixels = len;
while(pixels--) SPI_WRITE16(*colors++);
worked fine, thanks for the suggestion.
Pull request updated with the new code using writePixels and I can confirm the bitmap is displayed properly now.

marcmerlin · 2017-03-21T00:23:47Z

I'm close to having my code ready for release, but having it depend on an unmerged pull request against the TFT lib, is non ideal :)
I know you're juggling a dozen things as is, but would you be ok merging the non demosauce part of the pull request? That part should be small and easy to review/add.
I may also have time to debug the non working demos in demosauce later, now that I can add extra power to my board to make sure it doesn't crash due to lack of current.

me-no-dev · 2017-03-21T12:20:18Z

@marcmerlin I have started working with the LCD and the code that I need to create (which includes images and whatnot) but have been delayed because the new wrover kits come with ST7789V instead of ILI9341 and I need to adjust the drivers. @ladyada has already merged the changes and we actually need to sync to that and then add the needed method in both drivers or if I can go with just one driver for AVR it will be even better.
Anyway diving into graphics and menus :)

me-no-dev · 2017-03-22T09:21:01Z

@marcmerlin so... I fixed writePixels ;) and talked to ladyada to fork the ili driver to WROVER_LCD and work on that instead. New WROVERs have different LCD chip ST7789V which is really similar to the ILI but has some differences in init and rotation procedures (so far).
The WROVER driver detects which LCD is attached and spins the proper code. I advise us to work on the driver instead and when we have something that is good for all, we can push it to the main ILI driver (and ST driver if created).
I got to write some code to display BMP images on the screen already :) 16 and 24 bit BMPs work. Now onto getting jpegs to display :) I think you are gong to like it ;) reads from the file system and displays.
I'm waiting for someone with Write privileges to Espressif's github to fork the ILI driver to the new name and we can get with it. Agreed?

marcmerlin · 2017-04-01T05:10:42Z

So, I think we can close this pull request, but you said you had a new drawBitmap and much more such methods you wrote, but I'm not seeing them in this code.
Can you make sure either
adafruit#26
adafruit@63e77c0
adafruit@43fb49f
gets merged or push your new code that does a lot more?
Then we can close this pull request as obsolete :)

marcmerlin · 2017-04-14T15:20:05Z

this PR is now obsolete, but #2 replaces it by adding the one method that's still missing to draw bitmaps

New writeRect to write 16bpp pixmaps.

e4f9c2a

This is not my code, it was ported from https://github.com/PaulStoffregen/ILI9341_t3

marcmerlin mentioned this pull request Mar 4, 2017

Using your enhanced library with other chips PaulStoffregen/ILI9341_t3#42

Closed

marcmerlin added a commit to marcmerlin/IoTuz that referenced this pull request Mar 4, 2017

New demo to display a 16ppp bitmap.

0b9f7bc

requires this patch: espressif/WROVER_KIT_LCD#1

me-no-dev reviewed Mar 4, 2017

View reviewed changes

cleanups:

2d455f3

- removed redundant setAddr - writeRect renamed to drawRect as recommended by me-no-dev

marcmerlin added 2 commits March 4, 2017 17:24

drawRect -> drawBitmap.

30b6c4b

The original suggestion of using drawRect was a bad idea in the end since it conflicts with another totally different drawRect method in the T3 lib. drawBitmap is a better name to say what this does anyway.

Unmodified version of DemoSauce ILI9341_t3.

578a4b6

marcmerlin added 2 commits March 4, 2017 21:00

Change include to adafruit lib.

9cf907e

Added missing functions to Adafruit_ILI9341 and made DemoSauce work w…

771dd9e

…ith lib. - added missing setScroll and alternate interface for drawRect - modified DemoSauce to work with new Adafruit_ILI9341 lib. Awesome demo! Please try to reset 'DEBUG_ANIM = false' in DemoSauce.

me-no-dev reviewed Mar 5, 2017

View reviewed changes

comment out unimplemented readrect from TransitionScroll.

c26bc65

marcmerlin changed the title ~~New writeRect to write 16bpp pixmaps.~~ Ported writeRect to write 16bpp pixmaps, and setScroll from PaulStoffregen/ILI9341_t3 as well as DemoSauce examples Mar 5, 2017

marcmerlin changed the title ~~Ported writeRect to write 16bpp pixmaps, and setScroll from PaulStoffregen/ILI9341_t3 as well as DemoSauce examples~~ Ported writeRect to write 16bpp pixmaps from PaulStoffregen/ILI9341_t3 as well as DemoSauce examples Mar 5, 2017

marcmerlin added 3 commits March 5, 2017 05:54

Egg on my face, me-no-dev pointed out that scrollTo already existed.

a27e482

Added #define setScroll scrollTo to make the lib compatible with Paul's T3 library.

Simplified drawBitmap to use writePixels as suggsted by me-no-dev.

ae3517d

Replace all ILI9341_t3 -> Adafruit_ILI9341

5af7ea3

My apologies, this was already in my tree, but I messed up copying this over and those were missing from this PR.

marcmerlin added 2 commits March 5, 2017 06:50

Fixed bug in scrollTo using 8 bit spiwrite instead of 16 bit.

65bf9b2

converted to updated adafruit lib, added bitmap and scroll example.

88dc3b4

Merge branch 'master' into master

2b2d604

Revert ae3517d: writepixels damages the picture by inverting every ot…

dc080b7

…her column.

Improve drawBitmap to use fixed writePixels.

0baae26

Code change suggestion from me-no-dev

Revert drawRect interface convertion, not needed and caused bugs.

0b813b0

marcmerlin mentioned this pull request Mar 23, 2017

Added drawBitmap and demos to display bitmaps. adafruit/Adafruit_ILI9341#26

Merged

marcmerlin closed this Apr 14, 2017

MartyMacGyver mentioned this pull request Jun 26, 2017

Bottom quarter/fifth of LCD not updating/displaying MartyMacGyver/ESP32_Adafruit_ILI9341#3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ported writeRect to write 16bpp pixmaps from PaulStoffregen/ILI9341_t3 as well as DemoSauce examples #1

Ported writeRect to write 16bpp pixmaps from PaulStoffregen/ILI9341_t3 as well as DemoSauce examples #1

marcmerlin commented Mar 4, 2017 •

edited

Loading

me-no-dev Mar 4, 2017

me-no-dev Mar 4, 2017

me-no-dev Mar 4, 2017

me-no-dev Mar 4, 2017 •

edited

Loading

me-no-dev Mar 4, 2017

me-no-dev Mar 4, 2017

marcmerlin Mar 4, 2017

me-no-dev commented Mar 4, 2017

marcmerlin commented Mar 4, 2017

marcmerlin commented Mar 5, 2017

me-no-dev Mar 5, 2017

marcmerlin Mar 5, 2017

me-no-dev Mar 5, 2017

marcmerlin Mar 5, 2017 •

edited

Loading

marcmerlin Mar 5, 2017

me-no-dev Mar 5, 2017

marcmerlin Mar 5, 2017

me-no-dev commented Mar 5, 2017

marcmerlin commented Mar 5, 2017

marcmerlin commented Mar 5, 2017 •

edited

Loading

marcmerlin commented Mar 5, 2017

me-no-dev commented Mar 5, 2017

marcmerlin commented Mar 6, 2017

me-no-dev commented Mar 6, 2017

marcmerlin commented Mar 6, 2017

marcmerlin commented Mar 21, 2017

me-no-dev commented Mar 21, 2017

me-no-dev commented Mar 22, 2017

marcmerlin commented Apr 1, 2017

marcmerlin commented Apr 14, 2017

Ported writeRect to write 16bpp pixmaps from PaulStoffregen/ILI9341_t3 as well as DemoSauce examples #1

Ported writeRect to write 16bpp pixmaps from PaulStoffregen/ILI9341_t3 as well as DemoSauce examples #1

Conversation

marcmerlin commented Mar 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

me-no-dev Mar 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

me-no-dev commented Mar 4, 2017

marcmerlin commented Mar 4, 2017

marcmerlin commented Mar 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcmerlin Mar 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

me-no-dev commented Mar 5, 2017

marcmerlin commented Mar 5, 2017

marcmerlin commented Mar 5, 2017 • edited Loading

marcmerlin commented Mar 5, 2017

me-no-dev commented Mar 5, 2017

marcmerlin commented Mar 6, 2017

me-no-dev commented Mar 6, 2017

marcmerlin commented Mar 6, 2017

marcmerlin commented Mar 21, 2017

me-no-dev commented Mar 21, 2017

me-no-dev commented Mar 22, 2017

marcmerlin commented Apr 1, 2017

marcmerlin commented Apr 14, 2017

marcmerlin commented Mar 4, 2017 •

edited

Loading

me-no-dev Mar 4, 2017 •

edited

Loading

marcmerlin Mar 5, 2017 •

edited

Loading

marcmerlin commented Mar 5, 2017 •

edited

Loading