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

Add wayland support #51

Merged
merged 10 commits into from
Oct 24, 2024
Merged

Add wayland support #51

merged 10 commits into from
Oct 24, 2024

Conversation

etag4048
Copy link
Contributor

  • Add support for wayland
  • Update lv_conf with the lastest version
  • Update readme
  • Update CMakelists.txt

BTW the commit made 5h ago made changes to the Makefile, but the documentation states that it's currently broken..
Haven't tried to build with the makefile

@kisvegabor
Copy link
Member

Let's wait for merging Wayland in LVGL and update the submodule here too.

BTW the commit made 5h ago made changes to the Makefile, but the documentation states that it's currently broken..

@100ask does the Makefile work well now?

@100ask
Copy link
Collaborator

100ask commented Jul 25, 2024

does the Makefile work well now?

I am currently planning to fix this issue, the meaning here is not to abandon Makefile, right?

@etag4048
Copy link
Contributor Author

etag4048 commented Jul 25, 2024

@100ask Correct it means that's it's currently broken, and that cmake should be used instead. I've only removed the 'shall' verb, it was in future tense, making it slightly ambiguous. That's great news, let me know when you've fixed so that I can test it and even add the wayland target.

bool err = false;

/* Default values */
fullscreen = maximize = false;
Copy link
Member

@kisvegabor kisvegabor Jul 25, 2024

Choose a reason for hiding this comment

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

Move the config part into function for better readibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean move the switch/case used to get the options to a dedicated function ? Yes sure can

Copy link
Member

@kisvegabor kisvegabor Jul 25, 2024

Choose a reason for hiding this comment

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

Yes, just to keep main() easier to read.

main.c Outdated
Comment on lines 102 to 123
if (tickless == 0 && sleep_wait > 0) {
lv_tick_inc(sleep_wait);
}

/* Handle any Wayland/LVGL timers/events */
time_till_next = lv_wayland_timer_handler();

/* Run until the last window closes */
if (!lv_wayland_window_is_open(NULL)) {
break;
}

/* Wait for something for an event to happen */
if (time_till_next == LV_NO_TIMER_READY) {
sleep_wait = -1;
} else if (time_till_next > INT_MAX) {
sleep_wait = INT_MAX;
} else {
sleep_wait = time_till_next;
}

while ((poll(&pfd, 1, sleep_wait) < 0) && (errno == EINTR));
Copy link
Member

Choose a reason for hiding this comment

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

Can it be part of the Wayland driver?
It would be nice to keep using lv_timer_handler as it is. Can we have a timer are theard in the driver for polling?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lv_wayland_timer_handler was introduced by previous contributors, to poll for events, if there are no events the application sleeps until the next refresh. I don't see how it's possible to do this in the driver, the call to poll(2) needs to know the time to next flush so it must be called from the main run loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am open for suggestions, but it's going to take some more time to refactor and re-test

Copy link
Member

@kisvegabor kisvegabor Jul 25, 2024

Choose a reason for hiding this comment

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

What about just wrapping the whole polling + lv_timer_handler into an lv_wayland_event_loop? Similar to QNX. It' won't keep lv_timer_handlervisible, but at least it's less glue code for the user.

Copy link
Contributor Author

@etag4048 etag4048 Jul 26, 2024

Choose a reason for hiding this comment

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

At first I tried disabling the event driven timer handling, to move out the call to lv_timer_handler out of the driver. But early window maximization/fullscreen doesn't work anymore on weston... It crashes because the shared memory buffers are resized and thus re-arranged. So the references in the draw units of weston become incorrect and it crashes... This is the problem I had a few months back. And the 'solution' was to build fixes around the event driven timer handler, simply because it gives more control over when the lvgl flush occurs. It can not occur in the middle of a commit. To add on top of it the event driven timer was a 'bit of hack' to begin with lvgl/lv_drivers#207. I will study the QNX driver more carefully, I see that they are pausing the timer. Will come up with something similar and make it run on weston...

Copy link
Contributor Author

@etag4048 etag4048 Jul 26, 2024

Choose a reason for hiding this comment

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

After the import, it's also probably a good idea to start a re-write, it needs it...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kisvegabor I've adjusted the driver as much as I could to match the other ones lvgl/lvgl@7783303
I prefer to implement the sleeping when the window hidden or minimized, once we refactor the driver. It currently works well on weston and Mutter which is great. Is that fine with you ? If yes, I will take of the stubs this evening/tommorow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-introduced the sleep when hidden feature, by using the frame done event. It's a slightly different approach than original. Works well, tested and documented.

@etag4048 etag4048 marked this pull request as draft July 26, 2024 10:30
CMakeLists.txt Outdated
# Please set the wanted option to 'ON'. By default, FBDEV is used
# be sure to also enable the selected driver in lv_conf.h

option(LV_USE_WAYLAND "Use the wayland client backend" OFF)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, there is a potential issue here: https://cmake.org/cmake/help/latest/command/option.html

image

Before that, it should be ensured that the cache has been cleared, e.g:

if(DEFINED LV_USE_WAYLAND)
        unset(LV_USE_WAYLAND CACHE)
endif()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @YobeZhou I am not quite well versed at CMake, I've copied what I found in the existing SDL simulator https://github.com/lvgl/lv_port_pc_vscode/blob/master/CMakeLists.txt

