From 8bd516c9c9f63c23ac59187b2649937184b6358b Mon Sep 17 00:00:00 2001 From: Michele Spagnolo Date: Tue, 11 Jun 2024 15:31:04 +0200 Subject: [PATCH] Code review --- src/engraving/dom/engravingitem.cpp | 45 ------------------ src/engraving/dom/engravingitem.h | 2 - src/engraving/rendering/dev/systemlayout.cpp | 49 +++++++++++++++++++- src/engraving/rendering/dev/systemlayout.h | 1 + 4 files changed, 48 insertions(+), 49 deletions(-) diff --git a/src/engraving/dom/engravingitem.cpp b/src/engraving/dom/engravingitem.cpp index fdd88431efe22..d2f623165003b 100644 --- a/src/engraving/dom/engravingitem.cpp +++ b/src/engraving/dom/engravingitem.cpp @@ -1349,51 +1349,6 @@ void EngravingItem::setPlacementBasedOnVoiceApplication(DirectionV styledDirecti } } -bool EngravingItem::shouldBeCenteredBetweenStaves(const System* system) const -{ - if (!isStyled(Pid::OFFSET)) { - // NOTE: because of current limitations of the offset system, we can't center an element that's been manually moved. - return false; - } - - const Part* itemPart = part(); - bool centerStyle = style().styleB(Sid::dynamicsHairpinsAutoCenterOnGrandStaff); - AutoOnOff centerProperty = getProperty(Pid::CENTER_BETWEEN_STAVES).value(); - if (itemPart->nstaves() <= 1 || centerProperty == AutoOnOff::OFF || (!centerStyle && centerProperty != AutoOnOff::ON)) { - return false; - } - - if (centerProperty != AutoOnOff::ON && !itemPart->instrument()->isNormallyMultiStaveInstrument()) { - return false; - } - - const Staff* thisStaff = staff(); - const std::vector& partStaves = itemPart->staves(); - IF_ASSERT_FAILED(partStaves.size() > 0) { - return false; - } - - if ((thisStaff == partStaves.front() && placeAbove()) || (thisStaff == partStaves.back() && placeBelow())) { - return false; - } - - staff_idx_t thisIdx = thisStaff->idx(); - if (placeAbove()) { - IF_ASSERT_FAILED(thisIdx > 0) { - return false; - } - } - staff_idx_t nextIdx = placeAbove() ? thisIdx - 1 : thisIdx + 1; - - const SysStaff* thisSystemStaff = system->staff(thisIdx); - const SysStaff* nextSystemStaff = system->staff(nextIdx); - if (!thisSystemStaff->show() || !nextSystemStaff->show()) { - return false; - } - - return centerProperty == AutoOnOff::ON || appliesToAllVoicesInInstrument(); -} - void EngravingItem::relinkPropertiesToMaster(PropertyGroup propGroup) { assert(!score()->isMaster()); diff --git a/src/engraving/dom/engravingitem.h b/src/engraving/dom/engravingitem.h index 14be254c55065..8ad8e5109d2ed 100644 --- a/src/engraving/dom/engravingitem.h +++ b/src/engraving/dom/engravingitem.h @@ -676,8 +676,6 @@ class EngravingItem : public EngravingObject void checkVoiceApplicationCompatibleWithTrack(); void setPlacementBasedOnVoiceApplication(DirectionV styledDirection); - bool shouldBeCenteredBetweenStaves(const System* system) const; - void setOffsetChanged(bool val, bool absolute = true, const PointF& diff = PointF()); //! --------------------- diff --git a/src/engraving/rendering/dev/systemlayout.cpp b/src/engraving/rendering/dev/systemlayout.cpp index bdf190f5d1554..b54db11402ed3 100644 --- a/src/engraving/rendering/dev/systemlayout.cpp +++ b/src/engraving/rendering/dev/systemlayout.cpp @@ -2735,7 +2735,7 @@ void SystemLayout::centerElementsBetweenStaves(const System* system) std::vector centeredItems; for (SpannerSegment* spannerSeg : system->spannerSegments()) { - if (spannerSeg->isHairpinSegment() && spannerSeg->shouldBeCenteredBetweenStaves(system)) { + if (spannerSeg->isHairpinSegment() && elementShouldBeCenteredBetweenStaves(spannerSeg, system)) { centerElementBetweenStaves(spannerSeg, system); centeredItems.push_back(spannerSeg); } @@ -2747,7 +2747,7 @@ void SystemLayout::centerElementsBetweenStaves(const System* system) } for (const Segment& seg : toMeasure(mb)->segments()) { for (EngravingItem* item : seg.annotations()) { - if ((item->isDynamic() || item->isExpression()) && item->shouldBeCenteredBetweenStaves(system)) { + if ((item->isDynamic() || item->isExpression()) && elementShouldBeCenteredBetweenStaves(item, system)) { centerElementBetweenStaves(item, system); centeredItems.push_back(item); } @@ -2758,6 +2758,51 @@ void SystemLayout::centerElementsBetweenStaves(const System* system) AlignmentLayout::alignStaffCenteredItems(centeredItems, system); } +bool SystemLayout::elementShouldBeCenteredBetweenStaves(const EngravingItem* item, const System* system) +{ + if (!item->isStyled(Pid::OFFSET)) { + // NOTE: because of current limitations of the offset system, we can't center an element that's been manually moved. + return false; + } + + const Part* itemPart = item->part(); + bool centerStyle = item->style().styleB(Sid::dynamicsHairpinsAutoCenterOnGrandStaff); + AutoOnOff centerProperty = item->getProperty(Pid::CENTER_BETWEEN_STAVES).value(); + if (itemPart->nstaves() <= 1 || centerProperty == AutoOnOff::OFF || (!centerStyle && centerProperty != AutoOnOff::ON)) { + return false; + } + + if (centerProperty != AutoOnOff::ON && !itemPart->instrument()->isNormallyMultiStaveInstrument()) { + return false; + } + + const Staff* thisStaff = item->staff(); + const std::vector& partStaves = itemPart->staves(); + IF_ASSERT_FAILED(partStaves.size() > 0) { + return false; + } + + if ((thisStaff == partStaves.front() && item->placeAbove()) || (thisStaff == partStaves.back() && item->placeBelow())) { + return false; + } + + staff_idx_t thisIdx = thisStaff->idx(); + if (item->placeAbove()) { + IF_ASSERT_FAILED(thisIdx > 0) { + return false; + } + } + staff_idx_t nextIdx = item->placeAbove() ? thisIdx - 1 : thisIdx + 1; + + const SysStaff* thisSystemStaff = system->staff(thisIdx); + const SysStaff* nextSystemStaff = system->staff(nextIdx); + if (!thisSystemStaff->show() || !nextSystemStaff->show()) { + return false; + } + + return centerProperty == AutoOnOff::ON || item->appliesToAllVoicesInInstrument(); +} + void SystemLayout::centerElementBetweenStaves(EngravingItem* element, const System* system) { bool isAbove = element->placeAbove(); diff --git a/src/engraving/rendering/dev/systemlayout.h b/src/engraving/rendering/dev/systemlayout.h index 2c8efbe541bd0..e6630cfab9a0b 100644 --- a/src/engraving/rendering/dev/systemlayout.h +++ b/src/engraving/rendering/dev/systemlayout.h @@ -84,6 +84,7 @@ class SystemLayout std::vector& bl, Measure* measure); static double minVertSpaceForCrossStaffBeams(System* system, staff_idx_t staffIdx1, staff_idx_t staffIdx2); + static bool elementShouldBeCenteredBetweenStaves(const EngravingItem* item, const System* system); static void centerElementBetweenStaves(EngravingItem* element, const System* system); }; }