Redundant lines still saved
Reported version
3.0
Priority
P3 - Low
Type
Functional
Frequency
Once
Severity
S5 - Suggestion
Reproducibility
Always
Status
active
Regression
Yes
Workaround
No
Project
Version: 3.0.5
Start with /usr/share/mscore3-3.0/demos/Fugue_1.mscx then:
- go to measure 19
- double-click the first/upper 𝆒 and move its start and end points a bit
- colour the second/lower 𝆒 red
- save as MSCX again
- compare with the original
Result (excluding changes like programVersion and that it takes my custom style):
--- /usr/share/mscore3-3.0/demos/Fugue_1.mscx 2019-03-12 12:19:53.000000000 +0100
+++ Fugue_1.mscx 2019-03-28 14:37:00.539320959 +0100
@@ -5087,6 +5182,12 @@
<HairPin>
<subtype>0</subtype>
<linkedMain/>
+ <Segment>
+ <subtype>0</subtype>
+ <offset x="-1.99896" y="-2.5"/>
+ <off2 x="1.49922" y="0"/>
+ <offset x="-1.99896" y="-2.5"/>
+ </Segment>
</HairPin>
<next>
<location>
@@ -7249,6 +7351,8 @@
<HairPin>
<subtype>0</subtype>
<linkedMain/>
+ <color r="255" g="0" b="0" a="255"/>
+ <color r="255" g="0" b="0" a="255"/>
</HairPin>
<next>
<location>Both the offset and color tags double. The second is trivial to fix by piping through cat -s, the first isn’t.
If manually removed, MuseScore does not re-add the duplicates on saving.
If not manually removed, MuseScore sometimes removes them on saving, and most of the time doesn’t.
Comments
I think this probably has to do with lines having overall line properties as well as properties for each segment. Not sure it's meaningful to write the overall line properties, but we do need to write the segment ones. Right now it's harmless as far as I know and I'd be a little afraid of breaking something by changing it for no reason. In particular, it would be important to make sure the offset gets scaled properly - it's done for line segments in SLine::writeProperties but not in Element::writeProperties. The former is where I just fixed #283312: Hairpin position changes on reload if horizontal offset applied on small staff.
It’s probably harmless at the moment, except as soon as these two instances of the same property are no longer consistent.
I think segment colours are saved separately, under a
SegmentXML tag, too.Indeed, making the upper (moved) hairpin green results in:
@@ -5087,6 +5183,14 @@ <HairPin> <subtype>0</subtype> <linkedMain/> + <color r="0" g="255" b="0" a="255"/> + <color r="0" g="255" b="0" a="255"/> + <Segment> + <subtype>0</subtype> + <offset x="-1.99896" y="-2.5"/> + <off2 x="1.49922" y="0"/> + <color r="0" g="255" b="0" a="255"/> + </Segment> </HairPin> <next> <location>I think what happens is that Element::writeProperties is writing offset & color, only if they are set to NOSTYLE (meaning, there is not style setting to control the default for this element type). It is up to each specific element type to handle writing these properties if they are styled. For lines, we are writing color in SLine::write() but should not be because there is no property for this. Then, the individual segments of the line are also writing offsets there but really should not be sicne they too are not styled.
The fix is presumably to remove these two lines from SLine. Will probably break umpteen tests. I can't see any way these values could ever be written different except for the scaling bug I recently fixed, so to me the is harmless and not worth the time it would take to deal with test updates. But if you feel like, I'm pretty sure that's what you need to do.
Yeah I agree. There are actually a number of line saving issues which are all a bit messy right now. This is probably worth fixing only along with a general cleanup of line saving and line saving issues (which that is probably worth fixing at some point).