Skip to content

Conversation

@MattBrooks95
Copy link
Contributor

@MattBrooks95 MattBrooks95 commented Apr 5, 2024

I started on WLR_scene, but it's not finished.

Some of the C code that WLR_scene needs to work is trying to import a C header that is output from wayland-scanner, We may need to run wayland-scanner on some XML files and then check those C files into the repository as well. I'm not quite sure how to do it so I commented out the structs that I had started to write.

swaywm/wlroots#1180
Unless somebody else has this packaged up for us we may need to modify our build process to generate these files as well.

I looked at what hsroots was doing. I couldn't figure out how to generate these header files with the Cabal build hooks, so I did it manually with a command like
$ wayland-scanner server-code protocol/<xml file> protocol-headers/<header file name>

I also couldn't figure out how to tell Cabal to expect these local header files to be in protocol-headers, so I made a bash script that passes that info in on the command line

UPDATE 2024-04-07 not building because some header files can't be found. I also don't know how to define a marshallable type for array function parameters, and the C union syntax

struct myStruct {
    union {
       type1 foo,
       type2 bar
    };
}

update: wlr_xdg_shell took a long time because the C code includes some headers that I needed to figure out how to generate and include locally.

definition

skipped the dmabuf_compiled stuff because I cannot find the definition
in the Wlroots source or in the wlroots package
how do I import a typedef that defines a function pointer with hs2c?
more function definitions

and more function definitions

ayoooo more functions
@MattBrooks95 MattBrooks95 marked this pull request as draft April 7, 2024 06:34
@cassandracomar
Copy link

cassandracomar commented Apr 12, 2024

I don't think hsc2hs supports union types? bindings-dsl seems much more complete and it definitely supports both array fields and union fields. it provides a more robust implementation of the custom macro provided in Setup.hs in this repo. you'd include the bindings-dsl header (see the tutorial), use their macro to define the struct and union, and let hsc2hs take it from there as part of the build.

e.g.:

#starttype union myUnion
#field foo, type1
#field bar, type2
#stoptype

i.e. it's probably not worth trying to build an entirely bespoke set of macros to do this when the tool already exists?

@bradrn
Copy link
Owner

bradrn commented Apr 12, 2024

My apologies, I just moved countries and haven’t had much time to look at these changes. I’ll try to get around to reviewing it as soon as I can.

bindings-dsl seems much more complete and it definitely supports both array fields and union fields.

Interesting, I hadn’t heard of this library before. My instinct here is that, for such basic bindings, I’d like to avoid external libraries apart from the most widely accepted ‘official’ ones (like hsc2hs).

Also, I note that library hasn’t had an update in 2 years. It doesn’t seem to exist on Hackage either. None of this is a good sign…

@MattBrooks95
Copy link
Contributor Author

MattBrooks95 commented Apr 13, 2024

I learned about the include-dirs directive in Cabal, so I don't need my stupid shell script anymore, and it works in the repl.

library
    include-dirs:
        protocol-headers

Tells it about the xdg_shell and layer_shell local headers created by Wayland Scanner.

If hsc2hs doesn't support Unions, then I guess we could just write the Storable instance by hand, specify the size of the largest possible union member, and then reflect in the types somehow which member of the union is 'active' ?

My apologies, I just moved countries

Hope you've gotten comfy and over the jet lag

@bradrn
Copy link
Owner

bradrn commented Apr 13, 2024

Wait, why do we have any local headers at all? We shouldn’t need them… we can just use wlroots headers on the system, to my understanding.

For that matter, why did you add 15 XML files? My understanding is that those files don’t have enough information to auto-generate bindings… and if they do, then we should get rid of all these manually written bindings and start again.

@MattBrooks95
Copy link
Contributor Author

MattBrooks95 commented Apr 13, 2024

@bradrn the wlroots header files include other header files, some of which do not seem to be present (at least on my system).

Maybe it's because I'm using Nix? The flake that was setup for this project uses cabal2nix so it should find what it needs.

