flacjacket / pywlroots

Python binding to the wlroots library using cffi
University of Illinois/NCSA Open Source License
52 stars 14 forks source link

Update for wlroots 0.16.0 #108

Closed flacjacket closed 1 year ago

flacjacket commented 1 year ago

See https://gitlab.freedesktop.org/wlroots/wlroots/-/tags/0.16.0

m-col commented 1 year ago

Nice! Expect some PRs!

m-col commented 1 year ago

Excerpts from their release notes as potential TODOs to downstream to us (not exhaustive, just things I think are likely needed). Forgive me for ticking things off that I've started to work on locally without submitting yet - this will help me with tracking as I work on it. Of course if anybody else fancies working on anything then just post here :)

Sydiepus commented 1 year ago

Hello, sorry for being late but i wanted to contribute and i don't really know what to do.

I was trying to implement the new protocols, even though i am using the already implemented protocols as reference, i'm still unable to do so.

Is there any documentations that explains the process ?

I apologize if this was asked/answered before, nothing is clear to me after starring at C code for hours.

m-col commented 1 year ago

Hey @Sydiepus, that's awesome that you want to help out, as you can see some things are remaining. The PR https://github.com/flacjacket/pywlroots/pull/109 has all of the updates that I've made so far, i.e. those things listed above that are ticked off.

The process for adding things is a bit ad-hoc and there is no documentation for this, but I can tell you what I do. However, I should note that when I do add things they are pretty biased towards what Qtile needs to support, rather than adding classes that correspond to every struct, or methods for every function (e.g. see gamma control, which needed only a single function for support https://github.com/flacjacket/pywlroots/blob/main/wlroots/wlr_types/gamma_control_v1.py). When adding new protocols I'll either take an educated guess as to what I need (based on reading the source for sway/dwl/other compositors) or add support for that protocol to Qtile at the same time.

So considering how support for a protocol might be implemented in C:

  1. check out the release version of wlroots (that's important)
  2. copy the relevant parts of the headers from wlroots/include/wlr/types into pywlroots/wlroots/ffi_build.py. In many cases not everything within a file is needed to support that feature/protocol. Header extracts in ffi_build.py are largely in alphabetical order but not always, because sometimes things need to come before others otherwise it won't compile.
  3. If the headers come from a new file it will also need to be added here: https://github.com/flacjacket/pywlroots/blob/main/wlroots/ffi_build.py#L2489
  4. wlroots/include/wlr/types/ corresponds to pywlroots/wlroots/types/ so for a new protocol you can add a new python module there, probably easiest to just copy an existing one and follow the same structure.
  5. Most classes subclass Ptr (https://github.com/flacjacket/pywlroots/blob/main/wlroots/__init__.py#L19) which just adds support for testing equality of two python objects based on the underlying pointer.
  6. The names of classes are in most/all cases camel case versions of the C names without the wlr_ prefix, e.g. wlr_gamma_control_manager_v1 becomes GammaControlManagerV1
  7. A tricky thing to consider is how/when the python representation of an object would be created. For example, an instance of GammaControlManagerV1 is expected to be created once at compositor startup, so the wlroots function that creates a gamma control manager is called within __init__(). In contrast, many C functions and structs can be used to get a pointer to the same wlr_surface, so the Surface class is instantiated using an existing pointer, and doesn't create any new C objects. For protocols, the former approach is usually right for the manager struct, but other objects that they might emit likely need the other approach. For example, see the two classes in https://github.com/flacjacket/pywlroots/blob/main/wlroots/wlr_types/idle.py
  8. Structs that have signals use the Signal class in __init__() - just copy existing ones. Sometimes signals emit data too, which you can handle by passing a datawrapper= argument to signal, e.g. https://github.com/flacjacket/pywlroots/blob/main/wlroots/wlr_types/foreign_toplevel_management_v1.py#L55
  9. Add any methods that are needed. In wlroots generally the function name will start with the struct name and take its pointer as a first argument. I don't think their API is stable or 100% consistent but you can do what you think makes sense.
  10. For accessing members on the structs you can expose those via properties, decoding any strings or wrapping objects if appropriate

I hope that helps a bit. It was largely a stream of consciousness but hopefully gives a bit of clarity and maybe a reference to get started with adding new things to pywlroots. Happy to help out more when I can

Sydiepus commented 1 year ago

Thank you !

You explained a lot of things i didn't understand and things that i would have missed, i now have a clearer idea on what to do. I'll try my luck again though i don't expect much.

With hope that'll be able to contribute.

Sydiepus commented 1 year ago

I implemented the idle-notify-v1 protocol, however i am facing multiple problems.

  1. I am unable to compile the cffi bindings because of render/wlr_renderer.h not being found, shouldn't it be wlr/render/wlr_renderer.h or i'm missing some libraries.
  2. I don't know how i should test my implementation.
m-col commented 1 year ago
  1. Yeah I think you're right that it should be the wlr/ headers, which IIRC constitute their library API, whereas render/wlr_renderer.h is for their use (I think!). What does the error look like? If you are compiling using a git repo copy of their headers rather than system installed headers make sure to set any required env variables such as LD_LIBRARY_PATH
  2. If you added the parts needed for compositors to support it, you would need to add support to a compositor (e.g. the tiny compositor in this repo or qtile) and then test that the bindings are doing what you expect when a client uses the protocol
m-col commented 1 year ago

I figured out the include error and fixed it in https://github.com/flacjacket/pywlroots/pull/109/commits/f815fb655cf7bf21aa577ca96b09cc76efd6359a. When I said LD_LIBRARY_PATH that was a mistake, I meant C_INCLUDE_PATH

m-col commented 1 year ago

Update, commit is here: https://github.com/flacjacket/pywlroots/pull/109/commits/541a738b20f57b85b074cb25b1e287d459a0eede. I had to fix that PR's history as I messed it up a while ago.

Sydiepus commented 1 year ago

Sorry for the late response.

I'm using an alpine edge container to test my changes and i have wlroots installed from the official repos.

Thanks for the update, i would have never dared to touch the includes. I'll try to test the protocol with the tiny compositor.

Thanks again.

m-col commented 1 year ago

Can close this now :)