Skip to content

Commit

Permalink
Fix import of default pedal
Browse files Browse the repository at this point in the history
When line and sign are not specified on a pedal element, they default
to "no" and "yes" respectively. Previously, this was handled by adding
Symbol elements for the "Ped." and "*" markings. These would layout
poorly and were inconsistent with pedal markings added to the score
manually.

This commit better handles this default case, and also makes the
corresponding changes to the export code to correctly handle such
pedal markings.
  • Loading branch information
iveshenry18 committed Aug 5, 2021
1 parent c5af641 commit 481fec6
Show file tree
Hide file tree
Showing 8 changed files with 416 additions and 71 deletions.
23 changes: 13 additions & 10 deletions importexport/musicxml/exportxml.cpp
Expand Up @@ -4377,6 +4377,8 @@ void ExportMusicXml::pedal(Pedal const* const pd, int staff, const Fraction& tic
_xml.stag("direction-type");
QString pedalType;
QString pedalXml;
QString signText;
QString lineText = pd->lineVisible() ? " line=\"yes\"" : " line=\"no\"";
if (pd->tick() == tick) {
switch (pd->beginHookType()) {
case HookType::HOOK_45:
Expand All @@ -4393,19 +4395,20 @@ void ExportMusicXml::pedal(Pedal const* const pd, int staff, const Fraction& tic
default:
pedalType = "start";
}
signText = pd->beginText() == "" ? " sign=\"no\"" : " sign=\"yes\"";
}
else {
switch (pd->endHookType()) {
// "change" type is handled only on the beginning of pedal lines
case HookType::NONE:
pedalType = "discontinue";
break;
default:
pedalType = "stop";
}
if (!pd->endText().isEmpty() || pd->endHookType() == HookType::HOOK_90)
pedalType = "stop";
else
pedalType = "discontinue";
// "change" type is handled only on the beginning of pedal lines

signText = pd->endText() == "" ? " sign=\"no\"" : " sign=\"yes\"";
}
pedalXml = QString("pedal type=\"%1\" line=\"yes\"").arg(pedalType);
pedalXml += pd->beginText() == "" ? " sign=\"no\"" : " sign=\"yes\"";
pedalXml = QString("pedal type=\"%1\"").arg(pedalType);
pedalXml += lineText;
pedalXml += signText;
pedalXml += positioningAttributes(pd, pd->tick() == tick);
_xml.tagE(pedalXml);
_xml.etag();
Expand Down
115 changes: 58 additions & 57 deletions importexport/musicxml/importmxmlpass2.cpp
Expand Up @@ -3939,72 +3939,73 @@ void MusicXMLParserDirection::pedal(const QString& type, const int /* number */,
const int number { 0 };
QStringRef line = _e.attributes().value("line");
QString sign = _e.attributes().value("sign").toString();
if (_placement == "") _placement = "below";

// We have found that many exporters omit "sign" even when one is originally present,
// therefore we will default to "yes", even though this is technically against the spec.
bool overrideDefaultSign = true; // TODO: set this flag based on the exporting software
if (sign == "") {
if (line != "yes" || overrideDefaultSign) sign = "yes"; // MusicXML 2.0 compatibility
else if (line == "yes") sign = "no"; // MusicXML 2.0 compatibility
}
if (line == "yes") {
auto& spdesc = _pass2.getSpanner({ ElementType::PEDAL, number });
if (type == "start" || type == "resume") {
if (spdesc._isStarted && !spdesc._isStopped) {
// Previous pedal unterminated—likely an unrecorded "discontinue", so delete the line.
// TODO: if "change", create 0-length spanner rather than delete
_pass2.deleteHandledSpanner(spdesc._sp);
spdesc._isStarted = false;
}
auto p = spdesc._isStopped ? toPedal(spdesc._sp) : new Pedal(_score);
if (type == "resume")
p->setBeginHookType(HookType::NONE);
else
if (sign == "yes") p->setBeginText("<sym>keyboardPedalPed</sym>");
else p->setBeginHookType(HookType::HOOK_90);
p->setEndHookType(HookType::NONE);
// if (placement == "") placement = "below"; // TODO ? set default
starts.append(MusicXmlSpannerDesc(p, ElementType::PEDAL, number));
}
else if (type == "stop" || type == "discontinue") {
auto p = spdesc._isStarted ? toPedal(spdesc._sp) : new Pedal(_score);
p->setEndHookType(type == "discontinue" ? HookType::NONE : HookType::HOOK_90);
stops.append(MusicXmlSpannerDesc(p, ElementType::PEDAL, number));
if (line != "yes" || (overrideDefaultSign && type == "start"))
sign = "yes"; // MusicXML 2.0 compatibility
else if (line == "yes")
sign = "no"; // MusicXML 2.0 compatibility
}
auto& spdesc = _pass2.getSpanner({ ElementType::PEDAL, number });
if (type == "start" || type == "resume") {
if (spdesc._isStarted && !spdesc._isStopped) {
// Previous pedal unterminated—likely an unrecorded "discontinue", so delete the line.
// TODO: if "change", create 0-length spanner rather than delete
_pass2.deleteHandledSpanner(spdesc._sp);
spdesc._isStarted = false;
}
auto p = spdesc._isStopped ? toPedal(spdesc._sp) : new Pedal(_score);
if (line == "yes") p->setLineVisible(true);
else p->setLineVisible(false);
if (!p->lineVisible() || sign == "yes") {
p->setBeginText("<sym>keyboardPedalPed</sym>");
p->setContinueText("(<sym>keyboardPedalPed</sym>)");
}
else if (type == "change") {
// pedal change is implemented as two separate pedals
// first stop the first one
if (spdesc._isStarted && !spdesc._isStopped) {
auto p = toPedal(spdesc._sp);
p->setEndHookType(HookType::HOOK_45);
stops.append(MusicXmlSpannerDesc(p, ElementType::PEDAL, number));
}
else
_logger->logError(QString("\"change\" type pedal created without existing pedal"), &_e);
// then start a new one
auto p = new Pedal(_score);
p->setBeginHookType(HookType::HOOK_45);
p->setEndHookType(HookType::HOOK_90);
starts.append(MusicXmlSpannerDesc(p, ElementType::PEDAL, number));
}
else if (type == "continue") {
// ignore
else {
p->setBeginHookType(type == "resume" ? HookType::NONE : HookType::HOOK_90);
}
else
qDebug("unknown pedal type %s", qPrintable(type));
p->setEndHookType(HookType::NONE);
starts.append(MusicXmlSpannerDesc(p, ElementType::PEDAL, number));
}
else {
// TBD: what happens when an unknown pedal type is found ?
Symbol* s = new Symbol(_score);
s->setAlign(Align::LEFT | Align::BASELINE);
//s->setOffsetType(OffsetType::SPATIUM);
if (type == "start")
s->setSym(SymId::keyboardPedalPed);
else if (type == "stop")
s->setSym(SymId::keyboardPedalUp);
else if (type == "stop" || type == "discontinue") {
auto p = spdesc._isStarted ? toPedal(spdesc._sp) : new Pedal(_score);
if (line == "yes") p->setLineVisible(true);
else if (line == "no") p->setLineVisible(false);
if (!p->lineVisible() || sign == "yes")
p->setEndText("<sym>keyboardPedalUp</sym>");
else
_logger->logError(QString("unknown pedal type %1").arg(type), &_e);
_elems.append(s);
p->setEndHookType(type == "discontinue" ? HookType::NONE : HookType::HOOK_90);
stops.append(MusicXmlSpannerDesc(p, ElementType::PEDAL, number));
}
else if (type == "change") {
// pedal change is implemented as two separate pedals
// first stop the first one
if (spdesc._isStarted && !spdesc._isStopped) {
auto p = toPedal(spdesc._sp);
p->setEndHookType(HookType::HOOK_45);
if (line == "yes") p->setLineVisible(true);
else if (line == "no") p->setLineVisible(false);
stops.append(MusicXmlSpannerDesc(p, ElementType::PEDAL, number));
}
else
_logger->logError(QString("\"change\" type pedal created without existing pedal"), &_e);
// then start a new one
auto p = new Pedal(_score);
p->setBeginHookType(HookType::HOOK_45);
p->setEndHookType(HookType::HOOK_90);
if (line == "yes") p->setLineVisible(true);
else p->setLineVisible(false);
starts.append(MusicXmlSpannerDesc(p, ElementType::PEDAL, number));
}
else if (type == "continue") {
// ignore
}
else
qDebug("unknown pedal type %s", qPrintable(type));

_e.skipCurrentElement();
}
Expand Down
Binary file modified mtest/musicxml/io/testDirections1.pdf
Binary file not shown.

0 comments on commit 481fec6

Please sign in to comment.