st77xx.py is broken

[amirgon]

After adapting the Micropython Bindings to the new LVGL driver API (lvgl/lvgl#4011), st77xx.py broke.

Related: https://forum.lvgl.io/t/issue-st77xx-py-generic-driver/11216

The problem is as following:

lv_binding_micropython/driver/generic/st77xx.py

Lines 438 to 443 in cf206e1

	def disp_drv_flush_cb(self,disp_drv,area,color):
	# print(f"({area.x1},{area.y1}..{area.x2},{area.y2})")
	self.rp2_wait_dma() # wait if not yet done and DMA is being used
	# blit in background
	self.blit(area.x1,area.y1,w:=(area.x2-area.x1+1),h:=(area.y2-area.y1+1),disp_drv.draw_buf.buf_act.__dereference__(2*w*h),is_blocking=False)
	self.disp_drv.flush_ready()

st77xx.py accesses disp_drv.draw_buf.buf_act directly. This is no longer possible because draw_buf_act is now a private LVGL member that is not exposed to the users. It is not part of the LVGL public API and therefore not part of the Micropython bindings.

The user is supposed to access the data through color argument which represents the buffer.

@eudoxos - What was the reason you used buf_act directly instead of color?

[ngc6589]

Fix color.dereference in st77xx.py

self.blit(area.x1,area.y1,w:=(area.x2-area.x1+1),h:=(area.y2-area.y1+1),color.__dereference__(lv.color_t. __SIZE__*w*h),is_blocking=False)

lv.color_t.SIZE cannot be resolved.

Described at def init(....)
import lvgl as lv
import lv_utils
Better yet, move the import 2 line to the top of the file.

thank you.

[eudoxos]

@amirgon IIRC buf_act is the current output buffer and __dereference__ gets memoryview starting at its address with the size of 2*w*h (2 is bytes per pixel, of course). I dont't see how can this be replaced by dereferencing color?

[amirgon]

lv.color_t.SIZE cannot be resolved.

@ngc6589 I changed it back to 2 instead of lv.color_t. __SIZE__.
I think the original idea was to import and use LVGL only in St77xx_lvgl constructor, which makes the driver easily portable to other libraries and not LVGL specific, so I kept the imports in the constructor.

Please let me know if it works now, as I cannot test this on my side.

I dont't see how can this be replaced by dereferencing color?

@eudoxos Looking at the C callback:

/**
 * Set the flush callback whcih will be called to copy the rendered image to the display.
 * @param disp      pointer to a display
 * @param flush_cb  the flush callback
 */
void lv_disp_set_flush_cb(lv_disp_t * disp, void (*flush_cb)(struct _lv_disp_t * disp, const lv_area_t * area,
                                                             lv_color_t * color_p));

color_p is a pointer to the display buffer, which is an array with element type of lv_color_t.
I agree that this is not well documented, and the name "color" here is a poor choice... (cc: @kisvegabor)
draw_buf_act is internal to LVGL and not part of the API. The driver is expected to access the color_p array instead.

Here is an example from Linux FB driver:

static void flush_cb(lv_disp_t * disp, const lv_area_t * area, lv_color_t * color_p)
{
...
lv_memcpy(&fbp32[location], (uint32_t *)color_p, (area->x2 - area->x1 + 1) * 4);

[ngc6589]

@ngc6589 I changed it back to 2 instead of lv.color_t. SIZE.
I think the original idea was to import and use LVGL only in St77xx_lvgl constructor, which makes the driver easily portable to other libraries and not LVGL specific, so I kept the imports in the constructor.

Please let me know if it works now, as I cannot test this on my side.

I've seen a fix that changed lv.color_t.SIZE back to 2.
As a result of confirmation, it worked without error.

[kisvegabor]

I agree that this is not well documented, and the name "color" here is a poor choice... (cc: @kisvegabor)
draw_buf_act is internal to LVGL and not part of the API. The driver is expected to access the color_p array instead.

What name would you find more intuitive? E.g. rendered_img?

[amirgon]

What name would you find more intuitive? E.g. rendered_img?

First, I think that documentation does not say anything about the callback parameters and their meanings.
Instead of color, even buffer is more intuitive for me, to stand for the buffer where the rendered image data is.

[eudoxos]

Or perhaps pixmap, which conveys that it is a place for pixels?

[kisvegabor]

px_map? We use the px abbreviation.

[amirgon]

px_map? We use the px abbreviation.

Sounds good to me. But please also add comments/docs that explain the callback arguments, so everything would be clear.

[kisvegabor]

I updated it.