lvgl / lv_drivers

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

Enhanced Wayland support #173

Closed WallaceIT closed 2 years ago

WallaceIT commented 3 years ago

Hello,

this pull request enhances current Wayland support by adding the following features:

NOTE: xdg-shell and ivi-application are not core protocols and thus require some additional source code to be supported; typically, Wayland protocols are distributed as XML files and .c/.h files are generated by the wayland-scanner utility at build time. This pull request also contains such auto-generation, but an additional (manual) step is required at build time. Details are reported inside the README.

Since the changes requires a break in API compatibility (that is, periodic call to a cycle function is required), this pull request also changes the names for current functions, in order to stress the existence of the breakage.

API breakage has two reasons:

About client-side window decorations: they are very basic, just a simple gray bar on top with close and (only for XDG shell) minimize buttons. Even when enabled at compile time, decorations can be disabled at runtime setting as environment variable LV_WAYLAND_DISABLE_WINDOWDECORATION=1. Resizing and maximization are not supported, since there is no way to notify LVGL that window size is changed (correct me on this point if I'm wrong).

Support for IVI shell (that is, the ivi-application prototcol) has only been smoke-tested at the moment.

Since changes are quite invasive, I marked this as RFC - in particular, I'd like to hear opinions about the code auto-generation for extra protocols.

kisvegabor commented 3 years ago

I don't know wayland, so can not comment on the implementation details. :slightly_frowning_face:

We have a discussion about how to handle small breaking changes but couldn't draw a conclusion yet. IMO it's acceptable to add these minor breaking changes. @embeddedt ?

Resizing and maximization are not supported, since there is no way to notify LVGL that window size is changed (correct me on this point if I'm wrong).

If you see a Wayland event you can use lv_disp_drv_update to set the a new resolution. It already works with the windows port.

embeddedt commented 3 years ago

Why not ask the user to use monitor_cb to run lv_wayland_cycle? That should run it after the entire frame has been drawn (regardless of how many flush_cb calls it takes), which sounds like it's what you want.

Aside from that I think minor API tweaks to Wayland are fine. It's not one of the most popular drivers at the moment anyways.

rzr commented 2 years ago

If it helps, I can do some testing because I am already using it with this demo app:

https://forum.lvgl.io/c/my-projects/10

WallaceIT commented 2 years ago

If you see a Wayland event you can use lv_disp_drv_update to set the a new resolution.

Was not aware of this function, I'll look into it. Currently, I'm not really sure on how to handle resizing at Wayland side while keeping a clean implementation. I tried to mock up something, but client-side decorations resulted in a complete mess.

Why not ask the user to use monitor_cb to run lv_wayland_cycle?

This is actually a good idea, but I'll need to modify slightly lv_wayland_cycle, as it assumes to be run once per cycle (and not once per monitor). I'll try it as soon as I can.

I am also thinking about a way to optimize flush_cb in order to avoid memory copy at each frame - not sure if the two frame buffer mode would help (Wayland requires that the buffers which have to be rendered are allocated from shared memory and not with a simple malloc).

Hope I can find enough time to work on it in the next few days. In the mean time, thank you for the suggestions!

@rzr current version is fully functional if you want to give it a spin. Thank you!

WallaceIT commented 2 years ago

Hello,

following received suggestion about monitor_cb, I refactored a bit the implementation. Now a call to lv_wayland_cycle is no more required: first call to lv_wayland_flush will install a custom monitor_cb callback, which will perform the flush & dispatch to the Wayland compositor. Eventually, if the user application installed an own monitor_cb callback, it will be saved and called as part of the custom one.

I'll keep the PR in draft for now, as I'm still working on removing the memcpy at flush.

WallaceIT commented 2 years ago

Hello,

I've reworked the buffer allocation (now cleaner and with the possibility of zero-copy operation), now I like it a bit more. Testing is still not complete, in particular I am investigating why I am getting always 33FPS from the benchmark demo in all configurations.

I also had to switch away from the monitor_cb solution for the cycling, since it would cause the freeze of the application in case no refressh is happening (as monitor_cb is called only in case of changes). I resolved to rely on a timer:

lv_timer_t *wayland_cycle_timer = lv_timer_create(lv_wayland_cycle, 1, NULL);

In this way, the user can set up the timer at driver initialization and then forget it.

As suggested by @rzr I also removed IVI support for now; I will re-add it once the basic (re-)implementation is completed.

rzr commented 2 years ago

thanks I'll try to test, something that would be welcome too, is maximization, I remember I implemented it for qt-wayland years ago:

https://mastodon.social/@rzr/106170248748212334#qt-tizen-qtwayland-xdg-shell-20140425-rzr

septatrix commented 2 years ago

Is clipboard protocol also planned?

WallaceIT commented 2 years ago

Hello,

I did not consider it (yet?), right now I am working on window maximization

WallaceIT commented 2 years ago

Hello,

I added another round of modifications, with support for:

I reworked yet another time the display initialization, in order to provide the user a cleaner interface; now a single call to lv_wayland_create_window() is required to setup a LVGL display. An example follows:

lv_init();
lv_wayland_init();
disp1 = lv_wayland_create_window(DISP1_HRES, DISP1_VRES, "Window 1", close_cb);
disp2 = lv_wayland_create_window(DISP2_HRES, DISP2_VRES, "Window 2", close_cb);

Left to be done:

I am attaching as reference the minimal example I am using during development : example.c

kisvegabor commented 2 years ago

Hi,

Sounds very good! Thank you! I don't have a device with Wayland to test it now, but I'm sure that you have tested it and it works well. :slightly_smiling_face:

Let me know if I should merge it now, or after adding the mentioned last two features.

WallaceIT commented 2 years ago

Hello,

I just added window resizing through dragging of borders, while I was considering whether to add double-buffering in this round or not (inital testing shows a significant loss in performance, in fact).

In order to simplify integration by end-user of this library, I decided to embed the initialization of the input devices inside the initialization of each window, giving the possibility to get an handle for them (similarly to what is done inside the win32drv driver). I updated the README to reflect this change.

Let's wait another couple of days, while I try to find some touchscreen-equipped device to test the touch integration, then I'll remove the draft flag and clear it for integration.

Thank you

WallaceIT commented 2 years ago

Hello,

I tested (and fixed) the implementation also on a PC with a touch screen, so I think it can be ready for integration. I decided not to integrate (at least for now) the double-buffered feature, since using it performances are really poor. I squashed the commits into three, as they did not really add information about the implementation.

Please let me know if something more is required for the integration.

As a side note: if needed, the implementation can be easily tested on any machine running Linux using the Weston compositor, as it supports an X11 backend and can thus run also on systems still relying on the X window server.

Thank you

kisvegabor commented 2 years ago

Hello,

Thank you very much! I haven't tried it out yet, just reviewed but it looks good in general. :+1:

I merge it now.