compiler error output Preprocessing library for wlhs-bindings-0.1.0.. compiling /home/motoko/projects/wlhs/dist-newstyle/build/x86_64-linux/ghc-9.4.8/wlhs-bindings-0.1.0/build/WLR/Types/LayerShellV1_hsc_make.c failed (exit code 1) rsp file was: "/home/motoko/projects/wlhs/dist-newstyle/build/x86_64-linux/ghc-9.4.8/wlhs-bindings-0.1.0/build/WLR/Types/hsc2hscall1437508-0.rsp" command was: /nix/store/sfgnb6rr428bssyrs54d6d0vv2avi95c-gcc-wrapper-12.3.0/bin/cc -c /home/motoko/projects/wlhs/dist-newstyle/build/x86_64-linux/ghc-9.4.8/wlhs-bindings-0.1.0/build/WLR/Types/LayerShellV1_hsc_make.c -o /home/motoko/projects/wlhs/dist-newstyle/build/x86_64-linux/ghc-9.4.8/wlhs-bindings-0.1.0/build/WLR/Types/LayerShellV1_hsc_make.o -fuse-ld=gold -D__GLASGOW_HASKELL__=904 -Dlinux_BUILD_OS=1 -Dx86_64_BUILD_ARCH=1 -Dlinux_HOST_OS=1 -Dx86_64_HOST_ARCH=1 -I/nix/store/lplczgbk487vs53jgxr6zd84k8jk4p6z-pixman-0.42.2/include/pixman-1 -I/nix/store/zzpm2hqf6k0ldbw0gihyg0p3d6xc3a2r-wayland-1.22.0-dev/include -I/nix/store/1vwhqdf2cjl7nbrky4cyd6i1qaywik0s-wlroots-0.17.1/include -I/nix/store/lplczgbk487vs53jgxr6zd84k8jk4p6z-pixman-0.42.2/include/pixman-1 -I/nix/store/zzpm2hqf6k0ldbw0gihyg0p3d6xc3a2r-wayland-1.22.0-dev/include -I/nix/store/1vwhqdf2cjl7nbrky4cyd6i1qaywik0s-wlroots-0.17.1/include -I/home/motoko/projects/wlhs/dist-newstyle/build/x86_64-linux/ghc-9.4.8/wlhs-bindings-0.1.0/build/autogen -I/home/motoko/projects/wlhs/dist-newstyle/build/x86_64-linux/ghc-9.4.8/wlhs-bindings-0.1.0/build/global-autogen -include /home/motoko/projects/wlhs/dist-newstyle/build/x86_64-linux/ghc-9.4.8/wlhs-bindings-0.1.0/build/autogen/cabal_macros.h -I/nix/store/0cb2y3kcfw1692z1y7xajr9xcb8m5hma-ghc-9.4.8/lib/ghc-9.4.8/base-4.17.2.1/include -I/nix/store/kiw2pplx7iadra2zy0c204bhngrjw8xg-gmp-with-cxx-6.3.0-dev/include -I/nix/store/0cb2y3kcfw1692z1y7xajr9xcb8m5hma-ghc-9.4.8/lib/ghc-9.4.8/ghc-bignum-1.3/include -I/nix/store/0cb2y3kcfw1692z1y7xajr9xcb8m5hma-ghc-9.4.8/lib/ghc-9.4.8/rts/include -I/nix/store/nflijzxfd78vmqhxxm8nk7lrw245nk2y-libffi-3.4.4-dev/include -I/nix/store/0cb2y3kcfw1692z1y7xajr9xcb8m5hma-ghc-9.4.8/lib/ghc-9.4.8/include/ error: In file included from src-WLR-Types-LayerShellV11437501-0.hsc:8: /nix/store/1vwhqdf2cjl7nbrky4cyd6i1qaywik0s-wlroots-0.17.1/include/wlr/types/wlr_layer_shell_v1.h:16:10: fatal error: wlr-layer-shell-unstable-v1-protocol.h: No such file or directory 16 | #include "wlr-layer-shell-unstable-v1-protocol.h" | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated.
I can't find `wlr-layer-shell-unstable-v1-protocol.h` anywhere, I've tried searching for it in the headers in the wlroots package and I can't find it.

I'm afraid that some of these protocol extensions need to have the header files included in the project locally, which is why I checked in the XML files and used wayland-scanner to generate them.

When I looked at what hsroots had done, they also checked the header files into source control hsroots link.

