Make protocols modular

[Paraworker]

Currently Louvre supports new protocols by adding interfaces to the LCompositor class, which not only makes the class very large, but also forces users to use all protocols.

The basic idea is that you can design separate classes for each protocol and ‘register’ them in some way, a bit like what smithay did, but with adjustments based on Louvre and C++.

[Fox2Code]

https://github.com/CuarzoSoftware/Louvre/blob/2.0.0/src/examples/louvre-views/src/Compositor.cpp
https://github.com/CuarzoSoftware/Louvre/blob/2.0.0/src/examples/louvre-views/src/SessionLockManager.cpp
https://github.com/CuarzoSoftware/Louvre/blob/2.0.0/src/lib/core/LSessionLockManager.cpp

The biggest offender is having to implement a copy of each compositor elements, in this example, the need for a custom LSessionLockManager could be totally avoided with for example an event system.

Example of code I have in mind:

Compositor::Compositor() {
    register_event(SessionLockManager_stateChanged_event, &onLockStateChanged)
}

extern "C" void onLockStateChanged(LSessionLockManager sessionLockManager) {
    const bool locked { state() != Unlocked };
}

The SessionLockManager_stateChanged_event would be some kind of struct.
And to make the use of register_event more intuitive, using #define macro can do that trick?

It's only one idea in a sea off possible solutions to start fixing/solving this problem, I'm sure there would be better ideas out there.

[ehopperdietzel]

Hello, I've been very busy lately, so I haven't had much time to work on Louvre, which is why progress has been slow.

Regarding what you mentioned, @Paraworker, the LCompositor class simply follows the factory pattern and only offers virtual constructors for other classes that provide an interface that can be overridden. The idea behind this is for Louvre to provide a "basic" and default implementation of each protocol and other functionalities such as rendering, input handling, etc.

I could group all virtual constructors into a single one and provide an ID of the type of object being requested as an argument, but I think that would be more prone to errors.

The protocols used by the compositor can be modified in LCompositor::createGlobalsRequest(). Therefore, if you wish to omit any, you can do so there, or if you wish to implement one that Louvre does not implement. This way, the API to handle them in the classes will still exist, but it simply wouldn't serve any purpose as clients wouldn't be able to use those protocols.
For example if you disable the session lock global, the LSessionLockManager instance will still be created, but no client will ever request to lock the session or create session lock surfaces.
So what you're saying makes sense here. I could consider implementing some sort of "modules/extensions" concept instead of directly using wl_global_create.

Now, this relates to what @Fox2Code said. The reason I don't use the observer pattern and instead use virtual methods (for the user API) is because it forces the developer to have everything grouped/organized by classes and prevents spaghetti code. This, also allows anyone familiar with Louvre to immediately understand the structure of a project. I know it's tedious, but it only needs to be done once per class.

For version 2.0.0, I'm trying to solve several problems of this nature, so I appreciate your feedback. Does what I'm saying make sense to you?

[8bitprodigy]

Have you considered using an Entity-Component System? I think such a thing would be the good fit for such a problem as this, as this sounds like a problem that would be solved with composition, so people can pick the protocols they want as components and add them to the LCompositor which would be the entity.

[ehopperdietzel]

One more thing I forgot to mention is that when a global is deactivated, the API remains unchanged, the compositor will continue to function without crashing. Clients are free to not bind to a global if they wish anyway, so that isn't an issue. The only effect would be that clients wouldn't have access to that functionality. The only global that Louvre implements internally and cannot be deactivated is wl_output, which is created every time an LOutput is initialized.

[Paraworker]

"modules/extensions" looks something similar to what I mentioned. My idea is for LCompositor to handle only core protocols, while other wl_globals are wrapped by "protocol handler".

For example, in the future, if Louvre needs to support wlr_layer_shell, we would add an LWlrLayerShell class, and there would be something like a module registration function(or macro?), which would be used like this:

class EWlrLayerShell : public LWlrLayerShell
{
    // some impls
};

REGISTER_MODULE(EWlrLayerShell)

In this way, users don't need to care about wl_global directly, and there is no need to add new interfaces in LCompositor, because I am concerned that frequent modifications to LCompositor may break API stability.

[ehopperdietzel]

Alright, yes, I understand that the way I implemented the globals might be confusing. Basically, if you look at the files located in /protocols, you'll notice that some start with R (resource) or G (global). However, both are subclasses of LResource, which is a wrapper for wl_resource. The main difference is that globals have a static method (bind) that creates an instance of itself (the LResource) when a client requests it, which is why it's passed as an argument to wl_global_create.

So, LSessionLockManager doesn't directly interact with globals but rather with the resources created from the global. If the global isn't added, clients can't create the resources, and LSessionLockManager behaves as though no client wants to lock the session.

However, it's not necessary to remove the global if you want to prevent a client from locking the session. LSessionLockManager has a lockRequest() v method, which, by default, returns false to all clients, preventing session locking (all the session lock surfaces remain unmapped). You can override this method to authenticate the client before granting permission. All other globals follow a similar approach, so I would keep them all in the default implementation of LCompositor::createGlobalsRequest().

Now, regarding the other point, I'm considering creating a subclass of LResource, named LGlobalResource, which provides a way to access the wl_interface of the global and the bind method statically. That would be passed to LCompositor::createGlobal() (which I assume would be a template function) along with the desired version. Additionally, LGlobalResource should have a static maxVersion() or similar method to let the developer check up to which version it is implemented. createGlobal() would limit the passed version internally, for example, if it supports up to version 4 and the user specifies 5, it should be capped at 4.

Then LCompositor::createGlobal() would return an instance of LGlobal, allowing for safe removal later if needed through LCompositor::removeGlobal() (when I say "safely," I'm referring to this).

So, the usage would be like this:

LGlobal *seatGlobal = compositor->createGlobal(Protocols::Wayland::GSeat, 6 /* Or Protocols::Wayland::GSeat::maxVersion() */);

// Later for some reason...
compositor->removeGlobal(seatGlobal);


// Instead of
wl_global *seatGlobal = wl_global_create(display(), &wl_seat_interface, 6, this, &GSeat::bind);
wl_global_remove(seatGlobal);
...
wl_global_destroy(seatGlobal);

The version parameter could also have a default value of 0, indicating the use of the maximum version. And of course, if the compositor is stopped, the globals would be automatically removed.

The purpose of this would be simply to prevent users from using wl_global_create directly and having to link to libwayland-server, and also to reduce some boilerplate code, such as having to specify the wl_interface of the globals multiple times.

[Paraworker]

The way of creating/removing global looks good, but from what I understand, do we still need to keep the sessionLockManager() method in LCompositor in order to access LSessionLockManager?

[ehopperdietzel]

It is stored in the private impl of LCompositor, so it makes sense to me. Additionally, I've added a function in the Louvre namespace to access it globally and directly, without requiring compositor()->sessionLockManager(). Where would you put it?"

[Paraworker]

Using a helper function looks reasonable. looking forward to the changes then I can dig into more :)

[ehopperdietzel]

I've finished adding the pointer constraints protocol to the 2.0.0 branch. It should work in all the examples now. Also, I won't be available in the next 2 days, so I'm letting you know in case you don't get a reply from me.