Skip to content

Commit

Permalink
Code review corrections
Browse files Browse the repository at this point in the history 
Code review corrections - 2

Code review corrections - 3

Code review corrections - 4

Code review corrections - 5
  • Loading branch information
@mike-spa @cbjeukendrup
mike-spa authored and cbjeukendrup committed Sep 26, 2023
1 parent fb3bf8f commit a62d6f9
Show file tree
Hide file tree
Showing 16 changed files with 164 additions and 137 deletions.
80 changes: 40 additions & 40 deletions src/engraving/dom/tie.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ void TieSegment::changeAnchor(EditData& ed, EngravingItem* element)

void TieSegment::editDrag(EditData& ed)
{
consolidateAdjustOffsetIntoUserOffset();
consolidateAdjustmentOffsetIntoUserOffset();
Grip g = ed.curGrip;
ups(g).off += ed.delta;

Expand Down Expand Up @@ -197,61 +197,61 @@ void TieSegment::computeMidThickness(double tieLengthInSp)

void TieSegment::computeBezier(PointF shoulderOffset)
{
PointF tieStart = ups(Grip::START).p + ups(Grip::START).off;
PointF tieEnd = ups(Grip::END).p + ups(Grip::END).off;
const PointF tieStart = ups(Grip::START).p + ups(Grip::START).off;
const PointF tieEnd = ups(Grip::END).p + ups(Grip::END).off;

PointF tieEndNormalized = tieEnd - tieStart; // normalize to zero
if (RealIsNull(tieEndNormalized.x())) {
return;
}

double tieAngle = atan(tieEndNormalized.y() / tieEndNormalized.x()); // angle required from tie start to tie end--zero if horizontal
const double tieAngle = atan(tieEndNormalized.y() / tieEndNormalized.x()); // angle required from tie start to tie end--zero if horizontal
Transform t;
t.rotateRadians(-tieAngle); // rotate so that we are working with horizontal ties regardless of endpoint height difference
tieEndNormalized = t.map(tieEndNormalized); // apply that rotation
shoulderOffset = t.map(shoulderOffset); // also apply to shoulderOffset

double _spatium = spatium();
const double _spatium = spatium();
double tieLengthInSp = tieEndNormalized.x() / _spatium;

double minShoulderHeight = style().styleMM(Sid::tieMinShoulderHeight);
double maxShoulderHeight = style().styleMM(Sid::tieMaxShoulderHeight);
const double minShoulderHeight = style().styleMM(Sid::tieMinShoulderHeight);
const double maxShoulderHeight = style().styleMM(Sid::tieMaxShoulderHeight);
double shoulderH = minShoulderHeight + _spatium * 0.3 * sqrt(abs(tieLengthInSp - 1));
shoulderH = std::clamp(shoulderH, minShoulderHeight, maxShoulderHeight);

shoulderH -= shoulderOffset.y();

PointF shoulderAdjustOffset = tie()->up() ? PointF(0.0, shoulderOffset.y()) : PointF(0.0, -shoulderOffset.y());
addAdjustOffset(shoulderAdjustOffset, Grip::BEZIER1);
addAdjustOffset(shoulderAdjustOffset, Grip::BEZIER2);
addAdjustmentOffset(shoulderAdjustOffset, Grip::BEZIER1);
addAdjustmentOffset(shoulderAdjustOffset, Grip::BEZIER2);

if (!tie()->up()) {
shoulderH = -shoulderH;
}

double shoulderW = 0.6; // TODO: style

double tieWidth = tieEndNormalized.x();
double bezier1X = (tieWidth - tieWidth * shoulderW) * .5 + shoulderOffset.x();
double bezier2X = bezier1X + tieWidth * shoulderW + shoulderOffset.x();
const double tieWidth = tieEndNormalized.x();
const double bezier1X = (tieWidth - tieWidth * shoulderW) * .5 + shoulderOffset.x();
const double bezier2X = bezier1X + tieWidth * shoulderW + shoulderOffset.x();

PointF tieDrag = PointF(tieWidth * .5, 0.0);
const PointF tieDrag = PointF(tieWidth * .5, 0.0);

PointF bezier1(bezier1X, -shoulderH);
PointF bezier2(bezier2X, -shoulderH);
const PointF bezier1(bezier1X, -shoulderH);
const PointF bezier2(bezier2X, -shoulderH);

computeMidThickness(tieLengthInSp);

PointF tieThickness(0.0, m_midThickness);

PointF bezier1Offset = t.map(ups(Grip::BEZIER1).off);
PointF bezier2Offset = t.map(ups(Grip::BEZIER2).off);
const PointF bezier1Offset = t.map(ups(Grip::BEZIER1).off);
const PointF bezier2Offset = t.map(ups(Grip::BEZIER2).off);

//-----------------------------------calculate p6
PointF bezier1Final = bezier1 + bezier1Offset;
PointF bezier2Final = bezier2 + bezier2Offset;
const PointF bezier1Final = bezier1 + bezier1Offset;
const PointF bezier2Final = bezier2 + bezier2Offset;

PointF tieShoulder = 0.5 * (bezier1Final + bezier2Final);
const PointF tieShoulder = 0.5 * (bezier1Final + bezier2Final);
//-----------------------------------

m_path = PainterPath();
Expand Down Expand Up @@ -751,17 +751,17 @@ void TieSegment::adjustX()
}
}

