[MusicXML import] [3.6.0 regression] several strings are incorrectly shown as unplayed
Reported version
3.6
Type
Functional
Frequency
Once
Severity
S4 - Minor
Reproducibility
Always
Status
closed
Regression
Yes
Workaround
No
Project
Attached MusicXML file imports correctly up to 3.5.1, but in 3.6.0:
- the "G" is missing
- several strings are incorrectly shown as unplayed
Relates to #270643: [EPIC] MusicXML import/export issues and #283681: [EPIC] Fretboard diagram issues
| Attachment | Size |
|---|---|
| Guitar_fretboard_G.xml | 3.9 KB |
| Guitar_fretboard_G_3.5.1_OK.png | 8.24 KB |
| Guitar_fretboard_G_3.6.0_NOK.png | 8.38 KB |
Comments
No comment about missing "G" text here, but upon examing 3.x's current code, it looks as though the cause for the excessive "X" markings is caused by the following function:
void FretDiagram::init(StringData* stringData, Chord* chord)
line 268-9:
for (int string = 0; string < _strings; ++string)
....setMarker(string, FretMarkerType::CROSS);
FretMarkerType:: can also be NONE, which intuitively makes sense to have as the initialization. I'm not sure what the repercussions would be by making NONE be the init, as I don't at the very moment have a local build, but again intuitively that shouldn't cause any problems: that is what seems is in need of testing. Having FretMarkerType::CROSS be the default is relying upon the following setDot method calls to clear the crosses when now it no longer does so in order to facilitate multiple dots in addition to possible crosses for users who want fretboard diagrams not necessarily as chord charts but rather fret charts (scales/patterns/etc).
Update: a problem here is that musicxml export doesn't have any "X" data. For example: if you make a fretboard diagram as being for a 6 stringed instrument having a dot on the second fret for the 4th string, and have two "X's" on the 6th and 5th string like: [XX2---] where the first three strings have no data at all..., after exporting and reading the musicxml file, i see no explicit "x"s set anywhere but only the fret "2" marking!
Question becomes: is there a way to explicitly get "x" markings in musicxml? I would imagine, but I haven't researched this. If so, that also needs to be implemented because it doesn't seem to be currently involved in the musicxml writing process. Hope this helps.
Aside issue:
Another situation found while checking this is that the "frets" attribute is not being honored. For example, if the user makes a fretboard diagram only showing two frets, upon export/import, six frets are shown! After checking the corresponding MusicXML file, it looks as though the attribute ["frame-frets"] is written correctly with the appropriate data, so it seems to be having to do with reading/applying rather than exporting.
In reply to Update: a problem here is… by worldwideweary
No, this is implicit, see http://usermanuals.musicxml.com/MusicXML/MusicXML.htm#CT-MusicXML-frame… (quote) "The frame-note type represents each note included in the frame. An open string will have a fret value of 0, while a muted string will not be associated with a frame-note element."
In reply to No, this is implicit, see… by Leon Vinken
My previous comment was in reply to the Jan 29, 2021 - 01:18 question.
In reply to My previous comment was in… by Leon Vinken
Thanks for the documentation link. Based on that, it would seem impossible to follow the musicxml-spec and have the export/import be exactly as how the user defines a fretboard chart if the user doesn't explicitly place mutes on any string with no fret marks (All blank strings will result as having an 'x' upon export/import). Wonder if there's a way around that, or if that is more to be considered as the most appropriate behavior.
And again, switching to NONE instead of initializing to CROSS (mute) still seems appropriate so that an ["X" + a fret mark] don't occur simultaneously on the same string, since apparently OPEN is represented in the xml as a 0 fret and shouldn't cause any problems for fret charts importing/exporting.
Update: Actually, from the spec: The [frame type's unplayed] attribute indicates what to display above a string that has no associated frame-note element. Typical values are x and the empty string.. ... Sounds like that should be capable of being implemented to be used with 'x' depending on the MuseScore data so that [export/import musicxml] could have an explicit reference to 'x' and not implicitly place them when the user never explicitly used them upon import.
P.S.: I get appropriate chord names upon export/import, so for example the missing 'G' mentioned in the first post wasn't a problem on my system. But again, the non-honoring of the amount of frets displayed is slightly off-putting, especially since the actual data is stored in the exported xml file so shouldn't be a problem to get actualized.
P.P.S. The cause of the #-of-frets not working I think is due to setting _maxFrets instead of _frets for FretboardDiagram. Honestly, don't know what _maxFrets is for if _frets is the attribute showing however many frets in the diagram.
Another observation: exporting/importing guitar charts that have multiple dots doesn't seem to work at all when there is an open marker: the data doesn't get saved upon exporting musicxml as verified by reading the uncompressed musicxml file. If there are no markers, however, it appears okay.
I was going to attempt to fix all this coming in from the outset, and the code seemed reasonable to fix by changing how FretDiagram initializes and is read through its methods, but those don't seem to be where the code is occurring having to do with the issue as far I can tell upon first attempt. Hope it gets worked out.
A quick comment about the missing "G": The correlated chord-text isn't attached to the fret-diagram upon importing but rather to the corresponding ChordRest (or maybe just the segment) upon importing.
@Leon Vinken, do you happen to know where in the code the musicxml formation (writing/reading) for fretboard charts are executed? It was said in the dev-chat that you were probably the one who did the work related to it. It doesn't seem to be performed in the normal musicxml functions inside the fretboard diagram class after checking some debug output.
Reading is done by function MusicXMLParserPass2::frame() (for 3.x in importexport/musicxml/importmxmlpass2.cpp).
Writing by FretDiagram::writeMusicXML() (in libmscore/fret.cpp).
It seems FretDiagram::readMusicXML does not exist.
In reply to Reading is done by function… by Leon Vinken
Thanks for your response.
Weird, I can't seem to verify that FretDiagram::writeMusicXML() is being utilized when exporting to MusicXML. I put a qDebug() statement on its first line and it doesn't appear to output when exporting to MusicXML, nor does some code alteration affect the exported file when viewing it in a text file. Slightly perplexing.
I had code to fix this, but it didn't seem to work as I understood the functions presented as related to the above problem and I'll probably let it go for now. Logically speaking, this is very easy to fix (cancel the mutes when applying anything on first pass placing any element on a string upon import but make sure to use the 'add' flag for adding instead of replacing so that multiple dots work when 0 frets are present.
An example of discrepancy also (multiple dots:
MS3.6:
![mscore-diagram.png]( "mscore-diagram.png")
Imported MusicXML after export:
![xmldiagram.png]( "xmldiagram.png")
the uncompressed music xml shows that those dots are not present (weren't written upon export) rather than incorrectly imported.
Update: after rebasing I notice that the writeXML function is indeed going through the appropriate area now. I'll probably get a PR for this sometime soon (before 3.6.3 since 3.6.2 was released today)
https://github.com/musescore/MuseScore/pull/7455
1) Fixed extraneous mutes upon import
2) Allow for Open Marking + Fret diagrams upon import
3) Couldn't figure out how to stop some internal "style default" from overwriting the "Frets" attribute of the diagram. Again, the MusicXML is read/written correctly and forms the appropriate diagram with correct _frets attribute and appends to a segment, but afterwards somewhere somehow it's being reset to a stylistic default of "5". Any pointers about that would be cool.
Using the current 3.x (commit 7d1c732 by Joachim Schmitz on Sat Apr 3 07:51:02 2021 +0200) with minor changes to ease debugging, importing a MusicXML file containing a single fretboard results in four calls to FretDiagram::setFrets(int n), with n being 5, 4, 5 and 5. Relevant part of the stack traces below.
The first two calls are by the MusicXML importer itself, the last two by MuseScore's openScore() function calling styleChanged()
frame #0: 0x0000000100ff2efa mscore
Ms::FretDiagram::setFrets(this=0x0000000117e9a700, n=5) at fret.cpp:259Ms::FretDiagram::setProperty(this=0x0000000117e9a700, propertyId=FRET_FRETS, v=0x00007ffeefbf94f8) at fret.cpp:1388frame #1: 0x00000001010014ea mscore
frame #2: 0x000000010129bf78 mscore
Ms::ScoreElement::initElementStyle(this=0x0000000117e9a700, ss=0x0000000101c88120 size=8) at scoreElement.cpp:213Ms::FretDiagram::FretDiagram(this=0x0000000117e9a700, score=0x000000010cdc6200) at fret.cpp:55frame #3: 0x0000000100ff1233 mscore
frame #4: 0x0000000100ff12ed mscore
Ms::FretDiagram::FretDiagram(this=0x0000000117e9a700, score=0x000000010cdc6200) at fret.cpp:52Ms::MusicXMLParserPass2::frame(this=0x00007ffeefbfb2e8) at importmxmlpass2.cpp:4902frame #5: 0x0000000100e015d7 mscore
frame #6: 0x0000000100de4797 mscore
Ms::MusicXMLParserPass2::harmony(this=0x00007ffeefbfb2e8, partId=0x00007ffeefbfb068, measure=0x000000010af8a7a0, sTime=(_numerator = 0, _denominator = 1)) at importmxmlpass2.cpp:5145Ms::MusicXMLParserPass2::measure(this=0x00007ffeefbfb2e8, partId=0x00007ffeefbfb068, time=(_numerator = 0, _denominator = 1)) at importmxmlpass2.cpp:2017frame #7: 0x0000000100dde4ed mscore
frame #8: 0x0000000100ddbb54 mscore
Ms::MusicXMLParserPass2::part(this=0x00007ffeefbfb2e8) at importmxmlpass2.cpp:1684Ms::MusicXMLParserPass2::scorePartwise(this=0x00007ffeefbfb2e8) at importmxmlpass2.cpp:1573frame #9: 0x0000000100ddaf33 mscore
frame #10: 0x0000000100ddace3 mscore
Ms::MusicXMLParserPass2::parse(this=0x00007ffeefbfb2e8) at importmxmlpass2.cpp:1542Ms::MusicXMLParserPass2::parse(this=0x00007ffeefbfb2e8, device=0x00007ffeefbfb8c0) at importmxmlpass2.cpp:1523frame #11: 0x0000000100ddac48 mscore
frame #12: 0x0000000100d9bc2a mscore
Ms::importMusicXMLfromBuffer(score=0x000000010cdc6200, (null)=0x00007ffeefbfbcb0, dev=0x00007ffeefbfb8c0) at importmxml.cpp:57Ms::doValidateAndImport(score=0x000000010cdc6200, name=0x00007ffeefbfbcb0, dev=0x00007ffeefbfb8c0) at importxml.cpp:235frame #13: 0x0000000100e1f88f mscore
frame #14: 0x0000000100e1fb0c mscore
Ms::importMusicXml(score=0x000000010cdc6200, name=0x00007ffeefbfbcb0) at importxml.cpp:281Ms::readScore(score=0x000000010cdc6200, name=QString @ 0x00007ffeefbfbcb0, ignoreVersionError=false) at file.cpp:2353frame #15: 0x00000001002b0716 mscore
frame #16: 0x00000001002afc2b mscore
Ms::MuseScore::readScore(this=0x000000010c94de00, name=0x00007ffeefbfbf68) at file.cpp:473Ms::MuseScore::openScore(this=0x000000010c94de00, fn=0x00007ffeefbfbf68, switchTab=true, considerInCurrentSession=true, name=0x00007ffeefbfbf38) at file.cpp:415frame #17: 0x00000001002af43b mscore
frame #0: 0x0000000100ff2efa mscore
Ms::FretDiagram::setFrets(this=0x0000000117e9a700, n=4) at fret.cpp:259Ms::MusicXMLParserPass2::frame(this=0x00007ffeefbfb2e8) at importmxmlpass2.cpp:4920frame #1: 0x0000000100e019e4 mscore
frame #0: 0x0000000100ff2efa mscore
Ms::FretDiagram::setFrets(this=0x0000000117e9a700, n=5) at fret.cpp:259Ms::FretDiagram::setProperty(this=0x0000000117e9a700, propertyId=FRET_FRETS, v=0x00007ffeefbfb460) at fret.cpp:1388frame #1: 0x00000001010014ea mscore
frame #2: 0x000000010129fb47 mscore
Ms::ScoreElement::styleChanged(this=0x0000000117e9a700) at scoreElement.cpp:783Ms::updateStyle((null)=0x0000000000000000, e=0x0000000117e9a700) at score.cpp:1268frame #3: 0x0000000101255a7e mscore
frame #4: 0x0000000100ffe17c mscore
Ms::FretDiagram::scanElements(this=0x0000000117e9a700, data=0x0000000000000000, func=(mscoreMs::updateStyle(void*, Ms::Element) at score.cpp:1266), all=true)(void, Ms::Element), bool) at fret.cpp:1260frame #5: 0x0000000101305102 mscore
Ms::Segment::scanElements(this=0x0000000117ccfc00, data=0x0000000000000000, func=(mscoreMs::updateStyle(void, Ms::Element) at score.cpp:1266), all=true)(void, Ms::Element), bool) at segment.cpp:1195frame #6: 0x00000001011114a6 mscore
Ms::Measure::scanElements(this=0x000000010af8a7a0, data=0x0000000000000000, func=(mscoreMs::updateStyle(void, Ms::Element) at score.cpp:1266), all=true)(void, Ms::Element), bool) at measure.cpp:2621frame #7: 0x00000001012557a3 mscore
Ms::Score::scanElements(this=0x000000010cdc6200, data=0x0000000000000000, func=(mscoreMs::updateStyle(void, Ms::Element) at score.cpp:1266), all=true)(void, Ms::Element*), bool) at score.cpp:1862frame #8: 0x0000000101255999 mscore
Ms::Score::styleChanged(this=0x000000010cdc6200) at score.cpp:1279Ms::readScore(score=0x000000010cdc6200, name=QString @ 0x00007ffeefbfbcb0, ignoreVersionError=false) at file.cpp:2384frame #9: 0x00000001002b0d2a mscore
frame #10: 0x00000001002afc2b mscore
Ms::MuseScore::readScore(this=0x000000010c94de00, name=0x00007ffeefbfbf68) at file.cpp:473Ms::MuseScore::openScore(this=0x000000010c94de00, fn=0x00007ffeefbfbf68, switchTab=true, considerInCurrentSession=true, name=0x00007ffeefbfbf38) at file.cpp:415frame #11: 0x00000001002af43b mscore
frame #0: 0x0000000100ff2efa mscore
Ms::FretDiagram::setFrets(this=0x0000000117e9a700, n=5) at fret.cpp:259Ms::FretDiagram::setProperty(this=0x0000000117e9a700, propertyId=FRET_FRETS, v=0x00007ffeefbfb880) at fret.cpp:1388frame #1: 0x00000001010014ea mscore
frame #2: 0x000000010129fb47 mscore
Ms::ScoreElement::styleChanged(this=0x0000000117e9a700) at scoreElement.cpp:783Ms::updateStyle((null)=0x0000000000000000, e=0x0000000117e9a700) at score.cpp:1268frame #3: 0x0000000101255a7e mscore
frame #4: 0x0000000100ffe17c mscore
Ms::FretDiagram::scanElements(this=0x0000000117e9a700, data=0x0000000000000000, func=(mscoreMs::updateStyle(void*, Ms::Element) at score.cpp:1266), all=true)(void, Ms::Element), bool) at fret.cpp:1260frame #5: 0x0000000101305102 mscore
Ms::Segment::scanElements(this=0x0000000117ccfc00, data=0x0000000000000000, func=(mscoreMs::updateStyle(void, Ms::Element) at score.cpp:1266), all=true)(void, Ms::Element), bool) at segment.cpp:1195frame #6: 0x00000001011114a6 mscore
Ms::Measure::scanElements(this=0x000000010af8a7a0, data=0x0000000000000000, func=(mscoreMs::updateStyle(void, Ms::Element) at score.cpp:1266), all=true)(void, Ms::Element), bool) at measure.cpp:2621frame #7: 0x00000001012557a3 mscore
Ms::Score::scanElements(this=0x000000010cdc6200, data=0x0000000000000000, func=(mscoreMs::updateStyle(void, Ms::Element) at score.cpp:1266), all=true)(void, Ms::Element*), bool) at score.cpp:1862frame #8: 0x0000000101255999 mscore
Ms::Score::styleChanged(this=0x000000010cdc6200) at score.cpp:1279Ms::MuseScore::openScore(this=0x000000010c94de00, fn=0x00007ffeefbfbf68, switchTab=true, considerInCurrentSession=true, name=0x00007ffeefbfbf38) at file.cpp:440frame #9: 0x00000001002af8af mscore
@Leon Vinken, I appreciate your comment. Hopefully it leads to a fix for fret #'s not being imported correctly. Since 3.6 is no longer being developed according to the in-house team, I see no reason for expending any time fixing the problem in 3.x (my above mentioned PR never got merged for a 3.x release and might end up just floating) since it won't see the light of day, and I'm not sure if the XML stuff is fully within the master branch to begin correcting this specific issue. Hopefully whoever works on XML in the master branch can find this useful in the process.
The MusicXML import in master should still be mostly compatible with 3.x, so all 3.x fixes should be copied to master finally. Most likely a big part of that will be done by me. I do not yet consider PRs for 3.x a wasted effort, as finding the solution is typically more effort than copying it to another branch.
and 3.x is a better testbed for fixed like this, as there still all mtests are running.
Submitted separate issue for the missing chord symbol: #320487: Chord symbol invisible after importing due to style file read failure
See also https://github.com/musescore/MuseScore/pull/8014 (for master)
Submitted separate issue for the "Frets" problem, which is caused by a combination of an incorrect property flag and the 3.6 change to always call score->styleChanged() after loading a file. This results in overwriting Frets with the style default.
See #320573: [MusicXML import] fretboard frets property not saved in MuseScore format
Title updated for consistency with the PRs. Submitted a separate issue for the missing "G":
#320574: [MusicXML import] chord symbol missing from fretboard
Fixed in branch master, commit d74a1aaf89
Fix #316504: Update MusicXML Import of Fretboard Diagrams
Automatically closed -- issue fixed for 2 weeks with no activity.