Add display-list compositing for transparent backgrounds, RGB LCD driver, and new port commands

[drlinux]

Summary

This PR adds several improvements to AtomGL, developed and tested on a Waveshare ESP32-S3 7-inch RGB Touch LCD (waveshare,esp32-s3-touch-lcd-7).

1. Display-list compositing for transparent backgrounds

When a primitive uses transparent background, partial-alpha pixels (from anti-aliased uFont text or RGBA images) were previously blended against the current framebuffer content. With double-buffered region rendering, this produces visual artifacts because the work framebuffer contains uninitialized memory.

The fix adds dcs_lcd_resolve_pixel_rgb565() which walks the display list to find the solid colour from the next lower opaque item, then blends partial-alpha pixels over that resolved colour. This enables correct anti-aliased text on any background without a solid background rectangle behind the text.

2. RGB LCD display driver (ESP-IDF 5+)

A new driver for RGB LCD panels using the ESP32-S3 LCD peripheral. Developed and verified on the Waveshare ESP32-S3 7-inch 800×480 RGB touch display.

Supports:

• Double framebuffering in PSRAM for tear-free rendering
• Region-based partial updates (update_region)
• Direct RGB565 binary drawing (draw_rgb565_raw)
• Base64-encoded RLE RGB565 images with nearest-neighbour scaling
• Fullscreen background image buffer with per-line restore before region renders

Compatible strings: esp_lcd,rgb and waveshare,esp32-s3-touch-lcd-7

3. New port commands

• update_region — partial screen region update (pre-acked)
• draw_rgb565_raw — draw raw RGB565 binary directly (pre-acked)

4. Bug fixes

• Fix missing & 0xFF mask on red channel extraction in rgba8888_color_to_rgb565
• Fix EPD uFont pixel byte order to write RGBA bytes individually instead of relying on uint32 endianness
• Add display_color_to_rgb565 and display_color_to_surface helpers for internal display color conversions

Documentation

• New RGB LCD driver section in docs/display-drivers.md with all options, compat strings, and example
• update_region and draw_rgb565_raw command documentation
• Transparent compositing behaviour documented in docs/primitives.md
• RGB LCD panels listed in README supported hardware

Compatibility

All existing drivers continue to work unchanged. The RGB LCD driver is conditionally compiled for ESP-IDF 5+ only.

Hardware tested

• Waveshare ESP32-S3 7-inch RGB Touch LCD (800×480, RGB565, 16-bit parallel RGB interface)

[petermm]

AMP code review:

PR Review — RGB LCD driver + transparent compositing (last 8 commits)

Reviewed range: ff125ac..0f7fc29 (8 commits), authored by Ibrahim YILMAZ <ibrahim@drlinux.org>.

#	Commit	Summary
1	ff125ac	dcs_lcd_color: add display_color helpers, fix red-channel & 0xFF mask
2	23cccf3	dcs_lcd_draw: display-list compositing for transparent backgrounds
3	a055634	display_task: pre-ack update_region/draw_rgb565_raw, "fix" EPD pixel byte order
4	7fef6bf	Add RGB LCD display driver for ESP-IDF 5+ (rgb_lcd_display_driver.c, 1006 lines)
5	b98879d	docs: RGB LCD driver, update_region, draw_rgb565_raw, transparent compositing
6	3db31a1	README: list RGB LCD panels
7	dbff782	docs: name tested Waveshare ESP32-S3 7-inch panel
8	0f7fc29	rgb_lcd: preserve full framebuffer across region/raw updates

Verdict: request changes. Two of these commits are merge blockers, plus one serious API bug. The documentation commits (5–7) and commit 1 are fine.

---

🔴 Blocker 1 — do_update_region does not compile (missing brace)

Commit 0f7fc29 split if (work_fb >= 0 && render_items_to_framebuffer(...)) into two nested if blocks but never added the matching closing brace. A whole-file brace scan ends at depth 1 (unbalanced). The single-framebuffer fallback loop ends up nested inside the framebuffer_count > 1 branch and the function never closes — the translation unit fails to compile.

