lvgl icon indicating copy to clipboard operation
lvgl copied to clipboard

feat(opengles): add basic driver for opengles

Open danyyliu opened this issue 1 year ago • 9 comments

Description of the feature or fix

A clear and concise description of what the bug or new feature is.

Notes

danyyliu avatar May 22 '24 15:05 danyyliu

Thank you Dany, we will review it in detail, but I have a quick question to @GorgonMeducer: Shall we add the OpenGL related code/configs to CMSIS-Pack?

kisvegabor avatar May 23 '24 10:05 kisvegabor

@kisvegabor If you want, we can. As you might notice, I have already added PC/Windows/Linux related content to cmsis-pack. I see no special reason to reject any source code as long as you want them. LVGL owns the cmsis-pack. I am only the maintainer.

Once you merge this PR, I can arrange an update to cmsis-pack pdsc.

GorgonMeducer avatar May 23 '24 11:05 GorgonMeducer

Nice to see some work on opengles stuff, but should the driver really be called "opengles"?

There is some windowing and input event handling through glfw3, but those could be done through a SDL2 gles2 context as well. Tying the opengles stuff to glfw3 means it only supports x11/wayland on Linux, for embedded devices the DRM/KMS layer seems quite relevant and is already supported in SDL2.

Would it make sense to try to split the GLES renderer stuff from the glfw3 handling? Is this possible?

falkTX avatar May 24 '24 14:05 falkTX

I've tested it and found that LV_COLOR_DEPTH 16 with even hor_res is working, but with odd hor_res is crashes with:

=================================================================
==74422==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f918f64a3c0 at pc 0x7f91a4589397 bp 0x7ffd43675a50 sp 0x7ffd436751f8
READ of size 1602 at 0x7f918f64a3c0 thread T0
    #0 0x7f91a4589396 in __interceptor_memcpy ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
    #1 0x7f919bfcda39  (/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so+0x117a39)
    #2 0x7f919c514f5e  (/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so+0x65ef5e)
    #3 0x7f919cb4ef32  (/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so+0xc98f32)
    #4 0x7f919c021e2c  (/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so+0x16be2c)
    #5 0x7f919c0233b5  (/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so+0x16d3b5)
    #6 0x7f919bffa2c6  (/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so+0x1442c6)
    #7 0x7f919bffc661  (/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so+0x146661)
    #8 0x55ec6405cdf6 in lv_opengles_texture_init ../lvgl/src/drivers/opengles/lv_opengles_driver.c:295
    #9 0x55ec6405eedf in window_create ../lvgl/src/drivers/opengles/lv_opengles_window.c:362
    #10 0x55ec6405d8a2 in lv_opengles_window_create ../lvgl/src/drivers/opengles/lv_opengles_window.c:111
    #11 0x55ec64177ca9 in hal_init ../main.c:323
    #12 0x55ec64177be4 in main ../main.c:281
    #13 0x7f91a3cabd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #14 0x7f91a3cabe3f in __libc_start_main_impl ../csu/libc-start.c:392
    #15 0x55ec63de1c34 in _start (/home/kisvegabor/projects/lvgl/eclipse-workspace/lv_port_pc_eclipse/Debug/lv_port_pc_eclipse+0x91c34)

LV_COLOR_DEPTH 32 with even hor_res looked like this: image With odd hor_res: image


I've added this driver to the CI only for building to see the warnings.

kisvegabor avatar May 24 '24 19:05 kisvegabor

See the CI log for warning (treated as errors).

kisvegabor avatar May 24 '24 19:05 kisvegabor

@falkTX I find it important to have a standalone OpenGL ES driver for LVGL because in the practice it seems vendors add better support for OpenGL ES than for SDL. In some projects that I'm working only OpenGL ES is available but SDL is not.

kisvegabor avatar May 24 '24 20:05 kisvegabor

@falkTX I find it important to have a standalone OpenGL ES driver for LVGL because in the practice it seems vendors add better support for OpenGL ES than for SDL. In some projects that I'm working only OpenGL ES is available but SDL is not.

Well if the glfw3 layer would be separate it would allow for GLES in both glfw3 and SDL drivers.

falkTX avatar May 24 '24 20:05 falkTX

Well if the glfw3 layer would be separate it would allow for GLES in both glfw3 and SDL drivers.

If we keep both then it makes sense. I guess it will allow adding OpenGL stuff to SDL. In this or a new PR, it would be great to have a function like lv_opengles_bind_to_texture() (or so) to make LVGL draw on any textures without a window. I don't know much about OpenGL but it feel related to your suggestion.

kisvegabor avatar May 24 '24 20:05 kisvegabor

Well if the glfw3 layer would be separate it would allow for GLES in both glfw3 and SDL drivers.

If we keep both then it makes sense. I guess it will allow adding OpenGL stuff to SDL. In this or a new PR, it would be great to have a function like lv_opengles_bind_to_texture() (or so) to make LVGL draw on any textures without a window. I don't know much about OpenGL but it feel related to your suggestion.

