lvgl / lv_drivers

TFT and touch pad drivers for LVGL embedded GUI library
https://docs.lvgl.io/master/porting/index.html
MIT License
291 stars 310 forks source link

chore(wayland) : accelerate damage updates, reduce unnecessary cycles #249

Closed HR1025 closed 1 year ago

HR1025 commented 1 year ago

I think it's better to optimize the efficiency according to different occasions here.

HR1025 commented 1 year ago

Before: Peek 2022-11-04 17-13 After: Peek 2022-11-04 17-14

This is when running on my host computer. In fact, it takes about 8ms to refresh the entire image on my embedded target.

kisvegabor commented 1 year ago

Can we keep both? If LV_COLOR_DEPTH is the same as the system's color depth use memcpy else convert it.

HR1025 commented 1 year ago

The system's color depth can be different to LV_COLOR_DEPTH. Is that what you mean? I didn't consider the situation.

Maybe I need some time to consider how to improve it.

HR1025 commented 1 year ago

Hi @kisvegabor , I check the current code and find that this modification is sufficient.

As lv_conf.h show:

/*Color depth: 1 (1 byte per pixel), 8 (RGB332), 16 (RGB565), 32 (ARGB8888)*/
#define LV_COLOR_DEPTH 32

And logic for mapping pixel types in wayland.cpp :

static void shm_format(void *data, struct wl_shm *wl_shm, uint32_t format)
{
    struct application *app = data;

    switch (format)
    {
#if (LV_COLOR_DEPTH == 32)
    case WL_SHM_FORMAT_ARGB8888:
        app->format = format;
        break;
    case WL_SHM_FORMAT_XRGB8888:
        if (app->format != WL_SHM_FORMAT_ARGB8888)
        {
            app->format = format;
        }
        break;
#elif (LV_COLOR_DEPTH == 16)
    case WL_SHM_FORMAT_RGB565:
        app->format = format;
        break;
#elif (LV_COLOR_DEPTH == 8)
    case WL_SHM_FORMAT_RGB332:
        app->format = format;
        break;
#elif (LV_COLOR_DEPTH == 1)
    case WL_SHM_FORMAT_RGB332:
        app->format = format;
        break;
#endif
    default:
        break;
    }
}

I think there will not happen color depth is equal to 24 bits.And we don't need to consider it like fbdev.cpp:

    /*32 bit per pixel*/
    if(vinfo.bits_per_pixel == 32) {
        uint32_t * fbp32 = (uint32_t *)fbp;
        int32_t y;
        for(y = act_y1; y <= act_y2; y++) {
            location = (act_x1 + vinfo.xoffset) + (y + vinfo.yoffset) * finfo.line_length / 4;
            memcpy(&fbp32[location], (uint32_t *)color_p, (act_x2 - act_x1 + 1) * 4);
            color_p += w;
        }
    }
    /*24 bit per pixel*/
    else if(vinfo.bits_per_pixel == 24 && LV_COLOR_DEPTH == 32) {
        uint8_t * fbp8 = (uint8_t *)fbp;
        lv_coord_t x;
        int32_t y;
        uint8_t *pixel;
        for(y = act_y1; y <= act_y2; y++) {
            location = (act_x1 + vinfo.xoffset) + (y + vinfo.yoffset) * finfo.line_length / 3;
            for (x = 0; x < w; ++x) {
                pixel = (uint8_t *)(&color_p[x]);
                fbp8[3 * (location + x)] = pixel[0];
                fbp8[3 * (location + x) + 1] = pixel[1];
                fbp8[3 * (location + x) + 2] = pixel[2];
            }
            color_p += w;
        }
    }
    /*16 bit per pixel*/
    else if(vinfo.bits_per_pixel == 16) {
        uint16_t * fbp16 = (uint16_t *)fbp;
        int32_t y;
        for(y = act_y1; y <= act_y2; y++) {
            location = (act_x1 + vinfo.xoffset) + (y + vinfo.yoffset) * finfo.line_length / 2;
            memcpy(&fbp16[location], (uint32_t *)color_p, (act_x2 - act_x1 + 1) * 2);
            color_p += w;
        }
    }
    /*8 bit per pixel*/
    else if(vinfo.bits_per_pixel == 8) {
        uint8_t * fbp8 = (uint8_t *)fbp;
        int32_t y;
        for(y = act_y1; y <= act_y2; y++) {
            location = (act_x1 + vinfo.xoffset) + (y + vinfo.yoffset) * finfo.line_length;
            memcpy(&fbp8[location], (uint32_t *)color_p, (act_x2 - act_x1 + 1));
            color_p += w;
        }
    }
    /*1 bit per pixel*/
    else if(vinfo.bits_per_pixel == 1) {
        uint8_t * fbp8 = (uint8_t *)fbp;
        int32_t x;
        int32_t y;
        for(y = act_y1; y <= act_y2; y++) {
            for(x = act_x1; x <= act_x2; x++) {
                location = (x + vinfo.xoffset) + (y + vinfo.yoffset) * vinfo.xres;
                byte_location = location / 8; /* find the byte we need to change */
                bit_location = location % 8; /* inside the byte found, find the bit we need to change */
                fbp8[byte_location] &= ~(((uint8_t)(1)) << bit_location);
                fbp8[byte_location] |= ((uint8_t)(color_p->full)) << bit_location;
                color_p++;
            }

            color_p += area->x2 - act_x2;
        }
    } else {
        /*Not supported bit per pixel*/
    }

I think vinfo.bits_per_pixel == 24 && LV_COLOR_DEPTH == 32 is what you worry about before.

kisvegabor commented 1 year ago

The original code could convert to color depth if Wayland's color depth and LVGL's color depth wasn't matching. It seems this functionality is dropped in this PR.

HR1025 commented 1 year ago

There may be situations I don't know, so I revised it according to your suggestion.

But I did check the codes. Though original codes can solve the 24 lv color depth to 32 system color depth, it's impossible in such context. But anyway, thank you for your careful consideration.

kisvegabor commented 1 year ago

The last commit looks good to me. Thank you very much! :slightly_smiling_face: