Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature(lvgl_port): Initial support for ppa rendering in lvgl (BSP-563) #409

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ThunderDai
Copy link

  Mainly supports the following features:
   - color blend (simple fill)
   - color blend with opa 
   - blend normal (blend with argb8888) 
   - blend normal (color convert / memcpy)

@CLAassistant
Copy link

CLAassistant commented Oct 15, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot changed the title feature(lvgl_port): Initial support for ppa rendering in lvgl feature(lvgl_port): Initial support for ppa rendering in lvgl (BSP-563) Oct 15, 2024
@ThunderDai ThunderDai marked this pull request as draft October 15, 2024 06:44
@ThunderDai ThunderDai marked this pull request as ready for review October 15, 2024 07:34
@ThunderDai
Copy link
Author

@peter-marcisovsky @espzav hi, PTAL, this is my first commit for esp-bsp. Therefore, some code related rules are not familiar enough

Copy link
Collaborator

@peter-marcisovsky peter-marcisovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you consider adding some tests to your feature? If you take a look at my #357 , there are functionality and benchmark unit tests. Would it be possible to do the same for this feature?

Also, can you take a look at the failing CI?

@@ -55,6 +55,21 @@ typedef struct {

extern int lv_color_blend_to_argb8888_esp(asm_dsc_t *asm_dsc);

// Just for compatibility with v9.2.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not recommend this approach of solving compatibility issues with LVGL 9.2.
I tried to build lvlgl_port test app on your branch, with the HW accelerating enabled and the test app would not build, neither with LVGL9.1, nor with LVGL9.2.

peter@peter ➜ ~/esp/esp-bsp/components/esp_lvgl_port/test_apps/lvgl_port git:(pr/409) ✗ idf.py -DSDKCONFIG_DEFAULTS='sdkconfig.defaults;sdkconfig.ci.asm_render' set-target esp32s3 build

Is your feature working with LVGL9.1? If yes, could you consider using just LVGL9.1 in this MR?

We have agreed to make the HW acceleration support working with LVGL9.1, for now here, until we figure out what to do with this brekaing changes which was introduced in LVGL9.2.

@ThunderDai
Copy link
Author

Could you consider adding some tests to your feature? If you take a look at my #357 , there are functionality and benchmark unit tests. Would it be possible to do the same for this feature?

Also, can you take a look at the failing CI?

yes, can add some test. I have already included the newly submitted lvgl_port component as part of my project and have run the benchmark without any issues. I will also try running the existing test_apps. It is worth noting that because PPA has implemented some of your current functions, it conflicts with the current header file. I have added a new header file myself. The following configuration is required:

CONFIG_LV_DRAW_SW_ASM_CUSTOM=y
CONFIG_LV_DRAW_SW_ASM_CUSTOM_INCLUDE="esp_lvgl_port_lv_blend_ppa.h"
CONFIG_LV_USE_CUSTOM_MALLOC=y

@ThunderDai ThunderDai marked this pull request as draft October 21, 2024 08:47
@ThunderDai ThunderDai force-pushed the feat/add_ppa_for_lvgl_render branch 2 times, most recently from 24e762a to 0b1609e Compare October 24, 2024 02:24
@ThunderDai ThunderDai marked this pull request as ready for review October 24, 2024 02:26
@ThunderDai ThunderDai force-pushed the feat/add_ppa_for_lvgl_render branch 5 times, most recently from 865eada to 461a0f8 Compare October 28, 2024 03:07
      Mainly supports the following features:
       - color blend (simple fill)
       - color blend with opa
       - blend normal (blend with argb8888)
       - blend normal to color (color convert / memcpy)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants