Percussion shadow note popup - improve stability of popup

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
 }

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;

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"));

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<NotationSelection> m_selection = nullptr;
     muse::async::Notification m_selectionChanged;
+    muse::async::Notification m_shadowNoteChanged;
 
     DragData m_dragData;
     muse::async::Notification m_dragChanged;

src/notation/qml/MuseScore/NotationScene/internal/PercussionNotePopupContent.qml

@@ -34,6 +34,10 @@ Row {
 
     PercussionNotePopupContentModel {
         id: contentModel
+
+        Component.onCompleted: {
+            contentModel.init()
+        }
     }
 
     Row {

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));
 

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;

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,17 +89,15 @@ 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;
 
     engraving::ElementType elementType() const;
     const engraving::ElementTypeSet& dependentElementTypes() const;
 
-    void updateItemRect();
-
     PopupModelType m_modelType = PopupModelType::TYPE_UNDEFINED;
-    QRect m_itemRect;
 };
 
 using PopupModelType = AbstractElementPopupModel::PopupModelType;

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();
     }
 

src/notation/view/internal/percussionnotepopupcontentmodel.h

@@ -24,11 +24,13 @@
 
 #include <QObject>
 
+#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();
 

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);
 }

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

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());

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"