Skip to content

Implement page locks #33737

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

Open
miiizen wants to merge 10 commits into
base: main
Choose a base branch
from
+2,904 −528
Open

Implement page locks #33737

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e2d15a5
Rename SystemLock to RangeLock
miiizen Apr 29, 2026
c92e225
Implement page locks
miiizen May 1, 2026
736258a
Remove unnecessary mmrest checks
miiizen May 6, 2026
5cd2058
UI update
miiizen Jun 8, 2026
0cc86f2
Add page lock indicator
miiizen Jun 2, 2026
7fe9c9f
Rewrite & rename measureAtTick
miiizen Jun 9, 2026
4c7599f
Review fixes
miiizen Jun 9, 2026
3ac7e5a
Disable indicators in all modes but page mode
miiizen Jun 11, 2026
d2d9db0
System lock interaction fixes
miiizen Jun 16, 2026
200b731
Fix build
miiizen Jun 17, 2026
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
2 changes: 2 additions & 0 deletions src/engraving/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ target_sources(engraving PRIVATE
editing/editmeasures.h
editing/editnote.cpp
editing/editnote.h
editing/editpagelocks.cpp
editing/editpagelocks.h
editing/editpart.cpp
editing/editpart.h
editing/editproperty.cpp
Expand Down
1 change: 1 addition & 0 deletions src/engraving/api/v1/apitypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ enum class ElementType {
TAB_DURATION_SYMBOL = int(mu::engraving::ElementType::TAB_DURATION_SYMBOL),
FSYMBOL = int(mu::engraving::ElementType::FSYMBOL),
PAGE = int(mu::engraving::ElementType::PAGE),
PAGE_LOCK_INDICATOR = int(mu::engraving::ElementType::PAGE_LOCK_INDICATOR),
HAIRPIN = int(mu::engraving::ElementType::HAIRPIN),
OTTAVA = int(mu::engraving::ElementType::OTTAVA),
PEDAL = int(mu::engraving::ElementType::PEDAL),
Expand Down
4 changes: 2 additions & 2 deletions src/engraving/api/v1/elements.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,11 +503,11 @@ void System::setIsLocked(bool locked)
if (locked == isLocked()) {
return;
}
const mu::engraving::SystemLock* currentLock = system()->systemLock();
const mu::engraving::RangeLock* currentLock = system()->systemLock();
if (currentLock && !locked) {
EditSystemLocks::undoRemoveSystemLock(system()->score(), currentLock);
} else if (!currentLock && locked) {
EditSystemLocks::undoAddSystemLock(system()->score(), new mu::engraving::SystemLock(system()->first(), system()->last()));
EditSystemLocks::undoAddSystemLock(system()->score(), new mu::engraving::RangeLock(system()->first(), system()->last()));
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/engraving/dom/dom.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,8 @@ set(DOM_SRC
${CMAKE_CURRENT_LIST_DIR}/property.h
${CMAKE_CURRENT_LIST_DIR}/range.cpp
${CMAKE_CURRENT_LIST_DIR}/range.h
${CMAKE_CURRENT_LIST_DIR}/rangelock.cpp
${CMAKE_CURRENT_LIST_DIR}/rangelock.h
${CMAKE_CURRENT_LIST_DIR}/rasgueado.cpp
${CMAKE_CURRENT_LIST_DIR}/rasgueado.h
${CMAKE_CURRENT_LIST_DIR}/realizedharmony.cpp
Expand Down Expand Up @@ -321,8 +323,6 @@ set(DOM_SRC
${CMAKE_CURRENT_LIST_DIR}/symbol.h
${CMAKE_CURRENT_LIST_DIR}/synthesizerstate.cpp
${CMAKE_CURRENT_LIST_DIR}/synthesizerstate.h
${CMAKE_CURRENT_LIST_DIR}/systemlock.cpp
${CMAKE_CURRENT_LIST_DIR}/systemlock.h
${CMAKE_CURRENT_LIST_DIR}/system.cpp
${CMAKE_CURRENT_LIST_DIR}/system.h
${CMAKE_CURRENT_LIST_DIR}/systemdivider.cpp
Expand Down
5 changes: 4 additions & 1 deletion src/engraving/dom/engravingobject.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ class Ornament;
class Ottava;
class OttavaSegment;
class Page;
class PageLockIndicator;
class PalmMute;
class PalmMuteSegment;
class Parenthesis;
Expand Down Expand Up @@ -437,6 +438,7 @@ class EngravingObject
CONVERT(FretDiagram, FRET_DIAGRAM)
CONVERT(HarpPedalDiagram, HARP_DIAGRAM)
CONVERT(Page, PAGE)
CONVERT(PageLockIndicator, PAGE_LOCK_INDICATOR)
CONVERT(Text, TEXT)
CONVERT(MeasureNumber, MEASURE_NUMBER)
CONVERT(MMRestRange, MMREST_RANGE)
Expand Down Expand Up @@ -584,7 +586,7 @@ class EngravingObject
return isArticulationFamily() || isFermata();
}

bool isIndicatorIcon() const { return isSystemLockIndicator() || isStaffVisibilityIndicator(); }
bool isIndicatorIcon() const { return isSystemLockIndicator() || isPageLockIndicator() || isStaffVisibilityIndicator(); }
};

//---------------------------------------------------
Expand Down Expand Up @@ -766,6 +768,7 @@ CONVERT(ChordLine)
CONVERT(FretDiagram)
CONVERT(HarpPedalDiagram)
CONVERT(Page)
CONVERT(PageLockIndicator)
CONVERT(SystemText)
CONVERT(BracketItem)
CONVERT(Staff)
Expand Down
14 changes: 12 additions & 2 deletions src/engraving/dom/factory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@
#include "stringtunings.h"
#include "system.h"
#include "systemdivider.h"
#include "systemlock.h"
#include "rangelock.h"
#include "systemtext.h"
#include "soundflag.h"
#include "tapping.h"
Expand Down Expand Up @@ -300,6 +300,7 @@ EngravingItem* Factory::doCreateItem(ElementType type, EngravingItem* parent)
case ElementType::FIGURED_BASS_ITEM:
case ElementType::DUMMY:
case ElementType::SYSTEM_LOCK_INDICATOR:
case ElementType::PAGE_LOCK_INDICATOR:
case ElementType::HAMMER_ON_PULL_OFF_SEGMENT:
case ElementType::HAMMER_ON_PULL_OFF_TEXT:
case ElementType::TAPPING_HALF_SLUR:
Expand Down Expand Up @@ -475,6 +476,15 @@ MAKE_ITEM_IMPL(NoteLine, Note);

CREATE_ITEM_IMPL(Page, RootItem, isAccessibleEnabled)

PageLockIndicator* Factory::createPageLockIndicator(System * parent, const RangeLock * lock, bool isAccessibleEnabled)
{
PageLockIndicator* sli = new PageLockIndicator(parent, lock);
sli->setAccessibleEnabled(isAccessibleEnabled);
return sli;
}

COPY_ITEM_IMPL(PageLockIndicator)

CREATE_ITEM_IMPL(PartialTie, Note, isAccessibleEnabled)
COPY_ITEM_IMPL(PartialTie)

Expand Down Expand Up @@ -738,7 +748,7 @@ CREATE_ITEM_IMPL(TimeTickAnchor, Segment, isAccessibleEnabled)

CREATE_ITEM_IMPL(StaffVisibilityIndicator, System, isAccessibleEnabled)

SystemLockIndicator* Factory::createSystemLockIndicator(System * parent, const SystemLock * lock, bool isAccessibleEnabled)
SystemLockIndicator* Factory::createSystemLockIndicator(System * parent, const RangeLock * lock, bool isAccessibleEnabled)
{
SystemLockIndicator* sli = new SystemLockIndicator(parent, lock);
sli->setAccessibleEnabled(isAccessibleEnabled);
Expand Down
7 changes: 5 additions & 2 deletions src/engraving/dom/factory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
namespace mu::engraving {
class Instrument;
class RootItem;
class SystemLock;
class RangeLock;
class TremoloSingleChord;
class TremoloTwoChord;

Expand Down Expand Up @@ -120,7 +120,7 @@ class Factory

static StaffVisibilityIndicator* createStaffVisibilityIndicator(System* parent, bool isAccessibleEnabled = true);

static SystemLockIndicator* createSystemLockIndicator(System* parent, const SystemLock* lock, bool isAccessibleEnabled = true);
static SystemLockIndicator* createSystemLockIndicator(System* parent, const RangeLock* lock, bool isAccessibleEnabled = true);
static SystemLockIndicator* copySystemLockIndicator(const SystemLockIndicator& src);

static Lyrics* createLyrics(ChordRest* parent, bool isAccessibleEnabled = true);
Expand Down Expand Up @@ -148,6 +148,9 @@ class Factory

static Page* createPage(RootItem* parent, bool isAccessibleEnabled = true);

static PageLockIndicator* createPageLockIndicator(System* parent, const RangeLock* lock, bool isAccessibleEnabled = true);
static PageLockIndicator* copyPageLockIndicator(const PageLockIndicator& src);

static Parenthesis* createParenthesis(EngravingItem* parent, bool isAccessibleEnabled = true);
static Parenthesis* copyParenthesis(const Parenthesis& src);

Expand Down
97 changes: 90 additions & 7 deletions src/engraving/dom/measurebase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,13 @@ MeasureBase::~MeasureBase()

System* MeasureBase::prevNonVBoxSystem() const
{
bool mmRests = score()->style().styleB(Sid::createMultiMeasureRests);
System* curSystem = system();
IF_ASSERT_FAILED(curSystem) {
return nullptr;
}

System* prevSystem = curSystem;
for (const MeasureBase* mb = this; mb && prevSystem == curSystem; mb = mmRests ? mb->prevMM() : mb->prev()) {
for (const MeasureBase* mb = this; mb && prevSystem == curSystem; mb = mb->prevMM()) {
if (mb->isMeasure() || mb->isHBox()) {
prevSystem = mb->system();
} else {
Expand All @@ -120,14 +119,13 @@ System* MeasureBase::prevNonVBoxSystem() const

System* MeasureBase::nextNonVBoxSystem() const
{
bool mmRests = score()->style().styleB(Sid::createMultiMeasureRests);
System* curSystem = system();
IF_ASSERT_FAILED(curSystem) {
return nullptr;
}

System* nextSystem = curSystem;
for (const MeasureBase* mb = this; mb && nextSystem == curSystem; mb = mmRests ? mb->nextMM() : mb->next()) {
for (const MeasureBase* mb = this; mb && nextSystem == curSystem; mb = mb->nextMM()) {
if (mb->isMeasure() || mb->isHBox()) {
nextSystem = mb->system();
} else {
Expand All @@ -138,6 +136,41 @@ System* MeasureBase::nextNonVBoxSystem() const
return nextSystem != curSystem ? nextSystem : nullptr;
}

Page* MeasureBase::page() const
{
return system() ? system()->page() : nullptr;
}

Page* MeasureBase::prevPage() const
{
Page* curPage = system() ? system()->page() : nullptr;
IF_ASSERT_FAILED(curPage) {
return nullptr;
}

Page* prevPage = curPage;
for (const MeasureBase* mb = this; mb && prevPage == curPage; mb = mb->prevMM()) {
prevPage = mb->system()->page();
}

return prevPage != curPage ? prevPage : nullptr;
}

Page* MeasureBase::nextPage() const
{
Page* curPage = system() ? system()->page() : nullptr;
IF_ASSERT_FAILED(curPage) {
return nullptr;
}

Page* nextPage = curPage;
for (const MeasureBase* mb = this; mb && nextPage == curPage; mb = mb->nextMM()) {
nextPage = mb->system()->page();
}

return nextPage != curPage ? nextPage : nullptr;
}
Comment on lines +144 to +172

@coderabbitai coderabbitai Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Potential null dereference in page navigation loops.

In both prevPage() and nextPage(), the loop body accesses mb->system()->page() without checking if mb->system() is null. While the initial check validates this->system(), subsequent MeasureBase instances encountered via prevMM()/nextMM() may have null systems (e.g., unattached measures, frames outside systems).

🛡️ Proposed fix to add null guard 
 Page* MeasureBase::prevPage() const
 {
     Page* curPage = system() ? system()->page() : nullptr;
     IF_ASSERT_FAILED(curPage) {
         return nullptr;
     }
 
     Page* prevPage = curPage;
     for (const MeasureBase* mb = this; mb && prevPage == curPage; mb = mb->prevMM()) {
-        prevPage = mb->system()->page();
+        if (mb->system()) {
+            prevPage = mb->system()->page();
+        }
     }
 
     return prevPage != curPage ? prevPage : nullptr;
 }
 
 Page* MeasureBase::nextPage() const
 {
     Page* curPage = system() ? system()->page() : nullptr;
     IF_ASSERT_FAILED(curPage) {
         return nullptr;
     }
 
     Page* nextPage = curPage;
     for (const MeasureBase* mb = this; mb && nextPage == curPage; mb = mb->nextMM()) {
-        nextPage = mb->system()->page();
+        if (mb->system()) {
+            nextPage = mb->system()->page();
+        }
     }
 
     return nextPage != curPage ? nextPage : nullptr;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Page* MeasureBase::prevPage() const
{
Page* curPage = system() ? system()->page() : nullptr;
IF_ASSERT_FAILED(curPage) {
return nullptr;
}
Page* prevPage = curPage;
for (const MeasureBase* mb = this; mb && prevPage == curPage; mb = mb->prevMM()) {
prevPage = mb->system()->page();
}
return prevPage != curPage ? prevPage : nullptr;
}
Page* MeasureBase::nextPage() const
{
Page* curPage = system() ? system()->page() : nullptr;
IF_ASSERT_FAILED(curPage) {
return nullptr;
}
Page* nextPage = curPage;
for (const MeasureBase* mb = this; mb && nextPage == curPage; mb = mb->nextMM()) {
nextPage = mb->system()->page();
}
return nextPage != curPage ? nextPage : nullptr;
}
Page* MeasureBase::prevPage() const
{
Page* curPage = system() ? system()->page() : nullptr;
IF_ASSERT_FAILED(curPage) {
return nullptr;
}
Page* prevPage = curPage;
for (const MeasureBase* mb = this; mb && prevPage == curPage; mb = mb->prevMM()) {
if (mb->system()) {
prevPage = mb->system()->page();
}
}
return prevPage != curPage ? prevPage : nullptr;
}
Page* MeasureBase::nextPage() const
{
Page* curPage = system() ? system()->page() : nullptr;
IF_ASSERT_FAILED(curPage) {
return nullptr;
}
Page* nextPage = curPage;
for (const MeasureBase* mb = this; mb && nextPage == curPage; mb = mb->nextMM()) {
if (mb->system()) {
nextPage = mb->system()->page();
}
}
return nextPage != curPage ? nextPage : nullptr;
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/engraving/dom/measurebase.cpp` around lines 144 - 172, The loop bodies in
both prevPage() and nextPage() methods dereference mb->system()->page() without
first checking if mb->system() is null. While the initial check validates
this->system(), subsequent MeasureBase instances from prevMM() or nextMM() may
have null systems. Add a null check for mb->system() before accessing page() in
both loop bodies of prevPage() and nextPage() methods to prevent null pointer
dereference.


//---------------------------------------------------------
// add
/// Add new EngravingItem \a el to MeasureBase
Expand Down Expand Up @@ -638,20 +671,37 @@ bool MeasureBase::isBefore(const MeasureBase* other) const
return false;
}

const SystemLock* MeasureBase::systemLock() const
const RangeLock* MeasureBase::systemLock() const
{
return score()->systemLocks()->lockContaining(this);
}

bool MeasureBase::isStartOfSystemLock() const
{
const SystemLock* lock = score()->systemLocks()->lockStartingAt(this);
const RangeLock* lock = score()->systemLocks()->lockStartingAt(this);
return lock != nullptr;
}

bool MeasureBase::isEndOfSystemLock() const
{
const SystemLock* lock = systemLock();
const RangeLock* lock = systemLock();
return lock && lock->endMB() == this;
}

const RangeLock* MeasureBase::pageLock() const
{
return score()->pageLocks()->lockContaining(this);
}

bool MeasureBase::isStartOfPageLock() const
{
const RangeLock* lock = score()->pageLocks()->lockStartingAt(this);
return lock != nullptr;
}

bool MeasureBase::isEndOfPageLock() const
{
const RangeLock* lock = pageLock();
return lock && lock->endMB() == this;
}

Expand Down Expand Up @@ -902,6 +952,39 @@ Measure* MeasureBaseList::measureByTick(int tick) const
return nullptr;
}

MeasureBase* MeasureBaseList::measureBaseByTick(int tick) const
{
if (empty() || tick > m_last->endTick().ticks()) {
return nullptr;
}

auto it = m_tickIndex.upper_bound(tick);

if (it == m_tickIndex.begin()) {
MeasureBase* mb = it->second;

return mb;
}

--it;
for (;; --it) {
if (it == m_tickIndex.begin()) {
MeasureBase* mb = it->second;

return mb;
}

MeasureBase* mb = it->second;
if (!mb) {
break;
}

return mb;
}

return nullptr;
Comment on lines +955 to +985

@coderabbitai coderabbitai Bot Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Persisting arbitrary lock boundaries by tick is ambiguous here.

RangeLock now starts/ends on any MeasureBase, but this helper still collapses a tick to one floor entry from m_tickIndex. That loses information whenever multiple MeasureBases share the same tick (for example a frame and the following measure at tick 0), so a saved page/system lock can round-trip onto the wrong boundary and change which range is actually locked. This lookup needs an exact, disambiguated boundary key instead of upper_bound() on tick alone.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/engraving/dom/measurebase.cpp` around lines 955 - 985, measureBaseByTick
currently collapses multiple MeasureBase entries at the same tick by using
m_tickIndex.upper_bound(tick), losing disambiguation when several MeasureBases
share a tick; change it to perform an exact, disambiguated lookup (not
upper_bound on tick alone) by using the composite boundary key used by RangeLock
(e.g., tick + boundary type/offset) or by using m_tickIndex.equal_range(tick)
and then selecting the correct MeasureBase by checking the boundary identity
(start vs end, frame vs measure) so saved locks round-trip to the same
MeasureBase; update MeasureBaseList::measureBaseByTick to accept or derive the
extra boundary identifier and iterate the equal_range results to return the
exact MeasureBase instead of the current upper_bound-based logic.

}

std::vector<MeasureBase*> MeasureBaseList::measureBasesAtTick(int tick) const
{
std::vector<MeasureBase*> result;
Expand Down
12 changes: 10 additions & 2 deletions src/engraving/dom/measurebase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class LayoutBreak;
class Measure;
class Score;
class System;
class SystemLock;
class RangeLock;

//---------------------------------------------------------
// Repeat
Expand Down Expand Up @@ -82,6 +82,9 @@ class MeasureBase : public EngravingItem
System* system() const { return toSystem(explicitParent()); }
System* prevNonVBoxSystem() const;
System* nextNonVBoxSystem() const;
Page* page() const;
Page* prevPage() const;
Page* nextPage() const;
void setParent(System* s) { EngravingItem::setParent((EngravingObject*)(s)); }

virtual void scanElements(std::function<void(EngravingItem*)> func) override;
Expand Down Expand Up @@ -160,10 +163,14 @@ class MeasureBase : public EngravingItem
bool isAfter(const MeasureBase* other) const { return !isBeforeOrEqual(other); }
bool isAfterOrEqual(const MeasureBase* other) const { return !isBefore(other); }

const SystemLock* systemLock() const;
const RangeLock* systemLock() const;
bool isStartOfSystemLock() const;
bool isEndOfSystemLock() const;

const RangeLock* pageLock() const;
bool isStartOfPageLock() const;
bool isEndOfPageLock() const;

protected:
MeasureBase(const ElementType& type, System* system = 0);
MeasureBase(const MeasureBase&);
Expand Down Expand Up @@ -203,6 +210,7 @@ class MeasureBaseList
void updateTickIndex();

Measure* measureByTick(int tick) const;
MeasureBase* measureBaseByTick(int tick) const;
std::vector<MeasureBase*> measureBasesAtTick(int tick) const;

private:
Expand Down
12 changes: 12 additions & 0 deletions src/engraving/dom/navigate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,12 @@ EngravingItem* Score::nextElement()
e = toSystemLockIndicator(e)->systemLock()->endMB();
continue;
}
case ElementType::PAGE_LOCK_INDICATOR:
{
staffId = 0;
e = toPageLockIndicator(e)->pageLock()->endMB();
continue;
}
case ElementType::SOUND_FLAG:
if (EngravingItem* parent = toSoundFlag(e)->parentItem()) {
return parent;
Expand Down Expand Up @@ -1151,6 +1157,12 @@ EngravingItem* Score::prevElement()
e = toSystemLockIndicator(e)->systemLock()->endMB();
continue;
}
case ElementType::PAGE_LOCK_INDICATOR:
{
staffId = 0;
e = toPageLockIndicator(e)->pageLock()->endMB();
continue;
}
case ElementType::HARMONY: {
Harmony* harmony = toHarmony(e);
if (harmony->isInFretBox()) {
Expand Down
Loading
Loading