Skip to content

Conversation

@S0urceror
Copy link

Description

I noticed that if I'm not updating the whole screen of the QEMU RGB device that the output was incorrect. This is now corrected so that you can now update parts of the screen again.

Related

Testing


Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass.
  • Documentation is updated as needed.
  • Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

@github-actions github-actions bot changed the title rgb_update fix rgb_update fix (QEMU-277) Sep 13, 2025
@igrr igrr requested a review from o-marshmallow September 13, 2025 21:55
@o-marshmallow
Copy link
Collaborator

Hello @S0urceror ,

This is not a bug, the intend is to be able to update any part of the screen thanks to any arbitrary buffer that is given to the EPS_RGB component.
Thanks to this is it possible to use the target's internal RAM, which is liited, to update the screen, similarly to what you'd do with an SPI or I2C screen for example.

With the chage you propose, this is ot possible anymore and we would need to use the fake VRAM buffer QEMU only has (not present on the realy hardware).

If you still want to use the fake frambuffer to update your screen, the proper solution is to modify your ESP-IDF project to pass the correct update_content field, with the address of the area you want to update. Basically, this line:

            uint8_t* source = (uint8_t*)src + (s->from_y * s->width + s->from_x) * bytes_per_pixel;

Should in fact be part of your ESP-IDF project, to set the rgb_qemu_dev_t's update_content field

@S0urceror
Copy link
Author

This code is called from:

/**
 * @brief Draw bitmap on LCD panel
 *
 * @param[in] panel LCD panel handle, which is created by other factory API like `esp_lcd_new_panel_st7789()`
 * @param[in] x_start Start pixel index in the target frame buffer, on x-axis (x_start is included)
 * @param[in] y_start Start pixel index in the target frame buffer, on y-axis (y_start is included)
 * @param[in] x_end End pixel index in the target frame buffer, on x-axis (x_end is not included)
 * @param[in] y_end End pixel index in the target frame buffer, on y-axis (y_end is not included)
 * @param[in] color_data RGB color data that will be dumped to the specific window range
 * @return
 *          - ESP_OK on success
 */
esp_err_t esp_lcd_panel_draw_bitmap(esp_lcd_panel_handle_t panel, int x_start, int y_start, int x_end, int y_end, const void *color_data);

Where y_start points to the start pixel index from where updating the lcd_panel should start. In the current not-updated code it does not matter what value you provide it always start from index 0.

Comparing to similar code for esp_lcd_panel_rgb it looks like this:

// when this function is called, the frame buffer already reflects the draw buffer changes
// if the frame buffer is also mounted to the DMA, we need to do the sync between them
if (!rgb_panel->bb_size && rgb_panel->flags.fb_behind_cache) {
    uint8_t *cache_sync_start = rgb_panel->fbs[draw_buf_fb_index] + (y_start * h_res) * bytes_per_pixel;
    size_t cache_sync_size = (y_end - y_start) * bytes_per_line;
    esp_cache_msync(cache_sync_start, cache_sync_size, ESP_CACHE_MSYNC_FLAG_DIR_C2M | ESP_CACHE_MSYNC_FLAG_UNALIGNED);
}

Here it starts copying from startindex + y_start * h_res * bytes_per_pixel. Just like my code change does.

I am fine keeping your code as is but then we need to reflect this in the documentation.

@S0urceror
Copy link
Author

S0urceror commented Sep 20, 2025

Forget about what I said. I read the function brief again and it specifies:

y_start Start pixel index in the target frame buffer, on y-axis (y_start is included)

For RGB panels the display is DMA synced by PSRAM where source frame == target frame buffer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants