feat(opengles): add basic driver for opengles
Description of the feature or fix
A clear and concise description of what the bug or new feature is.
Notes
- Update the Documentation if needed.
- Add Examples if relevant.
- Add Tests if applicable.
- If you added new options to
lv_conf_template.hrun lv_conf_internal_gen.py and update Kconfig. - Run
scripts/code-format.py(astyle version v3.4.12 needs to be installed) and follow the Code Conventions. - Mark the Pull request as Draft while you are working on the first version, and mark is as Ready when it's ready for review.
- When changes were requested, re-request review to notify the maintainers.
- Help us to review this Pull Request! Anyone can approve or request changes.
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 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.
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?
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:
With odd hor_res:
I've added this driver to the CI only for building to see the warnings.
See the CI log for warning (treated as errors).
@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.
@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.
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.
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.
I've tested it and found that
LV_COLOR_DEPTH 16with 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 32with even hor_res looked like this:With odd hor_res:
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.
2 issues about the CI:
- 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.
- 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
I made a quick test and it works well so far.
I've also tried to fix the CI.
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
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_tmembersfb2,buf1,buf2if 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.
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).
With odd hor_res: 