Use [Channel1_Stem1] pattern for Stem COs. Improve speed a bit. #14244
Conversation
|
I've added a request on # 14240 regarding |
This should be updated as well IMO |
This is an entirely different topic IMHO. We would change the legacy naming names since Mixxx 2.0. Let's please please not conflate these topics and discuss this separately. |
src/engine/channels/enginedeck.cpp
Outdated
| QString EngineDeck::getGroupForStem(const QString& deckGroup, int stemIdx) { | ||
| DEBUG_ASSERT(deckGroup.endsWith("]") && stemIdx < 4); | ||
| QString stemGroup = | ||
| QStringView(deckGroup.constData(), deckGroup.size() - 1) + | ||
| QStringLiteral("_Stem0]"); | ||
| stemGroup[stemGroup.size() - 2] = QChar('1' + static_cast<char>(stemIdx)); | ||
| return stemGroup; | ||
| return QStringView(deckGroup.constData(), deckGroup.size() - 1) + |
There was a problem hiding this comment.
last nitpick:
QString EngineDeck::getGroupForStem(QStringView deckGroup, int stemIdx) {
DEBUG_ASSERT(stemIdx < 4);
DEBUG_ASSERT(deckGroup.endsWith(QChar(']')));
return deckgroup.chopped(1) +There was a problem hiding this comment.
Unfortunatly this comes witha a runtime overhead. Unlike QStringView(deckGroup.constData(), deckGroup.size() - 1), deckgroup.chopped(1) retuns a new allocated QString. It basically boils down to QString(deckGroup.constData(), deckGroup.size() - 1).
Just learned that that since 6.8 there is a QString chopped() && that will not introduce a new string when used on an rval
std::move(deckGroup).chopped(1)
but we have only a const QString&
There was a problem hiding this comment.
deckgroup.chopped(1) retuns a new allocated QString
Yes, but since my proposal also makes deckGroup a QStringView, the concern doesn't apply. If we know we don't need to store the string we should generally start taking QStringViews instead of const QString&s IMO. This is a perfect example of a "leaf" function where we know no inner function needs a proper QString so this is the place to start such as refactoring.
|
Committed suggestion and squashed history. Thank you for review. |
Fixes #14240 for a consistent rule how indexed groups are build.