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

Wayland GUI backend #1310

Closed
wants to merge 29 commits into from
Closed

Wayland GUI backend #1310

wants to merge 29 commits into from

Conversation

bi4k8
Copy link
Collaborator

@bi4k8 bi4k8 commented Nov 28, 2022

Checklist

  • I have described the changes
  • I have linked to any relevant GitHub issues, if applicable
  • Documentation in doc/ has been updated
  • All new code is licensed under GPLv3

Description

I have an old (last April) branch implementing a Wayland backend that I've finally fixed to not break X11 anymore. This needs lots of cleanups, but I wanted to get a PR open so people can test and I can get feedback.

Fixes #56.

This probably needs a huge rebase at this point. Rebased.

I've tested it under labwc and sway, but not other compositors. It uses Cairo for all rendering and the wlr-layer-shell-unstable-v1 protocol to place its window appropriately. This moves a relatively large amount of code from X11 files to shared GUI ones. It adds a out_to_wayland configuration setting; setting out_to_wayland = true should be all that's needed to test an existing config.

@netlify
Copy link

netlify bot commented Nov 28, 2022

Deploy Preview for conkyweb ready!

Name Link
🔨 Latest commit 75066bd
🔍 Latest deploy log https://app.netlify.com/sites/conkyweb/deploys/63a70d66091f7000094cde24
😎 Deploy Preview https://deploy-preview-1310--conkyweb.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions github-actions bot added extras Issue or PR that suggests/modifies extras sources PR modifies project sources documentation Issue or PR that suggests documentation improvements labels Nov 28, 2022
@bi4k8
Copy link
Collaborator Author

bi4k8 commented Nov 28, 2022

I'll clean up a lot of the smaller tweak commits. For example,

  • HACK: remove sleep(1) on config reload doesn't need to be there
  • bonus flush, remove resizing print, remove unused wl_shell binding, and a number of others will get squashed

and a bunch more.

