From e9cfe535cb08b1b1d046338ccab653588bcb73cd Mon Sep 17 00:00:00 2001 From: Calum Matheson Date: Mon, 13 Jan 2025 19:47:16 +0000 Subject: [PATCH] Percussion shadow note popup - improve stability of popup --- src/engraving/dom/engravingobject.h | 3 ++ src/notation/inotationinteraction.h | 1 + src/notation/internal/notationinteraction.cpp | 11 +++++ src/notation/internal/notationinteraction.h | 2 + .../internal/PercussionNotePopupContent.qml | 4 ++ .../tests/mocks/notationinteractionmock.h | 1 + .../view/abstractelementpopupmodel.cpp | 5 ++- src/notation/view/abstractelementpopupmodel.h | 6 +-- .../percussionnotepopupcontentmodel.cpp | 15 +++++-- .../percussionnotepopupcontentmodel.h | 6 ++- .../view/internal/shadownotepopupmodel.cpp | 42 ++++++++++++++++++- .../view/internal/shadownotepopupmodel.h | 6 +++ .../view/notationviewinputcontroller.cpp | 11 ++--- .../view/notationviewinputcontroller.h | 1 + 14 files changed, 97 insertions(+), 17 deletions(-) diff --git a/src/engraving/dom/engravingobject.h b/src/engraving/dom/engravingobject.h index b9d005be50753..968fadb869cc2 100644 --- a/src/engraving/dom/engravingobject.h +++ b/src/engraving/dom/engravingobject.h @@ -187,6 +187,7 @@ class VoltaSegment; class WhammyBar; class WhammyBarSegment; class FretCircle; +class ShadowNote; class LinkedObjects; @@ -452,6 +453,7 @@ class EngravingObject CONVERT(FretCircle, FRET_CIRCLE) CONVERT(StringTunings, STRING_TUNINGS) CONVERT(TimeTickAnchor, TIME_TICK_ANCHOR) + CONVERT(ShadowNote, SHADOW_NOTE) #undef CONVERT virtual bool isEngravingItem() const { return false; } // overridden in element.h @@ -857,5 +859,6 @@ CONVERT(SoundFlag) CONVERT(TimeTickAnchor) CONVERT(LaissezVib) CONVERT(PartialTie) +CONVERT(ShadowNote) #undef CONVERT } diff --git a/src/notation/inotationinteraction.h b/src/notation/inotationinteraction.h index 310d6b7768381..290cc5e4be188 100644 --- a/src/notation/inotationinteraction.h +++ b/src/notation/inotationinteraction.h @@ -48,6 +48,7 @@ class INotationInteraction virtual bool showShadowNote(const muse::PointF& pos) = 0; virtual void hideShadowNote() = 0; virtual muse::RectF shadowNoteRect() const = 0; + virtual muse::async::Notification shadowNoteChanged() const = 0; // Visibility virtual void toggleVisible() = 0; diff --git a/src/notation/internal/notationinteraction.cpp b/src/notation/internal/notationinteraction.cpp index e73fddf534f49..46440aafe5c78 100644 --- a/src/notation/internal/notationinteraction.cpp +++ b/src/notation/internal/notationinteraction.cpp @@ -366,6 +366,8 @@ bool NotationInteraction::showShadowNote(const PointF& pos) mu::engraving::Position position; if (!score()->getPosition(&position, pos, inputState.voice())) { shadowNote.setVisible(false); + shadowNote.setDrumNotePitch(-1); + m_shadowNoteChanged.notify(); return false; } @@ -419,9 +421,12 @@ bool NotationInteraction::showShadowNote(const PointF& pos) } while (pitch != startPitch); if (!drumNoteFound) { shadowNote.setVisible(false); + shadowNote.setDrumNotePitch(-1); + m_shadowNoteChanged.notify(); return false; } } else { + shadowNote.setDrumNotePitch(-1); voice = inputState.voice(); } @@ -455,6 +460,7 @@ bool NotationInteraction::showShadowNote(const PointF& pos) score()->renderer()->layoutItem(&shadowNote); shadowNote.setPos(position.pos); + m_shadowNoteChanged.notify(); return true; } @@ -485,6 +491,11 @@ RectF NotationInteraction::shadowNoteRect() const return rect; } +muse::async::Notification NotationInteraction::shadowNoteChanged() const +{ + return m_shadowNoteChanged; +} + void NotationInteraction::toggleVisible() { startEdit(TranslatableString("undoableAction", "Toggle visible")); diff --git a/src/notation/internal/notationinteraction.h b/src/notation/internal/notationinteraction.h index 16c6b9a78b257..dbb48e77d537a 100644 --- a/src/notation/internal/notationinteraction.h +++ b/src/notation/internal/notationinteraction.h @@ -68,6 +68,7 @@ class NotationInteraction : public INotationInteraction, public muse::Injectable bool showShadowNote(const muse::PointF& pos) override; void hideShadowNote() override; muse::RectF shadowNoteRect() const override; + muse::async::Notification shadowNoteChanged() const override; // Visibility void toggleVisible() override; @@ -440,6 +441,7 @@ class NotationInteraction : public INotationInteraction, public muse::Injectable std::shared_ptr m_selection = nullptr; muse::async::Notification m_selectionChanged; + muse::async::Notification m_shadowNoteChanged; DragData m_dragData; muse::async::Notification m_dragChanged; diff --git a/src/notation/qml/MuseScore/NotationScene/internal/PercussionNotePopupContent.qml b/src/notation/qml/MuseScore/NotationScene/internal/PercussionNotePopupContent.qml index 08cb811440e0e..78c50ee55ccec 100644 --- a/src/notation/qml/MuseScore/NotationScene/internal/PercussionNotePopupContent.qml +++ b/src/notation/qml/MuseScore/NotationScene/internal/PercussionNotePopupContent.qml @@ -34,6 +34,10 @@ Row { PercussionNotePopupContentModel { id: contentModel + + Component.onCompleted: { + contentModel.init() + } } Row { diff --git a/src/notation/tests/mocks/notationinteractionmock.h b/src/notation/tests/mocks/notationinteractionmock.h index 6a74cf606fc82..2321944df73f3 100644 --- a/src/notation/tests/mocks/notationinteractionmock.h +++ b/src/notation/tests/mocks/notationinteractionmock.h @@ -36,6 +36,7 @@ class NotationInteractionMock : public INotationInteraction MOCK_METHOD(bool, showShadowNote, (const muse::PointF&), (override)); MOCK_METHOD(void, hideShadowNote, (), (override)); MOCK_METHOD(muse::RectF, shadowNoteRect, (), (const, override)); + MOCK_METHOD(muse::async::Notification, shadowNoteChanged, (), (const, override)); MOCK_METHOD(void, toggleVisible, (), (override)); diff --git a/src/notation/view/abstractelementpopupmodel.cpp b/src/notation/view/abstractelementpopupmodel.cpp index a76a858a72095..36936d3f86ae7 100644 --- a/src/notation/view/abstractelementpopupmodel.cpp +++ b/src/notation/view/abstractelementpopupmodel.cpp @@ -22,6 +22,7 @@ #include "abstractelementpopupmodel.h" #include "internal/partialtiepopupmodel.h" +#include "internal/shadownotepopupmodel.h" #include "log.h" using namespace mu::notation; @@ -75,6 +76,8 @@ bool AbstractElementPopupModel::supportsPopup(const EngravingItem* element) switch (modelType) { case PopupModelType::TYPE_PARTIAL_TIE: return PartialTiePopupModel::canOpen(element); + case PopupModelType::TYPE_SHADOW_NOTE: + return ShadowNotePopupModel::canOpen(element); default: return true; } @@ -233,7 +236,7 @@ const mu::engraving::ElementTypeSet& AbstractElementPopupModel::dependentElement void AbstractElementPopupModel::updateItemRect() { - QRect rect = m_item ? fromLogical(m_item->canvasBoundingRect()).toQRect() : QRect(); + const QRect rect = m_item ? fromLogical(m_item->canvasBoundingRect()).toQRect() : QRect(); if (m_itemRect != rect) { m_itemRect = rect; diff --git a/src/notation/view/abstractelementpopupmodel.h b/src/notation/view/abstractelementpopupmodel.h index c569db86877a5..83e5f408e5a6b 100644 --- a/src/notation/view/abstractelementpopupmodel.h +++ b/src/notation/view/abstractelementpopupmodel.h @@ -71,6 +71,8 @@ class AbstractElementPopupModel : public QObject, public muse::Injectable, publi void itemRectChanged(QRect rect); protected: + virtual void updateItemRect(); + muse::PointF fromLogical(muse::PointF point) const; muse::RectF fromLogical(muse::RectF rect) const; @@ -87,6 +89,7 @@ class AbstractElementPopupModel : public QObject, public muse::Injectable, publi void changeItemProperty(mu::engraving::Pid id, const PropertyValue& value, engraving::PropertyFlags flags); EngravingItem* m_item = nullptr; + QRect m_itemRect; private: INotationSelectionPtr selection() const; @@ -94,10 +97,7 @@ class AbstractElementPopupModel : public QObject, public muse::Injectable, publi engraving::ElementType elementType() const; const engraving::ElementTypeSet& dependentElementTypes() const; - void updateItemRect(); - PopupModelType m_modelType = PopupModelType::TYPE_UNDEFINED; - QRect m_itemRect; }; using PopupModelType = AbstractElementPopupModel::PopupModelType; diff --git a/src/notation/view/internal/percussionnotepopupcontentmodel.cpp b/src/notation/view/internal/percussionnotepopupcontentmodel.cpp index 95884b7bb9bee..4fd8cd8ea359f 100644 --- a/src/notation/view/internal/percussionnotepopupcontentmodel.cpp +++ b/src/notation/view/internal/percussionnotepopupcontentmodel.cpp @@ -32,6 +32,15 @@ PercussionNotePopupContentModel::PercussionNotePopupContentModel(QObject* parent { } +void PercussionNotePopupContentModel::init() +{ + interaction()->shadowNoteChanged().onNotify(this, [this]() { + emit shouldShowButtonsChanged(); + emit percussionNoteNameChanged(); + emit keyboardShortcutChanged(); + }); +} + void PercussionNotePopupContentModel::prevDrumNote() { const Drumset* ds = currentDrumset(); @@ -98,7 +107,7 @@ bool PercussionNotePopupContentModel::shouldShowButtons() const { const Drumset* ds = currentDrumset(); const mu::engraving::ShadowNote* shadowNote = currentShadowNote(); - if (!ds || !shadowNote) { + if (!ds || !shadowNote || !shadowNote->visible() || shadowNote->drumNotePitch() < 0) { return false; } @@ -121,7 +130,7 @@ QString PercussionNotePopupContentModel::percussionNoteName() const { const Drumset* ds = currentDrumset(); const mu::engraving::ShadowNote* shadowNote = currentShadowNote(); - if (!ds || !shadowNote) { + if (!ds || !shadowNote || !shadowNote->visible() || shadowNote->drumNotePitch() < 0) { return QString(); } @@ -132,7 +141,7 @@ QString PercussionNotePopupContentModel::keyboardShortcut() const { const Drumset* ds = currentDrumset(); const mu::engraving::ShadowNote* shadowNote = currentShadowNote(); - if (!ds || !shadowNote) { + if (!ds || !shadowNote || !shadowNote->visible() || shadowNote->drumNotePitch() < 0) { return QString(); } diff --git a/src/notation/view/internal/percussionnotepopupcontentmodel.h b/src/notation/view/internal/percussionnotepopupcontentmodel.h index adb882e70c047..b2d86beac49b6 100644 --- a/src/notation/view/internal/percussionnotepopupcontentmodel.h +++ b/src/notation/view/internal/percussionnotepopupcontentmodel.h @@ -24,11 +24,13 @@ #include +#include "async/asyncable.h" + #include "modularity/ioc.h" #include "context/iglobalcontext.h" namespace mu::notation { -class PercussionNotePopupContentModel : public QObject, public muse::Injectable +class PercussionNotePopupContentModel : public QObject, public muse::Injectable, public muse::async::Asyncable { Q_OBJECT @@ -41,6 +43,8 @@ class PercussionNotePopupContentModel : public QObject, public muse::Injectable public: explicit PercussionNotePopupContentModel(QObject* parent = nullptr); + Q_INVOKABLE void init(); + Q_INVOKABLE void prevDrumNote(); Q_INVOKABLE void nextDrumNote(); diff --git a/src/notation/view/internal/shadownotepopupmodel.cpp b/src/notation/view/internal/shadownotepopupmodel.cpp index c3dfa97b1f2ee..359188ade7af8 100644 --- a/src/notation/view/internal/shadownotepopupmodel.cpp +++ b/src/notation/view/internal/shadownotepopupmodel.cpp @@ -30,16 +30,54 @@ ShadowNotePopupModel::ShadowNotePopupModel(QObject* parent) { } +bool ShadowNotePopupModel::canOpen(const EngravingItem* element) +{ + if (!element || !element->isShadowNote() || !element->visible()) { + return false; + } + + const mu::engraving::ShadowNote* shadowNote = toShadowNote(element); + if (shadowNote->drumNotePitch() > -1) { + // Can open with PERCUSSON_CONTENT type + return true; + } + + return false; +} + void ShadowNotePopupModel::init() { AbstractElementPopupModel::init(); m_item = interaction()->shadowNote(); + + interaction()->shadowNoteChanged().onNotify(this, [this]() { + updateItemRect(); + }); + emit currentPopupTypeChanged(); } ShadowNotePopupContent::ContentType ShadowNotePopupModel::currentPopupType() const { - // TODO: evaluate the current content type based on the context - return ShadowNotePopupContent::ContentType::PERCUSSION_CONTENT; + const mu::engraving::ShadowNote* shadowNote = interaction()->shadowNote(); + if (shadowNote && shadowNote->visible() && shadowNote->drumNotePitch() > -1) { + return ShadowNotePopupContent::ContentType::PERCUSSION_CONTENT; + } + return ShadowNotePopupContent::ContentType::NONE; +} + +void ShadowNotePopupModel::updateItemRect() +{ + const mu::engraving::ShadowNote* shadowNote = interaction()->shadowNote(); + if (!shadowNote || !shadowNote->visible()) { + m_itemRect = QRect(); + emit itemRectChanged(m_itemRect); + } + + muse::RectF noteHeadRect = shadowNote->symBbox(shadowNote->noteheadSymbol()); + noteHeadRect.translate(shadowNote->canvasPos().x(), shadowNote->canvasPos().y()); + + m_itemRect = fromLogical(noteHeadRect).toQRect(); + emit itemRectChanged(m_itemRect); } diff --git a/src/notation/view/internal/shadownotepopupmodel.h b/src/notation/view/internal/shadownotepopupmodel.h index 1709e777ae0a5..8e0e43596ab83 100644 --- a/src/notation/view/internal/shadownotepopupmodel.h +++ b/src/notation/view/internal/shadownotepopupmodel.h @@ -47,11 +47,17 @@ class ShadowNotePopupModel : public AbstractElementPopupModel public: explicit ShadowNotePopupModel(QObject* parent = nullptr); + + static bool canOpen(const EngravingItem* shadowNote); + Q_INVOKABLE void init(); ShadowNotePopupContent::ContentType currentPopupType() const; signals: void currentPopupTypeChanged(); + +protected: + void updateItemRect() override; }; } // namespace mu::notation diff --git a/src/notation/view/notationviewinputcontroller.cpp b/src/notation/view/notationviewinputcontroller.cpp index fc03f891cb86b..45392aef67b72 100644 --- a/src/notation/view/notationviewinputcontroller.cpp +++ b/src/notation/view/notationviewinputcontroller.cpp @@ -1014,10 +1014,6 @@ void NotationViewInputController::hoverMoveEvent(QHoverEvent* event) } m_view->showShadowNote(pos); - - if (event->modifiers() == Qt::ShiftModifier) { - updateShadowNotePopupVisibility(); - } } bool NotationViewInputController::shortcutOverrideEvent(QKeyEvent* event) @@ -1042,9 +1038,9 @@ void NotationViewInputController::keyPressEvent(QKeyEvent* event) dispatcher()->dispatch("edit-text"); event->accept(); } + } else if (event->key() == Qt::Key_Shift) { + updateShadowNotePopupVisibility(); } - - updateShadowNotePopupVisibility(); } void NotationViewInputController::keyReleaseEvent(QKeyEvent* event) @@ -1247,8 +1243,9 @@ void NotationViewInputController::togglePopupForItemIfSupports(const EngravingIt void NotationViewInputController::updateShadowNotePopupVisibility() { const mu::engraving::ShadowNote* shadowNote = viewInteraction()->shadowNote(); - if (!shadowNote || !shadowNote->visible()) { + if (!shadowNote || !AbstractElementPopupModel::supportsPopup(shadowNote)) { m_view->hideElementPopup(); // TEMP HACK - we only want to hide the shadow note popup here + return; } RectF noteHeadRect = shadowNote->symBbox(shadowNote->noteheadSymbol()); diff --git a/src/notation/view/notationviewinputcontroller.h b/src/notation/view/notationviewinputcontroller.h index 46a1b95c20f6e..ef9923e92b810 100644 --- a/src/notation/view/notationviewinputcontroller.h +++ b/src/notation/view/notationviewinputcontroller.h @@ -29,6 +29,7 @@ #include "actions/iactionsdispatcher.h" #include "actions/actionable.h" #include "async/asyncable.h" +#include "async/async.h" #include "context/iglobalcontext.h"