It's an old comment, but my internet search found this comment on GH that makes it seem like we are supposed to run wayland-scanner and use the header files that it produces:
swaywm wlroots issue link

I only needed to generate two of these header files 'wlr-layer-shell-unstable-v1-protocol.h', and 'xdg-shell-protocol.h', but I checked in the other protocol XML files too in case we needed them.

I'm not sure this is the right solution, but I can't figure out how we could do it otherwise. There is another package, wayland-protocols, but it doesn't seem to have the headers either, only the XML files.

@MattBrooks95 MattBrooks95 marked this pull request as ready for review April 21, 2024 02:42
@bradrn
Copy link
Owner

bradrn commented May 8, 2024

Finally, I got the time to look at this again… and this jumped out at me:

I checked in the XML files and used wayland-scanner to generate them.

I looked up wayland-scanner, and apparently it can generate header files from XML. But, if the XML gives enough information to do that — why are we bothering to write all this stuff out manually at all? Why aren’t we writing code to generate the Haskell bindings directly from the XML? That would be a far better use of our time than slowly working our way through one header file at a time.

@MattBrooks95
Copy link
Contributor Author

@bradrn Welcome back.

if the XML gives enough information to do that — why are we bothering to write all this stuff out manually at all?

I thought we were doing it the way we are so that we can use hsc2hs to generate a bunch of code for us. I don't think making a generic XML-to-Haskell library would be any easier than writing these bindings by hand.