I totally agree with @falkTX and you, the code in this PR contains two parts, as we talked in issue 6017: The code in opengl driver directory can be split into two parts: GLFW code for window/mouse/etc; OpenGL code for texture management. The GLFW part should stay in this directory, while the texture management should be moved to draw module.

Actually, the API of opengl texture(i.e. lv_opengles_driver.c), is not good either. Too much detail of opengl code is exposed to GLFW code. I will take some time to modify it.

danyyliu avatar May 27 '24 14:05 danyyliu

I've tested it and found that LV_COLOR_DEPTH 16 with even hor_res is working, but with odd hor_res is crashes with:

=================================================================
==74422==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f918f64a3c0 at pc 0x7f91a4589397 bp 0x7ffd43675a50 sp 0x7ffd436751f8
READ of size 1602 at 0x7f918f64a3c0 thread T0
    #0 0x7f91a4589396 in __interceptor_memcpy ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
    #1 0x7f919bfcda39  (/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so+0x117a39)
    #2 0x7f919c514f5e  (/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so+0x65ef5e)
    #3 0x7f919cb4ef32  (/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so+0xc98f32)
    #4 0x7f919c021e2c  (/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so+0x16be2c)
    #5 0x7f919c0233b5  (/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so+0x16d3b5)
    #6 0x7f919bffa2c6  (/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so+0x1442c6)
    #7 0x7f919bffc661  (/usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so+0x146661)
    #8 0x55ec6405cdf6 in lv_opengles_texture_init ../lvgl/src/drivers/opengles/lv_opengles_driver.c:295
    #9 0x55ec6405eedf in window_create ../lvgl/src/drivers/opengles/lv_opengles_window.c:362
    #10 0x55ec6405d8a2 in lv_opengles_window_create ../lvgl/src/drivers/opengles/lv_opengles_window.c:111
    #11 0x55ec64177ca9 in hal_init ../main.c:323
    #12 0x55ec64177be4 in main ../main.c:281
    #13 0x7f91a3cabd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #14 0x7f91a3cabe3f in __libc_start_main_impl ../csu/libc-start.c:392
    #15 0x55ec63de1c34 in _start (/home/kisvegabor/projects/lvgl/eclipse-workspace/lv_port_pc_eclipse/Debug/lv_port_pc_eclipse+0x91c34)

LV_COLOR_DEPTH 32 with even hor_res looked like this: image With odd hor_res: image

I've added this driver to the CI only for building to see the warnings.

The reason of odd hor_res crash is the buffer to opengl not aligned correctly. The LV_COLOR_DEPTH 32 bug is because no support for this kind of data. I will try to fix it tomorrow.

Update: These two issues fixed. Odd hor_res crash fixed; add suppot for LV_COLOR_DEPTH 8, 16, 24 and 32.

danyyliu avatar May 28 '24 15:05 danyyliu

2 issues about the CI:

  1. The 32bit version of opengl libraries is missing from the install-prerequisites.sh, so the "C/C++ CI / Run tests with 32bit build (pull_request)" fails.
  2. The opengl libraries for Windows is managed by vcpkg. But it seems the libraries download by vcpkg have some special code which is only supported by MSVC. This will cause the failure of "C/C++ CI / Build Windows GCC (pull_request)". @kisvegabor

danyyliu avatar May 28 '24 15:05 danyyliu

I made a quick test and it works well so far.

I've also tried to fix the CI.

kisvegabor avatar May 30 '24 20:05 kisvegabor

I am thinking it's better to rename lv_opengles_windows.c/lv_opengles_mouse.c to lv_glfw_***.c, because these files have no relationship with opengles directly now. If we do this, this opengles directory in driver should be renamed to glfw too. We can leave lv_opengles_driver.c in this directory for now, and move it out after we have a render of opengles. What do you think? @kisvegabor

danyyliu avatar May 31 '24 04:05 danyyliu

I was doing a pedantic review but, given that graceful close+reopen / multiple displays does not seem to be in scope for this PR, all of my suggestions are inapplicable.

Also, I can see that some parts are left to be filled in by other PRs, like using lv_glfw_window_t members fb2, buf1, buf2 if modes other than direct get added.

I agree with you. In this PR, the multiple displays are not supported, the opengl driver APIs don't smell good either. In my mind, this PR is just like a demo, which can just be used as a proof of concept, not a real feature can be used in a real product.

I will go on improving it in the next days, but it will take some time, since I need to go deep into the details of lvgl draw module.

danyyliu avatar Jun 06 '24 14:06 danyyliu

Merging, thank you!

@danyyliu as the next step please focus on making it work with an external texture without a window. It's very important for a customer project.

After that we can go for OpenGL ES based rendering (similar to lv_draw_sdl.c).

kisvegabor avatar Jun 07 '24 20:06 kisvegabor