feat: reorganization of batch system and output manager code #18

Merged
JoshStrobl merged 3 commits from code-reorg into main 2025-12-15 14:54:19 +01:00
Owner

The goal of this is to:

  1. Attempt to standardize on some file casing based off what we have done with Display Configurator. I'll get to the dbus part of the codebase in a follow-up PR since I want to move system-specific DBus Services under those systems rather than them being top-level.
  2. Introduce clearer namespacing and classes. Not exhaustive, but some examples.
    a. bd::Outputs - Contains output-related classes (excluding DBus and display-config TOML parsing, for now)
    b. bd::Outputs::Wlr - Wlroots-specific has been moved under this namespace.
    c. WaylandOrchestrator renamed to bd::Outputs::State to reflect it is intended to be a representation of the display state. Currently references bd::Outputs::Wlr::OutputManager (intentionally named this as it implements zwlr_output_manager_v1)
    d. BatchSystem renamed to bd::Outputs::Config::Model. Related to this is bd::Outputs::Config::Result/Action, enums.
  3. DBus interfaces renamed. Redundancies in naming removed.

There is still work to be done there on having meta-objects be generalized and expanded on so we can support more protocols (where in the case of Wlr, they'd be no-op, but when using better protocols they wouldn't e.g. for color profiles), but the intent was to better communicate "this is all wlroots-related" going forwards. Side-benefit is redundant naming of "Wayland[...]" is cleaned up. The prefix was redundant.

Other changes:

  • Tweaked usage of enums since we were relying a fair bit on copying the enum value into other QObjects. I also cleaned up our utils for converting to / from the enum values into toString / fromString in the classes themselves.
  • Updated Taskfile to specify prefix of /usr. All of our autostarts everywhere else point to /usr/bin/[...] and I don't want the pain during development of forgetting something getting installed to /usr/local instead and then wasting time wondering why XYZ isn't working.

Other notes:

  • There is some redundancy on namespace specification when using some classes, for example Wlr::MetaMode specified in Wlr::MetaHead. This will get cleaned up when MetaHead and MetaMode are generalized and moved out of Wlr::
The goal of this is to: 1. Attempt to standardize on some file casing based off what we have done with Display Configurator. I'll get to the dbus part of the codebase in a follow-up PR since I want to move system-specific DBus Services under those systems rather than them being top-level. 2. Introduce clearer namespacing and classes. Not exhaustive, but some examples. a. bd::Outputs - Contains output-related classes (excluding DBus and display-config TOML parsing, for now) b. bd::Outputs::Wlr - Wlroots-specific has been moved under this namespace. c. WaylandOrchestrator renamed to bd::Outputs::State to reflect it is intended to be a representation of the display state. Currently references bd::Outputs::Wlr::OutputManager (intentionally named this as it implements zwlr_output_manager_v1) d. BatchSystem renamed to bd::Outputs::Config::Model. Related to this is bd::Outputs::Config::Result/Action, enums. 3. DBus interfaces renamed. Redundancies in naming removed. There is still work to be done there on having meta-objects be generalized and expanded on so we can support more protocols (where in the case of Wlr, they'd be no-op, but when using better protocols they wouldn't e.g. for color profiles), but the intent was to better communicate "this is all wlroots-related" going forwards. Side-benefit is redundant naming of "Wayland[...]" is cleaned up. The prefix was redundant. Other changes: - Tweaked usage of enums since we were relying a fair bit on copying the enum value into other QObjects. I also cleaned up our utils for converting to / from the enum values into toString / fromString in the classes themselves. - Updated Taskfile to specify prefix of `/usr`. All of our autostarts everywhere else point to `/usr/bin/[...]` and I don't want the pain during development of forgetting something getting installed to `/usr/local` instead and then wasting time wondering why XYZ isn't working. Other notes: - There is some redundancy on namespace specification when using some classes, for example Wlr::MetaMode specified in Wlr::MetaHead. This will get cleaned up when MetaHead and MetaMode are generalized and moved out of Wlr::
The goal of this is to:

1. Attempt to standardize on some file casing based off what we have done with Display Configurator. I'll get to the dbus part of the codebase in a follow-up PR since I want to move system-specific DBus Services under those systems rather than them being top-level.
2. Introduce clearer namespacing, including moving wlroots-specific code into bd::OutputManager::Wlr namespace. There is still work to be done there on having meta-objects be generalized and expanded on so we can support more protocols, but the intent was to better communicate "this is all wlroots-related" going forwards. Side-benefit is redundant naming of "Wayland[...]" is cleaned up. The prefix was redundant.

Other changes:

