MuseScore
MuseScore copied to clipboard
Fix #16396: Tie invisibility crossing system breaks
Resolves: #16396
- [x] I signed the CLA
- [x] The title of the PR describes the problem it addresses
- [x] Each commit's message describes its purpose and effects, and references the issue it resolves
- [ ] If changes are extensive, there is a sequence of easily reviewable commits
- [x] The code in the PR follows the coding rules
- [x] There are no unnecessary changes
- [x] The code compiles and runs on my machine, preferably after each commit individually
- [ ] I created a unit test or vtest to verify the changes I made (if applicable)
I found that there are some similar problems:
- For ties, the same applies to the "color" property too
- For slurs, it also applies to the "color" property, but seemingly not to the "visible" property
- For glissandos, it applies to the "visible" property but seemingly not to the "color" property
There may be even more cases (I haven't tested bends for example). And perhaps there are even more properties to which this applies, for example the "auto-place" property. So we should either fix all cases separately, or look if a more general solution can be found.
tie color and glissando visibility seem easy to fix (1-liner each), slur color not so much (haven't found a way yet)
Actually that's a 1-liner too...
tie color:
diff --git a/libmscore/tie.cpp b/libmscore/tie.cpp
index 8d6f37c4c2..66f7f3bac3 100644
--- a/libmscore/tie.cpp
+++ b/libmscore/tie.cpp
@@ -1010,6 +1010,7 @@ TieSegment* Tie::layoutBack(System* system)
TieSegment* segment = segmentAt(1);
segment->setSystem(system);
segment->setVisible(visible());
+ segment->setColor(color());
qreal x = system->firstNoteRestSegmentX(true);
glissando visibility:
diff --git a/libmscore/glissando.cpp b/libmscore/glissando.cpp
index 4be6756624..6f36e1f9c1 100644
--- a/libmscore/glissando.cpp
+++ b/libmscore/glissando.cpp
@@ -82,7 +82,7 @@ void GlissandoSegment::draw(QPainter* painter) const
painter->save();
qreal _spatium = spatium();
- QPen pen(curColor(visible(), glissando()->lineColor()));
+ QPen pen(curColor(glissando()->visible(), glissando()->lineColor()));
pen.setWidthF(glissando()->lineWidth());
pen.setCapStyle(Qt::FlatCap);
painter->setPen(pen);
slur color
diff --git a/libmscore/slur.cpp b/libmscore/slur.cpp
index 28302c1b01..4aa7dba0c1 100644
--- a/libmscore/slur.cpp
+++ b/libmscore/slur.cpp
@@ -30,7 +30,7 @@ namespace Ms {
void SlurSegment::draw(QPainter* painter) const
{
- QPen pen(curColor());
+ QPen pen(curColor(slur()->visible(), slur()->color()));
qreal mag = staff() ? staff()->mag(slur()->tick()) : 1.0;
//Replace generic Qt dash patterns with improved equivalents to show true dots (keep in sync with tie.cpp)
That's for 3.x though
That's even better, much more consistent
But @cbjeukendrup is right: AutoPlace is affected too, albeit different: it disconects regardless whether set before apply a system braek or after, so seems a different issue altogether
Thanks @Jojo-Schmitz for finding the places for slurs and glissandos! I agree that autoplace is a different issue, since this PR currently only changes the visual rendering of the elements and not any actual properties themselves.
Same issue and fix seems to apply to GuitarBend(Segment and GuitarBendHoldSegment), as far as I can tell. Might apply to Beam too
Guitar bends seem fine in master (both visibility and color)
I guess it'd need to be item->getProperty(Pid::VISIBLE).toBool()
But what about the color?
Would that be item->getProperty(Pid::COLOR).toInt()
? Nah, that doesn't work at all
My understanding would be PropertyValue::fromValue(item->getProperty(Pid::COLOR))
My understanding would be
PropertyValue::fromValue(item->getProperty(Pid::COLOR))
Yep!
PropertyValue::fromValue(item->getProperty(Pid::COLOR))
doesn't make a lot of sense, because getProperty
already returns a PropertyValue, so that expression would be equivalent to (but less performant than) just item->getProperty(Pid::COLOR)
.
If you want to retrieve the property value as a Color
type, then item->getProperty(Pid::COLOR).value<Color>()
is probably what you want.