void TieSegment::consolidateAdjustOffsetIntoUserOffset()
void TieSegment::consolidateAdjustmentOffsetIntoUserOffset()
{
for (size_t i = 0; i < m_adjustOffsets.size(); ++i) {
for (size_t i = 0; i < m_adjustmentOffsets.size(); ++i) {
Grip grip = static_cast<Grip>(i);
PointF adjustOffset = m_adjustOffsets[i];
PointF adjustOffset = m_adjustmentOffsets[i];
if (!adjustOffset.isNull()) {
ups(grip).p -= adjustOffset;
ups(grip).off = adjustOffset;
}
}
resetAdjustOffset();
resetAdjustmentOffset();
}

//---------------------------------------------------------
Expand Down Expand Up @@ -953,18 +953,18 @@ void Tie::calculateIsInside()
return;
}

Note* startN = startNote();
Chord* startChord = startN ? startN->chord() : nullptr;
Note* endN = endNote();
Chord* endChord = endN ? endN->chord() : nullptr;
const Note* startN = startNote();
const Chord* startChord = startN ? startN->chord() : nullptr;
const Note* endN = endNote();
const Chord* endChord = endN ? endN->chord() : nullptr;

if (!startChord || !endChord) {
setIsInside(false);
return;
}

bool startIsSingleNote = startChord->notes().size() <= 1;
bool endIsSingleNote = endChord->notes().size() <= 1;
const bool startIsSingleNote = startChord->notes().size() <= 1;
const bool endIsSingleNote = endChord->notes().size() <= 1;

if (startIsSingleNote && endIsSingleNote) {
setIsInside(style().styleV(Sid::tiePlacementSingleNote).value<TiePlacement>() == TiePlacement::INSIDE);
Expand Down Expand Up @@ -1041,29 +1041,29 @@ bool Tie::isOuterTieOfChord(Grip startOrEnd) const
return false;
}

bool start = startOrEnd == Grip::START;
Note* note = start ? startNote() : endNote();
const bool start = startOrEnd == Grip::START;
const Note* note = start ? startNote() : endNote();
if (!note) {
return false;
}

Chord* chord = note->chord();
const Chord* chord = note->chord();

return (note == chord->upNote() && up()) || (note == chord->downNote() && !up());
}

