Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove offensive language (Pt. 3) #11960

Draft
wants to merge 10 commits into
base: 2.5
Choose a base branch
from
Draft

Remove offensive language (Pt. 3) #11960

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d2f2eb8
refactor: Rename `[Master]` to `[Main]` and add group alias
Holzhaus Sep 10, 2023
0e97149
refactor: Add group alias support for effect chains/slots
Holzhaus Sep 10, 2023
8a48f12
refactor: Use `[Main]` group in entire C++ code
Holzhaus Sep 10, 2023
c68577a
chore(keyboard): Use `[Main]` group in keyboard mappings
Holzhaus Sep 10, 2023
09dbc8f
chore(qml): Use `[Main]` group in QML skin
Holzhaus Sep 10, 2023
40bdb8b
refactor(skins): Use `[Main]` group in all skins
Holzhaus Sep 10, 2023
5ad20c2
refactor(tools): Use `[Main]` group in `make_xone.py` script
Holzhaus Sep 10, 2023
87763ce
refactor(controllers): Use `[Main]` group in built-in mappings
Holzhaus Sep 10, 2023
7722a5a
refactor: Rename `[MasterOutput]` to `[MainOutput]` and add group alias
Holzhaus Sep 10, 2023
beb9ba1
refactor(skins): Use `[MainOutput]` group in all skins
Holzhaus Sep 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 45 additions & 1 deletion src/control/control.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ QHash<ConfigKey, QWeakPointer<ControlDoublePrivate>> s_qCOHash
QHash<ConfigKey, ConfigKey> s_qCOAliasHash
GUARDED_BY(s_qCOHashMutex);

/// Hash of aliases between group names. Solely used for looking up the first
/// alias associated with a name when adding a new control.
QHash<QString, QString> s_qCOGroupAliasHash
GUARDED_BY(s_qCOHashMutex);

/// is used instead of a nullptr, helps to omit null checks everywhere
QWeakPointer<ControlDoublePrivate> s_pDefaultCO;
} // namespace
Expand Down Expand Up @@ -125,6 +130,32 @@ void ControlDoublePrivate::insertAlias(const ConfigKey& alias, const ConfigKey&
s_qCOHash.insert(alias, pControl);
}

// static
void ControlDoublePrivate::insertGroupAlias(
const QString& aliasGroup, const QString& originalGroup) {
MMutexLocker locker(&s_qCOHashMutex);

// Check if there are any pre-existing control with the group that we want
// to add an alias for and control aliases for them.
for (auto it = s_qCOHash.cbegin(), end = s_qCOHash.cend(); it != end; ++it) {
const auto originalKey = it.key();
if (originalKey.group != originalGroup) {
continue;
}

QSharedPointer<ControlDoublePrivate> pControl = it.value();
if (pControl.isNull()) {
continue;
}

const auto aliasKey = ConfigKey(aliasGroup, originalKey.item);
s_qCOAliasHash.insert(originalKey, aliasKey);
s_qCOHash.insert(aliasKey, pControl);
Comment on lines +152 to +153
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this get expensive if the group is large? would it be better to do the aliasing for the group on lookup instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That way, as part of the lookup we could also print a warning if the alias was used instead of the non-aliased group.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this get expensive if the group is large?

You mean memory-wise? It adds a regular alias for every member in a group, but it shouldn't be that much of an issue. In terms of runtime performance, it shouldn't be an issue because lookups are only performed when a new CO is created and it's O(1).

would it be better to do the aliasing for the group on lookup instead? That way, as part of the lookup we could also print a warning if the alias was used instead of the non-aliased group.

True, this could be worth considering. I'll have a look.

Copy link
Member

@Swiftb0y Swiftb0y Sep 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean memory-wise? It adds a regular alias for every member in a group, but it shouldn't be that much of an issue.

Right, if the group is large, this could bloat the table quite a bit. Yes, lookups are $O(1)$ but creating a group alias and creating COs becomes $O(n)$ (where $n$ is size of the group in the former case and the number of alias groups in the latter). The increased table size would cause the usual slowdowns associated with high memory consumption (data cache misses, more yields to the OS for more memory, more table relocations since it grows faster / more often, etc).

Doing the aliasing lookup would avoid that and only be worse performing on the unhappy path (probing via alias group, if CO exists return (happy path) otherwise lookup primary group name, probe again using primary group name, print warning, etc).

}

s_qCOGroupAliasHash.insert(originalGroup, aliasGroup);
}

// static
QSharedPointer<ControlDoublePrivate> ControlDoublePrivate::getControl(
const ConfigKey& key,
Expand Down Expand Up @@ -175,9 +206,22 @@ QSharedPointer<ControlDoublePrivate> ControlDoublePrivate::getControl(
bTrack,
bPersist,
defaultValue));
const MMutexLocker locker(&s_qCOHashMutex);
s_qCOHashMutex.lock();
//qDebug() << "ControlDoublePrivate::s_qCOHash.insert(" << key.group << "," << key.item << ")";
s_qCOHash.insert(key, pControl);

// Check if the control's group is aliased. If so, add a corresponding
// alias for this control.
auto it = s_qCOGroupAliasHash.constFind(key.group);
if (it != s_qCOGroupAliasHash.constEnd()) {
QString aliasGroup = it.value();
auto aliasKey = ConfigKey(aliasGroup, key.item);
s_qCOHashMutex.unlock();
insertAlias(aliasKey, key);
} else {
s_qCOHashMutex.unlock();
}
Comment on lines +219 to +223
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer if we used a locker and just manually unlocked just before.


return pControl;
}

Expand Down
7 changes: 7 additions & 0 deletions src/control/control.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ class ControlDoublePrivate : public QObject {
// using this UserSettings.
static void setUserConfig(const UserSettingsPointer& pConfig);

/// Adds a group alias for the control group `originalGroup`. Can be used
/// for supporting legacy / deprecated controls. Aliases for existing
/// controls with group `originalGroup` are added immediately. For new
/// controls, an alias for new controls with group `originalGroup` is added
/// when they are created.
static void insertGroupAlias(const QString& aliasGroup, const QString& originalGroup);

// Adds a ConfigKey for 'alias' to the control for 'key'. Can be used for
// supporting a legacy / deprecated control. The 'key' control must exist
// for this to work.
Expand Down
3 changes: 2 additions & 1 deletion src/coreservices.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,10 @@ void CoreServices::initialize(QApplication* pApp) {
emit initializationProgressUpdate(20, tr("effects"));
m_pEffectsManager = std::make_shared<EffectsManager>(pConfig, pChannelHandleFactory);

ControlDoublePrivate::insertGroupAlias(QStringLiteral("[Master]"), QStringLiteral("[Main]"));
m_pEngine = std::make_shared<EngineMixer>(
pConfig,
"[Master]",
"[Main]",
m_pEffectsManager.get(),
pChannelHandleFactory,
true);
Expand Down