rgb_lcd_display_driver.c, do_update_region() (~line 519):

     if (driver->framebuffer_count > 1) {
         int work_fb = select_work_framebuffer(driver);
         if (work_fb >= 0) {
             // Copy entire active framebuffer to the work buffer so content
             // outside the updated region (e.g. cover image drawn via
             // draw_rgb565_raw) is preserved across framebuffer switches.
             uint16_t *active_fb = active_framebuffer(driver);
             if (active_fb) {
                 size_t fb_bytes = (size_t) driver->screen.w * (size_t) driver->screen.h * sizeof(uint16_t);
                 memcpy(driver->framebuffers[work_fb], active_fb, fb_bytes);
             }
-            if (render_items_to_framebuffer(driver, work_fb, x0, y0, width, height, items, len)) {
-            esp_err_t err = switch_to_framebuffer(driver, work_fb);
-            if (err != ESP_OK) {
-                ESP_LOGE(TAG, "region framebuffer switch failed: %s", esp_err_to_name(err));
-            } else {
-                mirror_region_from_active_to_inactive_framebuffers(driver, x0, y0, width, height);
-            }
-        }
+            if (render_items_to_framebuffer(driver, work_fb, x0, y0, width, height, items, len)) {
+                esp_err_t err = switch_to_framebuffer(driver, work_fb);
+                if (err != ESP_OK) {
+                    ESP_LOGE(TAG, "region framebuffer switch failed: %s", esp_err_to_name(err));
+                } else {
+                    mirror_region_from_active_to_inactive_framebuffers(driver, x0, y0, width, height);
+                }
+            }
+        }
         display_items_delete(items, len);
         return;
     }

(The added } closes the if (work_fb >= 0) block, matching the structure already used in do_update(). Indentation of the inner block is also corrected.)

---

🔴 Blocker 2 — epd_draw_pixel byte order regression (drops red, swaps G/B)

Commit a055634 rewrote epd_draw_pixel to "write RGBA bytes individually instead of relying on uint32_t endianness." The stated rationale assumes the pixel buffer is later read with a native little-endian load. It is not: the consumer uses AtomVM's READ_32_UNALIGNED, which is big-endian (it byte-swaps on little-endian targets). So the surface buffer must be laid out in memory as bytes [R, G, B, A].

surface.fg_color is built as value 0x00BBGGRR (red in the low byte, alpha byte cleared) — see display_items.c lines 214–216.

The new code writes:

pixel[0] = (surface->fg_color >> 24) & 0xFFu; // = 0x00  (the cleared alpha byte, NOT red!)
pixel[1] = (surface->fg_color >> 16) & 0xFFu; // = B
pixel[2] = (surface->fg_color >> 8)  & 0xFFu; // = G
pixel[3] = alpha;

→ memory [0x00, B, G, A] → READ_32_UNALIGNED yields 0x00BBGGAA →
rgba8888_color_to_rgb565 reads r = 0 (red lost), g = B, b = G (green/blue swapped).

The original one-liner was correct on the ESP32 (little-endian) target this driver runs on:

*pixel = ((uint32_t) alpha << 24) | (surface->fg_color & 0x00FFFFFFu);
// value 0xAABBGGRR -> native LE store -> memory [R,G,B,A] -> big-endian read 0xRRGGBBAA  ✓

Fix (display_items.c, epd_draw_pixel) — keep explicit bytes but index fg_color correctly:

-    pixel[0] = (surface->fg_color >> 24) & 0xFFu;
-    pixel[1] = (surface->fg_color >> 16) & 0xFFu;
-    pixel[2] = (surface->fg_color >> 8) & 0xFFu;
-    pixel[3] = alpha;
+    pixel[0] = surface->fg_color & 0xFFu;         // R
+    pixel[1] = (surface->fg_color >> 8) & 0xFFu;  // G
+    pixel[2] = (surface->fg_color >> 16) & 0xFFu; // B
+    pixel[3] = alpha;

(Reverting to the original one-liner also works; the explicit-byte form above is endian-portable.)

Note: the fg_color comment ("RGBA8888 little-endian byte order") is misleading — the value is 0x00BBGGRR. Worth clarifying while here.

---

🟠 Serious — RLE draw commands never reply to port.call

process_message() handles draw_rgb565_rle_base64 and draw_rgb565_rle_base64_scaled and then returns without sending an OK reply, relying on the pre-ack path. But try_pre_ack_render_cmd() in display_task.c does not list those two atoms, so a port.call for either RLE command can time out even when the draw succeeds.

Fix (display_task.c, try_pre_ack_render_cmd):

     if (cmd != globalcontext_make_atom(ctx->global, "\x6" "update")
             && cmd != globalcontext_make_atom(ctx->global, "\xD" "update_region")
             && cmd != globalcontext_make_atom(ctx->global, "\xF" "draw_rgb565_raw")
+            && cmd != globalcontext_make_atom(ctx->global, "\x16" "draw_rgb565_rle_base64")
+            && cmd != globalcontext_make_atom(ctx->global, "\x1D" "draw_rgb565_rle_base64_scaled")
             && cmd != globalcontext_make_atom(ctx->global,
                     "\xB" "draw_buffer")) {
         return false;
     }

