feat: reorganization of batch system and output manager code #18
No reviewers
Labels
No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
BuddiesOfBudgie/budgie-desktop-services!18
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "code-reorg"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The goal of this is to:
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.
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:
/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/localinstead and then wasting time wondering why XYZ isn't working.Other notes:
I think that while we're at it, we can simplify a bunch of the naming further. Instead of
BatchSystem, maybe something more descriptive likeBatchConfig, or less verbose like justBatch.BatchSystem::OutputBatchSystem, for instance, could just beBatchConfig::Output, orBatch::OutputConfig.@ -0,0 +32,4 @@static QSharedPointer<ConfigurationAction> primary(const QString& serial, QObject *parent = nullptr);~ConfigurationAction() override;In the
.cppfile, 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 {This can probably just be called
Action. I would argue that the configuration part is implied because it's in thebatch-system/@ -0,0 +6,4 @@#include <string>namespace bd::BatchSystem {class ConfigurationHorizontalAnchor : public QObject {This can be
HorizontalAnchor.@ -0,0 +11,4 @@public:enum Type {NoHorizontalAnchor, // No horizontal anchor setCan we call this
None?@ -0,0 +62,4 @@}};class ConfigurationVerticalAnchor : public QObject {This can be
VerticalAnchor.@ -0,0 +67,4 @@public:enum Type {NoVerticalAnchor, // No vertical anchor setSame here about
None.@ -0,0 +12,4 @@public:Result(QObject *parent = nullptr);~Result() override;This is set to default in the
.cppfile; we can do that here instead.@ -0,0 +15,4 @@public:TargetState(QString serial, QObject *parent = nullptr);~TargetState() override;This is set to default in the
.cppfile; we can do it here, instead.@ -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() {Is there something missing here?
@ -0,0 +18,4 @@Q_OBJECTpublic:MetaHead(QObject *parent, KWayland::Client::Registry *registry);This should probably be
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_v1as 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:
bd::Outputsnamespace)Orchestrator- This would be the current output / output state manager (currently this isbd::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.WaylandOutputConfigurationHeadbut those would be renamed to justConfigurationHead. Eventually moved into protocol-specific subsection likebd::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.bd::Outputs::Wlrwlroots-specific namespace)Head- Our existing non-"meta" head.Mode- Our existing non-"mode" head. Same deal as above.I think just calling it
Batchdoesn't communicate what it necessarily does. Having both an Orchestrator and a Manager could be confusing, so I imagineConfig/Configuration/Configuratorwould be better at communicating it.I think the
HeadandModenamings are fine. It sounds to me thatOrchestratorreally is more of aManager, andManageris... 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.
From weekly standup actions:
The only thing I'd suggest is removing the directories in
outputs/wlr, and putting the files inoutputs/wlritself. 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.