That said, I'd like particular attention on these commits:

  • fix segfault when no display outputs are enabled (is this a sane fix?)
  • fix misbehavior in specials handling that fails badly with Wayland ba… (the code here seemed wrong, but it isn't clear why it would behave worse with Wayland than X11)
  • fix right-alignment with wayland (I don't understand why this works, nor why it was initially broken on Wayland compared to X11)

@bi4k8 bi4k8 marked this pull request as ready for review November 28, 2022 16:14
@bi4k8
Copy link
Collaborator Author

bi4k8 commented Nov 28, 2022

Cleaned everything up substantially. I think this is now in a reviewable state.

EDIT: I just realized I didn't have meaningful committer info in this history; I'll fix that too.

@bi4k8 bi4k8 force-pushed the wayland-gui-backend branch 2 times, most recently from 1c76e79 to d9c6c48 Compare November 28, 2022 19:23
@mmuman
Copy link
Collaborator

mmuman commented Nov 30, 2022

I guess I'll have to have a try at a Wayland session soon :-)

You may want to use clang-format over the source files.

@mmuman
Copy link
Collaborator

mmuman commented Nov 30, 2022

Not sure there's a need to have a separate wl.cc file. I left the x11.cc because I didn't want to touch too much of it, but possibly it could be merged someday.

@brndnmtthws
Copy link
Owner

Can you rebase on top of #1250 (or wait for it to get merged)? I want to make sure these don't conflict.

@bi4k8
Copy link
Collaborator Author

bi4k8 commented Dec 4, 2022

Can you rebase on top of #1250 (or wait for it to get merged)? I want to make sure these don't conflict.

Sure. I'll wait for #1250 to merge first so I'm not rebasing onto a moving target.

@bi4k8 bi4k8 mentioned this pull request Dec 8, 2022
@github-actions github-actions bot added the gh-actions Issue or PR that suggest changing GitHub actions label Dec 9, 2022
@bi4k8 bi4k8 force-pushed the wayland-gui-backend branch 9 times, most recently from cf90d83 to 9410b8e Compare December 12, 2022 23:44
@bi4k8
Copy link
Collaborator Author

bi4k8 commented Dec 12, 2022

Tests are now green.

@mmuman
Copy link
Collaborator

mmuman commented Dec 13, 2022

Great, still busy this week, hopefully I'll get time to try it next week.

@github-actions github-actions bot added the web Issue or PR related to documentation website label Dec 24, 2022
@brndnmtthws
Copy link
Owner

This is a big change, and there's a decent chance things will break, but I think we can ship it just in time for Xmas (if that's something you care about) 🙂

Thanks for the PR, this is great!

@brndnmtthws
Copy link
Owner

I merged this manually: cf16110

@stacyharper
Copy link

stacyharper commented Dec 24, 2022

I tried to compile master and got this:

CMake Warning (dev) at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:438 (message):                                                              
  The package name passed to `find_package_handle_standard_args`                                                                                                
  (WAYLAND_CLIENT) does not match the name of the calling package (Wayland).                                                                                    
  This can lead to problems in calling code that expects `find_package`                                                                                         
  result variables (e.g., `_FOUND`) to follow a certain pattern.                                                                                                
Call Stack (most recent call first):                                                                                                                            
  cmake/FindWayland.cmake:44 (find_package_handle_standard_args)                                                                                                
  cmake/ConkyPlatformChecks.cmake:356 (find_package)                                                                                                            
  CMakeLists.txt:35 (include)                                                                                                                                   
This warning is for project developers.  Use -Wno-dev to suppress it.                                                                                           
                                                                                                                                                                

and then some issues while building. If it can help debuging. It is from Alpine Linux

Dear santa is lovely this year \o/

@brndnmtthws
Copy link
Owner

I tried to compile master and got this:

CMake Warning (dev) at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:438 (message):                                                              
  The package name passed to `find_package_handle_standard_args`                                                                                                
  (WAYLAND_CLIENT) does not match the name of the calling package (Wayland).                                                                                    
  This can lead to problems in calling code that expects `find_package`                                                                                         
  result variables (e.g., `_FOUND`) to follow a certain pattern.                                                                                                
Call Stack (most recent call first):                                                                                                                            
  cmake/FindWayland.cmake:44 (find_package_handle_standard_args)                                                                                                
  cmake/ConkyPlatformChecks.cmake:356 (find_package)                                                                                                            
  CMakeLists.txt:35 (include)                                                                                                                                   
This warning is for project developers.  Use -Wno-dev to suppress it.                                                                                           
                                                                                                                                                                

and then some issues while building. If it can help debuging. It is from Alpine Linux

Dear santa is lovely this year \o/

I'm in the process of fixing that, check back in a little while :)

@stacyharper
Copy link

Mah, I still got problems with BUILD_WAYLAND=ON

ninja: job failed: /usr/bin/g++ -D_LARGEFILE64_SOURCE -D_POSIX_C_SOURCE=200809L -I/home/willow/sources/aports/main/conky/src/conky-1.16.0/3rdparty/toluapp/inclu
de -I/home/willow/sources/aports/main/conky/src/conky-1.16.0/build -I/usr/include/freetype2 -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0
/include -I/usr/include/harfbuzz -I/usr/include/libpng16 -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/fribidi -I/usr/include/cairo -I/usr/include
/pixman-1 -I/usr/include/lua5.3 -I/usr/include/libxml2 -I/home/willow/sources/aports/main/conky/src/conky-1.16.0/build/data -I/home/willow/sources/aports/main/c
onky/src/conky-1.16.0/build/src -Os -fomit-frame-pointer -Os -DNDEBUG -std=c++17 -MD -MT src/CMakeFiles/conky.dir/display-wayland.cc.o -MF src/CMakeFiles/conky.
dir/display-wayland.cc.o.d -o src/CMakeFiles/conky.dir/display-wayland.cc.o -c /home/willow/sources/aports/main/conky/src/conky-1.16.0/src/display-wayland.cc   
/home/willow/sources/aports/main/conky/src/conky-1.16.0/src/display-wayland.cc: In function 'int set_cloexec_or_close(int)':                                    
/home/willow/sources/aports/main/conky/src/conky-1.16.0/src/display-wayland.cc:80:3: error: 'close' was not declared in this scope; did you mean 'pclose'?      
   80 |   close(fd);                                                                                                                                            
      |   ^~~~~
      |   pclose
/home/willow/sources/aports/main/conky/src/conky-1.16.0/src/display-wayland.cc: In function 'int create_tmpfile_cloexec(char*)':
/home/willow/sources/aports/main/conky/src/conky-1.16.0/src/display-wayland.cc:94:5: error: 'unlink' was not declared in this scope; did you mean 'inline'?
   94 |     unlink(tmpname);
      |     ^~~~~~
      |     inline
/home/willow/sources/aports/main/conky/src/conky-1.16.0/src/display-wayland.cc: In function 'int os_create_anonymous_file(off_t)':
/home/willow/sources/aports/main/conky/src/conky-1.16.0/src/display-wayland.cc:148:5: error: 'close' was not declared in this scope; did you mean 'pclose'?
  148 |     close(fd);
      |     ^~~~~
      |     pclose
/home/willow/sources/aports/main/conky/src/conky-1.16.0/src/display-wayland.cc: In function 'wl_shm_pool* conky::make_shm_pool(wl_shm*, int, void**)':
/home/willow/sources/aports/main/conky/src/conky-1.16.0/src/display-wayland.cc:943:5: error: 'close' was not declared in this scope; did you mean 'pclose'?
  943 |     close(fd);
      |     ^~~~~
      |     pclose
/home/willow/sources/aports/main/conky/src/conky-1.16.0/src/display-wayland.cc:949:3: error: 'close' was not declared in this scope; did you mean 'pclose'?
  949 |   close(fd);
      |   ^~~~~
      |   pclose
ninja: subcommand failed

@brndnmtthws
Copy link
Owner

That's insanely annoying. Can you try #1337?

@stacyharper
Copy link

Yes with this it compile ! And it works !

@mmuman
Copy link
Collaborator

mmuman commented Dec 25, 2022

Cool, we might get some PR from some youtuber soon now :-D

Just a few things I noted when I started reviewing:
The cURL check is now twice in cmake/ConkyPlatformChecks.cmake.
The asserts have been commented in src/fonts.cc and I'm not sure we want this.

@brndnmtthws
Copy link
Owner

brndnmtthws commented Dec 25, 2022

Cool, we might get some PR from some youtuber soon now :-D

Just a few things I noted when I started reviewing: The cURL check is now twice in cmake/ConkyPlatformChecks.cmake. The asserts have been commented in src/fonts.cc and I'm not sure we want this.

I think a lot of things broke with this PR, unfortunately. I should have tested better :)

I'm trying to figure out what caused the breakage, but a lot shifted around so it's not easy to debug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issue or PR that suggests documentation improvements extras Issue or PR that suggests/modifies extras gh-actions Issue or PR that suggest changing GitHub actions sources PR modifies project sources web Issue or PR related to documentation website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Any plan for wayland?
5 participants