I usually rm -rf build when I change options.

@etag4048 etag4048 marked this pull request as ready for review July 29, 2024 09:28
@100ask 100ask closed this Jul 29, 2024
@100ask 100ask reopened this Jul 29, 2024
@100ask
Copy link
Collaborator

100ask commented Jul 29, 2024

@100ask Correct it means that's it's currently broken, and that cmake should be used instead. I've only removed the 'shall' verb, it was in future tense, making it slightly ambiguous. That's great news, let me know when you've fixed so that I can test it and even add the wayland target.

Makefile fixed: #53

lv_conf.h Outdated
#define LV_LINUX_FBDEV_RENDER_MODE LV_DISPLAY_RENDER_MODE_DIRECT
#define LV_LINUX_FBDEV_BUFFER_COUNT 1
#define LV_LINUX_FBDEV_RENDER_MODE LV_DISPLAY_RENDER_MODE_PARTIAL
#define LV_LINUX_FBDEV_BUFFER_COUNT 0
#define LV_LINUX_FBDEV_BUFFER_SIZE 60
#endif
Copy link
Member

Choose a reason for hiding this comment

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

LV_USE_WAYLAND is missing from here.

@anaGrad
Copy link

anaGrad commented Sep 17, 2024

Hello! Any updates on this PR?

@kisvegabor
Copy link
Member

It seems it was forgotten. @etag4048 Will you have time to finalize it?

@etag4048
Copy link
Contributor Author

@kisvegabor @anaGrad Will attempt to finish this week, I wanted to add wayland to the regular makefile too.

@kisvegabor
Copy link
Member

Fantastic, thank you! 👍

@nicusorcitu
Copy link

Hi @etag4048 any progress with this PR? Thanks.

@etag4048
Copy link
Contributor Author

etag4048 commented Oct 3, 2024

@nicusorcitu my colleague @EDGEMTech-GabrielC is currently reviewing, testing as well as making adjustments.

@etag4048 etag4048 force-pushed the add-wayland-support branch from 4b3f802 to 743a790 Compare October 11, 2024 08:52
@etag4048
Copy link
Contributor Author

@nicusorcitu it should be working now, can you test please ?

@etag4048 etag4048 requested a review from kisvegabor October 11, 2024 09:41
@nicusorcitu
Copy link

@anaGrad can you please verify? Thanks.

@anaGrad
Copy link

anaGrad commented Oct 14, 2024

I tested and on first run it looks great!

@nicusorcitu
Copy link

Will it work also with LVGL master/v9.2 branch? can you please verify?

@anaGrad
Copy link

anaGrad commented Oct 14, 2024

Yes, works fine

@kisvegabor
Copy link
Member

@etag4048 please resolve the conflict and we are good to go 🙂

Remove the step that tells to edit the CMakeLists.txt file
and manually enable the option to select the backend
@etag4048 etag4048 force-pushed the add-wayland-support branch from abe682d to b28dc67 Compare October 24, 2024 06:39
@etag4048
Copy link
Contributor Author

@kisvegabor Ready.

@kisvegabor
Copy link
Member

Thank you Erik.

Pleas take a look at the CI, now all the test are failing. :(

@kisvegabor
Copy link
Member

Thank you, Erik! Merging.

@kisvegabor kisvegabor merged commit 8b106e2 into lvgl:master Oct 24, 2024
8 checks passed
@etag4048
Copy link
Contributor Author

@kisvegabor Fixed the CI... I am not familiar with the RHEL, oraclelinux and rocky linux. Not my style of distributions @debug-richard Could you help out and please tell me the name of the packages for wayland-scanner and wayland-client in those distributions? Before the CI I've never had issues compiling on Ubuntu 22.04, I am interested in what issues did you experience when you say you had a hard time building.

@debug-richard
Copy link
Contributor

@etag4048
About my experience:

  1. This project did not specify the required build dependencies. To run the example I had to find the correct dependencies which can take hours and is not very satisfying 😉. They are now documented in the Dockerfiles.
  2. The find_package command tries to find the cmake config for this library. This file is not provided on Debian and Ubuntu. There is a hack on Stackoverflow that suggests creating the file manually, but this is not portable. pkg-config is the distribution independent standard for this.
  3. It is the purpose of CI to build in a clean environment to detect these problems

Unfortunately, you forgot to enable the wayland build in the CI file which means the dependencies for the wayland build are currently missing.

I fired up the Docker containers and these seem to be the required dependencies:

RHEL:
dnf -y install libxkbcommon-devel wayland-devel wayland-protocols-devel
DEBIAN:
apt-get install -y libxkbcommon-dev libwayland-dev wayland-protocols

@etag4048
Copy link
Contributor Author

etag4048 commented Oct 24, 2024

@debug-richard Thanks for the package names, at least on Ubuntu one can use apt-cache search <term> to find packages, but oracle linux idk.. :)
I did not forget to add the wayland test, it's just that I don't want to mess with RHEL

@etag4048
Copy link
Contributor Author

@nicusorcitu I want to perform some small refactoring, like doing a separate file per backend. Improve the readme, to list the required dependencies for popular distros. Also integrating the backend selection to the GNU Makefile would be nice.

@nicusorcitu
Copy link

Sure, any improvement is much appreciated :)

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.

7 participants