Skip to content

Commit 8579f67

Browse files
committed
Refactor addHairpinOnGripDrag and fix crash
- Fix crash when the created hairpin has a length of zero ticks; in that case, it wouldn't have any HairpinSegments. - Now, the `Hairpin` is only created at the moment that we're certain it will be added to the score, fixing a potentially somewhat significant memory leak. - Now, it is also possible to create a hairpin that spans to the next or previous system.
1 parent 068225d commit 8579f67

File tree

6 files changed

+56
-45
lines changed

6 files changed

+56
-45
lines changed

src/engraving/dom/edit.cpp

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "infrastructure/messagebox.h"
2828

2929
#include "accidental.h"
30+
#include "anchors.h"
3031
#include "articulation.h"
3132
#include "barline.h"
3233
#include "beam.h"
@@ -3964,33 +3965,43 @@ void Score::addHairpinToDynamic(Hairpin* hairpin, Dynamic* dynamic)
39643965
undoAddElement(hairpin);
39653966
}
39663967

3967-
void Score::addHairpinOnGripDrag(Hairpin* hairpin, Dynamic* dynamic, const PointF& pos, Grip grip)
3968+
Hairpin* Score::addHairpinToDynamicOnGripDrag(Dynamic* dynamic, bool isLeftGrip, const PointF& pos)
39683969
{
3969-
track_idx_t track = dynamic->track();
3970-
hairpin->setTrack(track);
3971-
hairpin->setTrack2(track);
3972-
3973-
Segment* seg = nullptr;
3974-
double spacingFactor = 0.5;
3970+
const track_idx_t track = dynamic->track();
39753971
staff_idx_t staffIndex = dynamic->staffIdx();
3972+
Segment* seg = nullptr;
3973+
constexpr double spacingFactor = 0.5;
3974+
3975+
// Ensure time tick segments are created
3976+
EditTimeTickAnchors::updateAnchors(dynamic, track);
39763977

39773978
// Find segment of type ChordRest or TimeTick near cursor postion
3978-
dragPosition(pos, &staffIndex, &seg, spacingFactor, hairpin->allowTimeAnchor());
3979+
dragPosition(pos, &staffIndex, &seg, spacingFactor, /*allowTimeAnchor*/ true);
3980+
3981+
const bool hasValidTick = seg && (isLeftGrip
3982+
? seg->tick() < dynamic->tick()
3983+
: seg->tick() > dynamic->tick());
3984+
if (!hasValidTick) {
3985+
return nullptr;
3986+
}
3987+
3988+
Hairpin* hairpin = Factory::createHairpin(dummy()->segment());
3989+
hairpin->setHairpinType(isLeftGrip ? HairpinType::DECRESC_HAIRPIN : HairpinType::CRESC_HAIRPIN);
39793990

3980-
switch (grip) {
3981-
case Grip::LEFT:
3982-
hairpin->setTick(seg ? seg->tick() : dynamic->segment()->prev1ChordRestOrTimeTick()->tick());
3991+
hairpin->setTrack(track);
3992+
hairpin->setTrack2(track);
3993+
3994+
if (isLeftGrip) {
3995+
hairpin->setTick(seg->tick());
39833996
hairpin->setTick2(dynamic->tick());
3984-
break;
3985-
case Grip::RIGHT:
3997+
} else {
39863998
hairpin->setTick(dynamic->tick());
3987-
hairpin->setTick2(seg ? seg->tick() : dynamic->segment()->next1ChordRestOrTimeTick()->tick());
3988-
break;
3989-
default:
3990-
break;
3999+
hairpin->setTick2(seg->tick());
39914000
}
39924001

39934002
undoAddElement(hairpin);
4003+
4004+
return hairpin;
39944005
}
39954006

39964007
//---------------------------------------------------------

src/engraving/dom/score.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -922,7 +922,7 @@ class Score : public EngravingObject, public muse::Injectable
922922
Hairpin* addHairpin(HairpinType type, ChordRest* cr1, ChordRest* cr2 = nullptr);
923923
void addHairpin(Hairpin* hairpin, ChordRest* cr1, ChordRest* cr2 = nullptr);
924924
void addHairpinToDynamic(Hairpin* hairpin, Dynamic* dynamic);
925-
void addHairpinOnGripDrag(Hairpin* hairpin, Dynamic* dynamic, const PointF& pos, Grip grip);
925+
Hairpin* addHairpinToDynamicOnGripDrag(Dynamic* dynamic, bool isLeftGrip, const PointF& pos);
926926

927927
ChordRest* findCR(Fraction tick, track_idx_t track) const;
928928
ChordRest* findChordRestEndingBeforeTickInStaff(const Fraction& tick, staff_idx_t staffIdx) const;

src/notation/inotationinteraction.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ class INotationInteraction
186186
virtual void addLaissezVibToSelection() = 0;
187187
virtual void addSlurToSelection() = 0;
188188
virtual void addOttavaToSelection(OttavaType type) = 0;
189-
virtual void addHairpinOnGripDrag(engraving::Dynamic* dynamic) = 0;
189+
virtual void addHairpinOnGripDrag(engraving::Dynamic* dynamic, bool isLeftGrip) = 0;
190190
virtual void addHairpinsToSelection(HairpinType type) = 0;
191191
virtual void addAccidentalToSelection(AccidentalType type) = 0;
192192
virtual void putRestToSelection() = 0;

src/notation/internal/notationinteraction.cpp

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,7 +1113,11 @@ void NotationInteraction::drag(const PointF& fromPos, const PointF& toPos, DragM
11131113
m_editData.element->editDrag(m_dragData.ed);
11141114

11151115
if (m_editData.element->isDynamic()) {
1116-
addHairpinOnGripDrag(toDynamic(m_editData.element));
1116+
// When the dynamic has no left grip, the right grip will have index zero, a.k.a. Grip::LEFT.
1117+
// TODO: refactor all code that works with Grips, so that this is not necessary
1118+
Dynamic* dynamic = toDynamic(m_editData.element);
1119+
bool isLeftGrip = dynamic->hasLeftGrip() ? m_editData.curGrip == Grip::LEFT : false;
1120+
addHairpinOnGripDrag(toDynamic(m_editData.element), isLeftGrip);
11171121
}
11181122
} else {
11191123
if (m_editData.element) {
@@ -4369,39 +4373,35 @@ void NotationInteraction::addOttavaToSelection(OttavaType type)
43694373
apply();
43704374
}
43714375

4372-
void NotationInteraction::addHairpinOnGripDrag(Dynamic* dynamic)
4376+
void NotationInteraction::addHairpinOnGripDrag(Dynamic* dynamic, bool isLeftGrip)
43734377
{
4374-
const PointF pos = m_dragData.ed.pos;
4375-
Hairpin* pin = Factory::createHairpin(score()->dummy()->segment());
4376-
4377-
// Right grip
4378-
if (dynamic->rightDragOffset() >= pin->spatium() * 0.8) {
4379-
pin->setHairpinType(HairpinType::CRESC_HAIRPIN);
4380-
4381-
startEdit(TranslatableString("undoableAction", "Add hairpin"));
4382-
score()->addHairpinOnGripDrag(pin, dynamic, pos, Grip::RIGHT);
4383-
apply();
4378+
startEdit(TranslatableString("undoableAction", "Add hairpin"));
43844379

4385-
dynamic->resetRightDragOffset(); // Reset grip offset to zero after drawing the hairpin
4380+
const PointF pos = m_dragData.ed.pos;
4381+
Hairpin* hairpin = score()->addHairpinToDynamicOnGripDrag(dynamic, isLeftGrip, pos);
43864382

4387-
LineSegment* segment = pin->frontSegment();
4388-
select({ segment });
4389-
startEditGrip(segment, Grip::END);
4383+
if (!hairpin) {
4384+
rollback();
4385+
return;
43904386
}
43914387

4392-
// Left grip
4393-
if (abs(dynamic->leftDragOffset()) >= pin->spatium() * 0.8) {
4394-
pin->setHairpinType(engraving::HairpinType::DECRESC_HAIRPIN);
4388+
apply();
43954389

4396-
startEdit(TranslatableString("undoableAction", "Add hairpin"));
4397-
score()->addHairpinOnGripDrag(pin, dynamic, pos, Grip::LEFT);
4398-
apply();
4390+
// Reset grip offset to zero after drawing the hairpin
4391+
dynamic->resetRightDragOffset();
43994392

4400-
dynamic->resetLeftDragOffset(); // Reset grip offset to zero after drawing the hairpin
4393+
IF_ASSERT_FAILED(!hairpin->segmentsEmpty()) {
4394+
return;
4395+
}
44014396

4402-
LineSegment* segment = pin->frontSegment();
4397+
if (isLeftGrip) {
4398+
LineSegment* segment = hairpin->frontSegment();
44034399
select({ segment });
44044400
startEditGrip(segment, Grip::START);
4401+
} else {
4402+
LineSegment* segment = hairpin->backSegment();
4403+
select({ segment });
4404+
startEditGrip(segment, Grip::END);
44054405
}
44064406
}
44074407

src/notation/internal/notationinteraction.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ class NotationInteraction : public INotationInteraction, public muse::Injectable
189189
void addTiedNoteToChord() override;
190190
void addSlurToSelection() override;
191191
void addOttavaToSelection(OttavaType type) override;
192-
void addHairpinOnGripDrag(engraving::Dynamic* dynamic) override;
192+
void addHairpinOnGripDrag(engraving::Dynamic* dynamic, bool isLeftGrip) override;
193193
void addHairpinsToSelection(HairpinType type) override;
194194
void addAccidentalToSelection(AccidentalType type) override;
195195
void putRestToSelection() override;

src/notation/tests/mocks/notationinteractionmock.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ class NotationInteractionMock : public INotationInteraction
148148
MOCK_METHOD(void, addTiedNoteToChord, (), (override));
149149
MOCK_METHOD(void, addSlurToSelection, (), (override));
150150
MOCK_METHOD(void, addOttavaToSelection, (OttavaType), (override));
151-
MOCK_METHOD(void, addHairpinOnGripDrag, (engraving::Dynamic*), (override));
151+
MOCK_METHOD(void, addHairpinOnGripDrag, (engraving::Dynamic*, bool), (override));
152152
MOCK_METHOD(void, addHairpinsToSelection, (HairpinType), (override));
153153
MOCK_METHOD(void, addAccidentalToSelection, (AccidentalType), (override));
154154
MOCK_METHOD(void, putRestToSelection, (), (override));

0 commit comments

Comments
 (0)