The last people who tried to do Haskell bindings for wlroots ended up trying to implement the Wayland protocol in Haskell (which sounds like what you're proposing) and the result seems to be this (long dead?) project: https://github.com/abooij/sudbury

I think there's no way forward besides writing all of the bindings by hand to hopefully be a catalyst for getting XMonad onto Wayland eventually.

@bradrn
Copy link
Owner

bradrn commented May 11, 2024

I thought we were doing it the way we are so that we can use hsc2hs to generate a bunch of code for us. I don't think making a generic XML-to-Haskell library would be any easier than writing these bindings by hand.

On the contrary, I think it would be immensely easier than writing code by hand. The motivation is the same as for hsc2hs — we want to avoid writing repetitive code as much as possible, to avoid mistakes and to make the library easier to create and maintain. And if the protocol code is repetitive enough to be described in XML, we would certainly want to avoid writing it manually.

@MattBrooks95
Copy link
Contributor Author

@bradrn Another reason why I decided that I should check in the non-standard headers was because they are only a transitive dependency of ours. I don't think we need to import xdg-shell-protocol.h directly, rather, when we import wlroots's wlr_xdg_shell.h, that wlroots code imports "xdg-shell-protocol.h".

I found this GH issue where it looks like some package exists, but I couldn't find a pkgconfig dependency for our Cabal file that made the project build.

This issue also exists that mentions wayland-protocols, but even if I add wayland-protocols to our cabal file's pkgconfig-depends section, it won't build because it says that "xdg-shell-protocol" is missing.

even if I add in the wayland-protocols pkgconfig-depends entry and comment out the line that includes the protocol-headers folder that I made, we still get a build error

    pkgconfig-depends:  wlroots ==0.17.1, wayland-server, pixman-1, wayland-protocols
    -- include-dirs: protocol-headers
    hs-source-dirs:     src

"fatal error: xdg-shell-protocol.h: No such file or directory"

Maybe it doesn't work because I'm on NixOS? I thought cabal2nix was handling these sort of things.

@MattBrooks95
Copy link
Contributor Author

@bradrn I still don't fully understand what's going on, but even other compositors seem to have some sort of build step for running wayland scanner:
hyperwm

@bradrn
Copy link
Owner

bradrn commented May 11, 2024

I still don't fully understand what's going on, but even other compositors seem to have some sort of build step for running wayland scanner

I found this in the Wayland documentation:

The interfaces, requests and events are defined in protocol/wayland.xml. This xml is used to generate the function prototypes that can be used by clients and compositors.

The protocol entry points are generated as inline functions which just wrap the wl_proxy_* functions. The inline functions aren't part of the library ABI and language bindings should generate their own stubs for the protocol entry points from the xml.

It therefore seems clear to me that we’re expected to generate these header files ourselves, just like Hyprland does.

@MattBrooks95
Copy link
Contributor Author

@bradrn Does that mean that I'm on the right track? I honestly wasn't sure.

I was kind of going to just going to generate them manually and check them in, because I'm hoping that wlroots is relatively stable at this point.

Do you think we should have a more automated build flow? I'm not sure I could build it in a reasonable amount of time.

@bradrn
Copy link
Owner

bradrn commented May 12, 2024

Does that mean that I'm on the right track? I honestly wasn't sure.

No, I think it means we should be focussing our efforts on converting the XML to Haskell bindings.

I'm hoping that wlroots is relatively stable at this point.

Alas, many of the headers are specifically marked as ‘unstable’. I don’t think this is an assumption we can make. (And even if it is, XML generation is still less work.)

@MattBrooks95
Copy link
Contributor Author

@bradrn These are headers that describe the Wlroots code (which we're trying to bind to) wlroots repo files

Among these is wlr_xdg_shell, which has a struct like this:

struct wlr_xdg_positioner_rules {
	struct wlr_box anchor_rect;
	enum xdg_positioner_anchor anchor;
	enum xdg_positioner_gravity gravity;
	enum xdg_positioner_constraint_adjustment constraint_adjustment;

	bool reactive;

	bool has_parent_configure_serial;
	uint32_t parent_configure_serial;

	struct {
		int32_t width, height;
	} size, parent_size;

	struct {
		int32_t x, y;
	} offset;
};

Which we can use in our bespoke templating language like this:

{{ struct wlr/types/wlr_xdg_shell.h,
    wlr_xdg_positioner_rules,
    anchor_rect, WLR_box,
    anchor, XDG_positioner_anchor,
    gravity, XDG_positioner_gravity,
    constraint_adjustment, XDG_positioner_constraint_adjustment,
    reactive, CBool,
    has_parent_configure_serial, CBool,
    parent_configure_serial, Word32,
    size width, Int32,
    size height, Int32,
    parent_size width, Int32,
    parent_size height, Int32,
    offset x, Int32,
    offset y, Int32
}}

Which will use hsc2hs features to create a Haskell data structure that has the Storable instance that it needs to be manipulated from Haskell.

In doing so, hsc2hs will include wlr_xdg_shell.h, which then includes xdg-shell-protocol.h using the include syntax that means "search within current project before searching system headers" (the double quotes instead of angle brackets):
#include "xdg-shell-protocol.h"

Which means that we need this xdg-shell-protocol.h file to exist in our repo, no?

Unless you're saying that there exists a Wlroots XML file that describes wlr_xdg_shell.h so perfectly that we can generate code for it in any language, without knowing the size and memory layout of the C structs?

@MattBrooks95
Copy link
Contributor Author

Something I've been thinking of too is that we might not need to write all of these type definitions. I was wondering if some of the hard ones, ones where I had to find header files or look deep into the wlroots or wayland repositories to figure out what to do, maybe those are hidden by wlroots.

Maybe just writing an interface to all of the wlroots public-facing functions that return opaque pointers would be enough?

The ultimate goal is to unblock development of XMonad for Wayland, and if at that time we needed something specific we could write the bindings then instead of trying to write out every single type.

@cassandracomar
Copy link

cassandracomar commented Sep 16, 2024 via email

@jackoe
Copy link

jackoe commented Sep 22, 2024

Apologies if everyone already knows this, I'm writing it down for myself and others.

So I've looked into this, and my conclusion is that the xml doesn't describe wlroots' C api. Instead it describes a wire protocol that the wayland socket ultimately speaks. The xml describes that protocol and its extensions.

Wayland-scanner translates that XML into C that can speak the wire protocol. Wlroots doesn't include that C, and asks instead that its users use wayland-scanner in their build step to make it available. To quote the wiki:

You also need to link to libwayland and wire up wayland-scanner to scan any protocols you want to use. wlroots provides, for example, and xdg-shell implementation, but it expects you to call wayland-scanner to generate the header before you include wlr/types/wlr_xdg_shell.h

Regarding not needing implement the entire thing, I agree. No clue what they do and don't need though.

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.

4 participants