1. Tweaked usage of enums since we were relying a fair bit on copying the enum value into other QObjects. I also cleaned up our utils for converting to / from the enum values into toString / fromString in the classes themselves.
2. Updated DBus interfaces & paths from BudgieDaemon to Services, communicate anchors in strings instead of the ints to keep it consistent with how we use it in the configs
EbonJaeger left a comment
Owner

I think that while we're at it, we can simplify a bunch of the naming further. Instead of BatchSystem, maybe something more descriptive like BatchConfig, or less verbose like just Batch. BatchSystem::OutputBatchSystem, for instance, could just be BatchConfig::Output, or Batch::OutputConfig.

I think that while we're at it, we can simplify a bunch of the naming further. Instead of `BatchSystem`, maybe something more descriptive like `BatchConfig`, or less verbose like just `Batch`. `BatchSystem::OutputBatchSystem`, for instance, could just be `BatchConfig::Output`, or `Batch::OutputConfig`.
@ -0,0 +32,4 @@
static QSharedPointer<ConfigurationAction> primary(const QString& serial, QObject *parent = nullptr);
~ConfigurationAction() override;
Owner

In the .cpp file, we set this to default. We can do that here in the header, instead.

In the `.cpp` file, we set this to default. We can do that here in the header, instead.
@ -0,0 +5,4 @@
#include <QString>
namespace bd::BatchSystem {
class ConfigurationActionType : public QObject {
Owner

This can probably just be called Action. I would argue that the configuration part is implied because it's in the batch-system/

This can probably just be called `Action`. I would argue that the configuration part is implied because it's in the `batch-system`/
@ -0,0 +6,4 @@
#include <string>
namespace bd::BatchSystem {
class ConfigurationHorizontalAnchor : public QObject {
Owner

This can be HorizontalAnchor.

This can be `HorizontalAnchor`.
JoshStrobl marked this conversation as resolved
@ -0,0 +11,4 @@
public:
enum Type {
NoHorizontalAnchor, // No horizontal anchor set
Owner

Can we call this None?

Can we call this `None`?
JoshStrobl marked this conversation as resolved
@ -0,0 +62,4 @@
}
};
class ConfigurationVerticalAnchor : public QObject {
Owner

This can be VerticalAnchor.

This can be `VerticalAnchor`.
JoshStrobl marked this conversation as resolved
@ -0,0 +67,4 @@
public:
enum Type {
NoVerticalAnchor, // No vertical anchor set
Owner

Same here about None.

Same here about `None`.
JoshStrobl marked this conversation as resolved
@ -0,0 +12,4 @@
public:
Result(QObject *parent = nullptr);
~Result() override;
Owner

This is set to default in the .cpp file; we can do that here instead.

This is set to default in the `.cpp` file; we can do that here instead.
JoshStrobl marked this conversation as resolved
@ -0,0 +15,4 @@
public:
TargetState(QString serial, QObject *parent = nullptr);
~TargetState() override;
Owner

This is set to default in the .cpp file; we can do it here, instead.

This is set to default in the `.cpp` file; we can do it here, instead.
JoshStrobl marked this conversation as resolved
@ -0,0 +6,4 @@
Head::Head(QObject* parent, ::zwlr_output_head_v1* wlr_head)
: QObject(parent), zwlr_output_head_v1(wlr_head), m_wlr_head(wlr_head) {}
::zwlr_output_head_v1* Head::getWlrHead() {
Owner

Is there something missing here?

Is there something missing here?
@ -0,0 +18,4 @@
Q_OBJECT
public:
MetaHead(QObject *parent, KWayland::Client::Registry *registry);
Owner

This should probably be

MetaHead(KWayland::Client::Registry *registry, QObject *parent = nullptr);
This should probably be ```cpp MetaHead(KWayland::Client::Registry *registry, QObject *parent = nullptr); ```
JoshStrobl marked this conversation as resolved
Author
Owner

Code

Good actionable items on destructors and further cleaning up the naming. Also a fan of QObject refs being at end, so will do a pass on that including the MetaHead you highlighted. The struct itself on head is correct though, as it's returning the underlying zwlr_output_head_v1 as opposed to the QObject...version? of it. At least that's been my understanding of it, I can double check the generated files.

Naming

Yea the naming isn't amazing and should certainly be improved. What are your thoughts on:

  • Classes (within bd::Outputs namespace)
    • Orchestrator - This would be the current output / output state manager (currently this is bd::WaylandOrchestrator). The end goal of is to effectively determine the current running compositor, perform a roundtrip to acquire any heads, modes, etc. for that given compositor and shove them into their compositor-specific class (that implements a common interface, which some functions being no-ops), meta objects, etc. and handling lifecycle changes. It also is what is responsible for the actual calls to the compositor like setting modes, disabling / enabling heads, so forth.
      • Some related classes still remain like WaylandOutputConfigurationHead but those would be renamed to just ConfigurationHead. Eventually moved into protocol-specific subsection like bd::Outputs::Wlr. Baby steps.
    • Manager - Or alternative Config? Or something 🤷- This is the "batch system" responsible for queuing up and calculating the result, which is then used for calls (applying the configuration) to the Orchestrator before then dumping the new state into the config.
    • Head - This is our current "meta head" that holds information when the compositor or respective protocol cannot be entrusted to retain specific references.
    • Mode - This is our current "meta mode". Same deal as above.
  • Classes (within bd::Outputs::Wlr wlroots-specific namespace)
    • Head - Our existing non-"meta" head.
      • Eventually implements either a class w/ overrides (helps to have some functional defaults for when a protocol is missing support for features like color profiles, HDR, so forth).
    • Mode - Our existing non-"mode" head. Same deal as above.

I think just calling it Batch doesn't communicate what it necessarily does. Having both an Orchestrator and a Manager could be confusing, so I imagine Config / Configuration / Configurator would be better at communicating it.

**Code** Good actionable items on destructors and further cleaning up the naming. Also a fan of QObject refs being at end, so will do a pass on that including the MetaHead you highlighted. The struct itself on head is correct though, as it's returning the underlying `zwlr_output_head_v1` as opposed to the QObject...version? of it. At least that's been my understanding of it, I can double check the generated files. **Naming** Yea the naming isn't amazing and should certainly be improved. What are your thoughts on: - Classes (within `bd::Outputs` namespace) - `Orchestrator` - This would be the current output / output state manager (currently this is `bd::WaylandOrchestrator`). The end goal of is to effectively determine the current running compositor, perform a roundtrip to acquire any heads, modes, etc. for that given compositor and shove them into their compositor-specific class (that implements a common interface, which some functions being no-ops), meta objects, etc. and handling lifecycle changes. It also is what is responsible for the actual calls to the compositor like setting modes, disabling / enabling heads, so forth. - Some related classes still remain like `WaylandOutputConfigurationHead` but those would be renamed to just `ConfigurationHead`. Eventually moved into protocol-specific subsection like `bd::Outputs::Wlr`. Baby steps. - `Manager` - Or alternative Config? Or something 🤷- This is the "batch system" responsible for queuing up and calculating the result, which is then used for calls (applying the configuration) to the Orchestrator before then dumping the new state into the config. - `Head` - This is our current "meta head" that holds information when the compositor or respective protocol cannot be entrusted to retain specific references. - `Mode` - This is our current "meta mode". Same deal as above. - Classes (within `bd::Outputs::Wlr` wlroots-specific namespace) - `Head` - Our existing non-"meta" head. - Eventually implements either a class w/ overrides (helps to have some functional defaults for when a protocol is missing support for features like color profiles, HDR, so forth). - `Mode` - Our existing non-"mode" head. Same deal as above. I think _just_ calling it `Batch` doesn't communicate what it necessarily does. Having both an Orchestrator and a Manager could be confusing, so I imagine `Config` / `Configuration` / `Configurator` would be better at communicating it.
Owner

I think the Head and Mode namings are fine. It sounds to me that Orchestrator really is more of a Manager, and Manager is... Also sort of a manager? Hmm. I'm not quite sure what to suggest for that.

I need to think more about the batch system as a whole, too.

I think the `Head` and `Mode` namings are fine. It sounds to me that `Orchestrator` really is more of a `Manager`, and `Manager` is... Also sort of a manager? Hmm. I'm not quite sure what to suggest for that. I need to think more about the batch system as a whole, too.
Author
Owner

From weekly standup actions:

  1. Rename OutputManager to just Outputs
  2. Rename Orchestrator to State
  3. Rename batch system to ConfigModel, its dbus interface to Config
From weekly standup actions: 1. Rename OutputManager to just Outputs 2. Rename Orchestrator to State 3. Rename batch system to ConfigModel, its dbus interface to Config
EbonJaeger left a comment
Owner

The only thing I'd suggest is removing the directories in outputs/wlr, and putting the files in outputs/wlr itself. Having a bunch of directories that only contain one header and one source file, I feel, doesn't make a lot of sense in this sort of instance.

The only thing I'd suggest is removing the directories in `outputs/wlr`, and putting the files in `outputs/wlr` itself. Having a bunch of directories that only contain one header and one source file, I feel, doesn't make a lot of sense in this sort of instance.
JoshStrobl deleted branch code-reorg 2025-12-15 14:54:20 +01:00
Sign in to join this conversation.
No description provided.