(Atom lengths verified: draw_rgb565_rle_base64 = 22 = 0x16, draw_rgb565_rle_base64_scaled = 29 = 0x1D.)

---

🟡 Minor — display_init leaks / leaves a half-initialized port on panel failure

If esp_lcd_panel_reset() or esp_lcd_panel_init() fails, the function returns without freeing driver, driver->screen.pixels, or the panel, and without registering ctx->platform_data. rgb_lcd_display_create_port() still returns a Context whose native_handler is set; if it is later driven, display_task_consume_mailbox() dereferences the unset ctx->platform_data. xQueueCreate/xTaskCreate results are also unchecked.

Suggestion: add a single error-cleanup path (free scanline, esp_lcd_panel_del(panel), free driver) for the reset/init failures, mirroring the cleanup already present on the esp_lcd_new_rgb_panel failure, and validate the queue/task creation.

rgb_lcd_display_driver.c (~line 988):

     err = esp_lcd_panel_reset(driver->panel);
     if (err != ESP_OK) {
         ESP_LOGE(TAG, "panel reset failed: %s", esp_err_to_name(err));
+        esp_lcd_panel_del(driver->panel);
+        heap_caps_free(driver->screen.pixels);
+        free(driver);
         return;
     }
     err = esp_lcd_panel_init(driver->panel);
     if (err != ESP_OK) {
         ESP_LOGE(TAG, "panel init failed: %s", esp_err_to_name(err));
+        esp_lcd_panel_del(driver->panel);
+        heap_caps_free(driver->screen.pixels);
+        free(driver);
         return;
     }

---

🟡 Minor — full-frame mirroring/pre-copy cost on every update

mirror_active_to_inactive_framebuffers() does a full ~768 KB PSRAM memcpy (800×480×2) after each successful update, and commit 0f7fc29 additionally copies the full active framebuffer into the work buffer before each region/raw/RLE update. Both being full-frame on partial updates is likely redundant and a measurable throughput cost in PSRAM. Not a correctness bug — consider whether the mirror or the pre-copy can be narrowed to the changed region for partial-update commands.

---

🟢 Nit — num_fbs hardcoded to 2 vs. triple-buffer scaffolding

framebuffers[3] and select_work_framebuffer() carry triple-buffer logic (previous_fb_index), but init requests only num_fbs = 2 and esp_lcd_rgb_panel_get_frame_buffer(panel, 2, ...). The third slot and the previous_fb_index avoidance are effectively dead. Either wire up a configurable/triple-buffer mode or simplify to two. Harmless as-is.

---

✅ Looks good

• ff125ac — the & 0xFF red-channel mask fix in rgba8888_color_to_rgb565 is correct, and the new display_color_* helpers read cleanly.
• 23cccf3 — display-list pixel resolution for transparent (brcolor == 0) backgrounds is a sound approach for double-buffered region rendering; blending partial-alpha over the resolved lower layer rather than uninitialized work-FB memory is the right call.
• Docs/README commits (b98879d, 3db31a1, dbff782) are accurate and helpful.

Suggested merge gate

1. Blocker 1 (compile) — required.
2. Blocker 2 (color regression) — required.
3. Serious (RLE reply/pre-ack) — required.
4. Minors/nits — follow-up acceptable.

Note: findings are from static analysis of the source; not built or run on hardware (driver is ESP-IDF ≥ 5 only).

[bettio]

I see the rationale behind update_region, however I'm not sure about adding this new semantic to the driver.
I would rather find the damaged region by diffing the display list and then using it as update_region. Still the internal framework for this is not yet available, so it might be a reasonable workaround.

[bettio]

I would rather have a function for drawing a buffer, and it should take one of the image tuples we have. See the image branch in init_item (display_items.h).

[bettio]

Feel free to add your name.

[bettio]

This shouldn't be necessary, color is a uint32_t, so >> 24 already returns a value in the range [0, 0xFF].

Your commit says "Fix missing & 0xFF mask on the red channel extraction in rgba8888_color_to_rgb565()."

Are you maybe trying to fix a bug elsewhere?

[bettio]

rgba8888_color_to_rgb565 and display_color_to_rgb565 are the same function.

[drlinux]

Superseded by PR #14.