start work on implementing libdecor for csd #1349

2025-02-18T16:56:21Z

shailpatels commented

2025-02-18 16:56:21 +00:00

(Migrated from github.com)

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: https://github.com/hexops/mach/issues/1298

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

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:

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!

- [X] 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: https://github.com/hexops/mach/issues/1298 This is a screenshot of the core-triangle example running with client side decorations, ![image](https://github.com/user-attachments/assets/197f9c73-519c-47e6-b59d-3f4548ab6e00) 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](https://github.com/user-attachments/assets/a6b7a83e-1fab-4e7a-9555-e405a76e3d27) 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](https://github.com/user-attachments/assets/d16cf78a-e0d5-45df-b0b2-e37bc521aa43) **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!

shailpatels commented

2025-02-23 21:23:20 +00:00

(Migrated from github.com)

CC: @RonaldZielaznicki

ronald-mz (Migrated from github.com) reviewed

2025-04-09 01:20:48 +00:00

ronald-mz (Migrated from github.com) left a comment

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.

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.

src/core/linux/Wayland.zig

					
				@ -176,2 +207,4 @@

				        core_ptr.windows.setValue(window_id, core_window);

				    }

				    // Wait for events to get pushed

				    _ = libwaylandclient.?.wl_display_roundtrip(wl.display);

Was there a reason to move this condition here?

Why not put it in the condition at line 190? Then it could be:

if (wl.interfaces.zxdg_decoration_manager_v1 == null) {
    //unable to use csd or ssd at this point
    return error.NoDecorationSupport;
}

and we'll not do any additional setup unnecessarily.

Was there a reason to move this condition here? Why not put it in the condition at line 190? Then it could be: ```zig if (wl.interfaces.zxdg_decoration_manager_v1 == null) { //unable to use csd or ssd at this point return error.NoDecorationSupport; } ``` and we'll not do any additional setup unnecessarily.

shailpatels (Migrated from github.com) reviewed

2025-04-18 01:04:37 +00:00

src/core/linux/Wayland.zig

					
				@ -176,2 +207,4 @@

				        core_ptr.windows.setValue(window_id, core_window);

				    }

				    // Wait for events to get pushed

				    _ = libwaylandclient.?.wl_display_roundtrip(wl.display);

Oh good point, moved it up to ln 190!

shailpatels commented

2025-04-18 01:06:51 +00:00

(Migrated from github.com)

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.

> 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.

👍 1

ronald-mz commented

2025-05-25 18:57:50 +00:00

(Migrated from github.com)

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!

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!

This pull request is broken due to missing fork information.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin shailpatels/start_libdecor_implementation:shailpatels/start_libdecor_implementation

git switch shailpatels/start_libdecor_implementation

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main

git merge --no-ff shailpatels/start_libdecor_implementation

git switch shailpatels/start_libdecor_implementation

git rebase main

git switch main

git merge --ff-only shailpatels/start_libdecor_implementation

git switch shailpatels/start_libdecor_implementation

git rebase main

git switch main

git merge --no-ff shailpatels/start_libdecor_implementation

git switch main

git merge --squash shailpatels/start_libdecor_implementation