Skip to content

start work on implementing libdecor for csd #1349

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

shailpatels
Copy link

@shailpatels shailpatels commented Feb 18, 2025

  • By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.

This PR introduces libdecor to the wayland backend to help render client side decorations. See this issue for more context: #1298

This is a screenshot of the core-triangle example running with client side decorations,
image

I'm testing on nixos with gnome mutter which does not support server side decorations, from the logs, the error:

error(mach): Server Side Decorations aren't supported

Falling back to X11

No longer shows!

Here is an example of falling back to ssd rendering if libdecor is not available:
image

This PR is a bit incomplete however, things like resizing are still wonky: (updated added back the code that attempts to recreate the swapchain but causes a segfault for me, if this is removed the behavior looks like the attached video: )
Screencast From 2025-02-18 11-45-01.webm

Other things I'm not sure about

  • libdecor has a couple of plugins implemented for rendering decorations for different backends, it appears these are packaged with the library but I'm not sure if I need to dynamically load them for cross compilation or if there should be a way to select which plugin is used.
  • Need to test server side rendering to make sure I didn't break that functionality
  • Introduced an issue where a keyboard press causes a crash in libxkbcommon's interface This might of been an issue before my changes, it looks like the keyboard listener was not getting called before the core window would be written with null values for the xkb variables, causing a crash because wl.xkb_state was null when keyboardHandleKey was called, this appears to be fixed with another wl_display_roundtrip call which doesn't feel like a great solution, but no sure how else to handle this :/
  • Currently client side decorations are always used if possible, need to implement a way for a user to request which mode, potentially through: https://wayland.app/protocols/xdg-decoration-unstable-v1#zxdg_toplevel_decoration_v1:event:configure?

This is also my first PR to mach so additional feedback / guidance is appreciated :)
Feel free to push changes to the fork as well!

this change dynamically loads the libdecor library to render
client side decorations, this is useful for compositors that
do not support server side decorations on wayland.
not a great solution but if the core_window is set before
the keyboard listener sets the values for libxkbcommon they
are null for the rest of the window initialization
@shailpatels shailpatels marked this pull request as ready for review February 23, 2025 00:07
@shailpatels
Copy link
Author

CC: @RonaldZielaznicki

Copy link
Contributor

@ronald-mz ronald-mz left a comment

Choose a reason for hiding this comment

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

Alright, finally got a time to sit down and look this over. Thank you again for getting this in place. Cause this does follow the logic we laid out in #1298.

As an aside, I'm wondering if we can't do something to shift all the conditions choosing between libdecor and ssd into one place, but I'm ok for some repetition as long as it works. We can clean anything too bothersome up later on.

move exit check earlier and remove if condition inside while loop
@shailpatels
Copy link
Author

Alright, finally got a time to sit down and look this over. Thank you again for getting this in place. Cause this does follow the logic we laid out in #1298.

As an aside, I'm wondering if we can't do something to shift all the conditions choosing between libdecor and ssd into one place, but I'm ok for some repetition as long as it works. We can clean anything too bothersome up later on.

I thought about how we could move all the logic thats conditionally selecting behavior between libdecor and wayland but couldn't organize it in a great way :/
I did move the conditional out of the while loop and had both options casted to a wrapper function which is a bit gross but I think doing something similar for the rest of the file would make a lot harder to read.

@ronald-mz
Copy link
Contributor

You didn't need to worry much about getting everything switched around. As I was happy with the movement of the conditional we discussed here.

Otherwise, thank you for the changes!

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.

2 participants