bool Tie::hasTiedSecondInside() const
{
Note* note = startNote();
const Note* note = startNote();
if (!note) {
return false;
}

Chord* chord = note->chord();
int line = note->line();
int secondInsideLine = up() ? line + 1 : line - 1;
const Chord* chord = note->chord();
const int line = note->line();
const int secondInsideLine = up() ? line + 1 : line - 1;

for (Note* otherNote : chord->notes()) {
for (const Note* otherNote : chord->notes()) {
if (otherNote->line() == secondInsideLine && otherNote->tieFor() && otherNote->tieFor()->up() == up()) {
return true;
}
Expand All @@ -1074,8 +1074,8 @@ bool Tie::hasTiedSecondInside() const

bool Tie::isCrossStaff() const
{
Note* startN = startNote();
Note* endN = endNote();
const Note* startN = startNote();
const Note* endN = endNote();

return (startN && startN->chord()->staffMove() != 0) || (endN && endN->chord()->staffMove() != 0);
}
Expand Down
10 changes: 5 additions & 5 deletions src/engraving/dom/tie.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class TieSegment final : public SlurTieSegment

double m_midThickness = 0.0;

std::array<PointF, static_cast<size_t>(Grip::GRIPS)> m_adjustOffsets = { PointF() };
std::array<PointF, static_cast<size_t>(Grip::GRIPS)> m_adjustmentOffsets;

/*************************
* DEPRECATED
Expand All @@ -65,10 +65,10 @@ class TieSegment final : public SlurTieSegment
void adjustY(const PointF& p1, const PointF& p2);
void adjustX();

void addAdjustOffset(const PointF& offset, Grip grip) { m_adjustOffsets[static_cast<size_t>(grip)] += offset; }
void resetAdjustOffset() { m_adjustOffsets.fill(PointF()); }
PointF adjustOffset(Grip grip) { return m_adjustOffsets[static_cast<size_t>(grip)]; }
void consolidateAdjustOffsetIntoUserOffset();
void addAdjustmentOffset(const PointF& offset, Grip grip) { m_adjustmentOffsets[static_cast<size_t>(grip)] += offset; }
void resetAdjustmentOffset() { m_adjustmentOffsets.fill(PointF()); }
PointF adjustmentOffset(Grip grip) { return m_adjustmentOffsets[static_cast<size_t>(grip)]; }
void consolidateAdjustmentOffsetIntoUserOffset();

void finalizeSegment();

Expand Down
10 changes: 0 additions & 10 deletions src/engraving/rendering/dev/chordlayout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -639,16 +639,6 @@ void ChordLayout::layoutSpanners(Chord* item, LayoutContext& ctx)
}
}

void ChordLayout::layoutSpanners(Chord* item, System* system, const Fraction& stick, LayoutContext& ctx)
{
//! REVIEW Needs explanation
for (const Note* note : item->notes()) {
for (Spanner* sp : note->spannerBack()) {
TLayout::layout(sp, ctx);
}
}
}

//---------------------------------------------------------
// layoutArticulations
// layout tenuto and staccato
Expand Down
1 change: 0 additions & 1 deletion src/engraving/rendering/dev/chordlayout.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ class ChordLayout
static void layout(Chord* item, LayoutContext& ctx);

static void layoutSpanners(Chord* item, LayoutContext& ctx);
static void layoutSpanners(Chord* item, System* system, const Fraction& stick, LayoutContext& ctx);

static void layoutArticulations(Chord* item, LayoutContext& ctx);
static void layoutArticulations2(Chord* item, LayoutContext& ctx, bool layoutOnCrossBeamSide = false);
Expand Down
3 changes: 1 addition & 2 deletions src/engraving/rendering/dev/measurelayout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,13 @@ void MeasureLayout::layout2(Measure* item, LayoutContext& ctx)
continue;
}
Chord* chord = toChord(element);
ChordLayout::layoutSpanners(chord, item->system(), stick, ctx);
for (Note* note : chord->notes()) {
Tie* tieFor = note->tieFor();
Tie* tieBack = note->tieBack();
if (tieFor && tieFor->isCrossStaff()) {
SlurTieLayout::tieLayoutFor(tieFor, item->system());
}
if (tieBack && tieBack->tick() < item->system()->tick() && tieBack->isCrossStaff()) {
if (tieBack && tieBack->tick() < stick && tieBack->isCrossStaff()) {
SlurTieLayout::tieLayoutBack(tieBack, item->system());
}
}
Expand Down
Loading

0 comments on commit a62d6f9

Please sign in to comment.