Hi Excerpts from Alan Carvalho de Assis's message of July 20, 2022 5:21 pm: > Hi Karel, > > On 7/20/22, Karel Kočí <cyn...@email.cz> wrote: >> Hi >> >> I discovered that commit 664d45dcbace03a879017aa99566592be31f4308 broke LCD >> >> framebuffer (at least for ST7789). The issue is with `putarea` call. >> Originally >> it was called only when full display redraw was requested but now it is >> called >> every time when defined. The core of the issue is that from documentation >> the >> buffer passed to the putarea should contain pixels for that area but LCD >> framebuffer always passes the complete buffer. >> > > Analyzing logically the expected behavior, now putarea() is now called > correctly, so the modification that the guy did on PR #6551 is good. > In the past when you was wanting to update only an area of the LCD > there existed an updatearea() function inside each LCD drivers that > was called. > > If you enable CONFIG_NX_UPDATE you will see that > graphics/nxbe/nxbe_notify_rectangle.c still expecting that > updatearea() inside the lcd_dev_s (I discovered it yesterday and I'm > trying to fix it). > > That function was transfered to lcd_framebuffer.c but we you noticed > it was calling putarea() only to redraw all the display, that is > obviously incorrect. In the other hand, putrun() is a raster function > that normally update the LCD line by line (it could be any row or > column, but not an area like putarea). > > Yesterday I sent the PR #6639 that uses the putarea() to rendering of > APA102 because it was using putrun() and I could see the image be > rendered in the screen because APA102 requires me to send the bit > stream sequentially for all the LEDs in the matrix. > > Now it is very fast as you can see here: > > https://www.youtube.com/watch?v=qA-UmD8ujlE > >> I see two possible fixes. Either we call putarea only when full display >> update >> is requested or we change the definition of the putarea in such a way that >> it is >> driver's resposibility to pick the are from buffer. The first solution is >> simple >> and is pretty much revert to the previous state. The second change is kind >> of >> what is suggested in the new comment added in >> 664d45dcbace03a879017aa99566592be31f4308 but no work was done to propagate >> that >> to actuall API documentation in 'include/nuttx/lcd/lcd.h' 😡. >> >> Thus, what do you think? Revert or propagation of the new behavior? >> > > Although the first solution is simpler it is incorrect (that is the > way it was doing before), putarea exist to update an area of the LCD > screen. I am not against the idea at all. It makes sense.
> I think the right solution is to fix ST7789 to update only the > requested area received by putarea(). I am going to do that (at least for ST7789). I am also going to update documentation for LCD API. Honestly, the only ambiguity and the reason for my original mail and the need to actually debug this was that this comment was ignored https://github.com/apache/incubator-nuttx/pull/6551#discussion_r912844774 and changes were merged without updated documentation. It wasn't hard for me to locate the commit that broke it but it was hard to figure out what was the original idea of the committer as that was missing in the commit message. Going trough comments in PR is not exactly optimal even if I would not ignore PR all together. With regards Karel Kočí
pgpYieK2nB_Jd.pgp
Description: PGP signature
