Stack trace 100% identical as in #105436: SVG export crashes MuseScore with files containing horizontal frames so very likely a duplicate or at least the same root cause. Here too the Crash doesn't happen when forcing byMeasure to true, i.e. not using the optimized code for creating all staves lines in one go rather than measure by measure
Very strange, just before calling saveSVG() the score gets switched to page mode (and back to continuous mode after), so this mode shouldn't be the issue at all?!?
Hah! In continuous view it is the vertical frame (shown in page mode only) that causes the crash!
So something goes wrong when that temporary switching to page mode happens.
Would you like my help on this?
I can't tell yet if this has anything to do with my code changes.
The SVG Export has always iterated over the score elements by page, and I have maintained that in my changes. I believe it does this because it is interested in the Page object's version of systems and staves, not the Score object's version. The Page version is the visually oriented version, and is in the proper paint order, etc.
Page mode is an odd thing relative to SVG Export: SVG doesn't have any concept of pages, so the export simply increments a x-axis offset and starts painting the next "page" on the same canvas. They are horizontally-spaced pseudo-pages. SVG really is "continuous mode" in its core implementation.
As far as the byMeasure code, I kept it very simple in structure, so if that's the problem it should be easy to rectify.
I have looked at it yesterday, for hours, and came up with a fix, just that fix doesn't work and I don't understand why. Will publish my repo later today and place a link here for others to see and comment.
Forcing byMeasure to true does fix it too, but my code tried to detect whether it has to be forced (because there is a vertical or horizontal frame) and that detection didn't work.
What is VBOX/HBOX's parent element? I don't see a parent() override in libmscore/box.h
The code is iterating by system, and all the other types of LAYOUT_BREAK cause the system to break. So maybe you should simply check for a LAYOUT_BREAK within this system in an alternate way, prior to the existing code for byMeasure.
I just saw your latest update, and just looked at your code. So the MeasureBase itself is an Element::Type of HBOX|VBOX? I don't see how that works inside MeasureBase.cpp/.h. Are you sure that logic is correct? Then again I don't yet see how HBOX and VBOX types are applied or returned at all (yet...) Otherwise the logic and placement of code seems correct to me:
You need to check for yet another condition where you set byMeasure = true, in addition to invisible measures.
Pretty simple. If the HBOX/VBOX is not a measure itself, then at least one more for loop will be required. But all that's relatively obvious too. At this moment I understand this less than you do, so I don't know if I'm being helpful here.
I think this is a problem for page mode too, it just doesn't crash. If there is a break in the staff lines within the system, byMeasure should be true regardless of the page/continuous mode. I imagine that if this is in page mode the exported SVG does not break the staff lines within the system, as it should.
I see you changed cs to score, line 2517, thanks for catching that. I had left it as a general open issue - it's confusing the way cs and score are used in general.
(another edit: To be absolutely clear:
- This issue has nothing to do with continuous view or page view modes. It is purely an issue relating to HBOX and VBOX element types (vertical and horizontal frames). The HBOX is always an issue because it never breaks the system. The VBOX is only an issue in continuous view, when it is invisible, because then the system doesn't break.)
(code edited - slight improvement to logic) I have found the problem: the check for HBOX and VBOX needs to happen prior to the check for .visible(). Apparently the HBOX and VBOX MeasureBase objects don't handle the visible() property well. Here is sample code:{syntaxhighlighter SPEC}
bool byMeasure = false;
for (MeasureBase* mb = s->firstMeasure(); mb != 0; mb = s->nextMeasure(mb)) {
if (mb->type() == Element::Type::HBOX
|| mb->type() == Element::Type::VBOX
|| !static_cast(mb)->visible(i)) {
byMeasure = true;
break;
}
}
if (byMeasure) { // Draw visible staff lines by measure
for (MeasureBase* mb = s->firstMeasure(); mb != 0; mb = s->nextMeasure(mb)) {
if (mb->type() != Element::Type::HBOX
&& mb->type() != Element::Type::VBOX
&& mb->visible()) {
Measure* m = static_cast(mb);
StaffLines* sl = m->staffLines(i);
printer.setElement(sl);
paintElement(p, sl);
}
}
}
{/syntaxhighlighter}
If you want, I can include this is my pending pull request, otherwise feel free to copy/paste from the code above.
I somehow had the feeling the order matters, my suspicion was the cast, but didn't go the extra step to just try it.
Glad that my logic wasn't completely screwed.
The truly bad thing is that a MeasureBase of type HBOX or VBOX crashes when you check its .visible() property. Maybe that should be fixed at the source too, even if it always returns false or something kludgy like that. I'll update my PR later today to fix SVG Export, and I'll try to remember to indent it properly :-). Do I need to include this issue number in the PR somehow? I'll included it in the check-in description.
False is definitely the choice if you're going to hard code a value. These are not printable elements anyway, so as far as printing/exporting is concerned, they are always invisible.
If it's fixable by setting a value in the constructor, that sounds like a good fix to me. I think that's a fix you should implement, since it's outside my area of expertise in MuseScore, and I've never touched that file.
If you make this change, then the fix/change for this issue goes away, and there is no need to change file.cpp. What do you think about that?
edit - The only question/concern that comes to my mind relates to the way MuseScore displays/hides these BOX elements via the View menu options. But if that code used the .visible property, it would be crashing too, right?
I just reverted my code and got it to crash again. The problem is in measure.cpp, line 2425, Measure::visible(), and the problem is that the Measure's list of staves is empty (measure.h, line 127, the "staves" class variable). Of course, BOX elements don't have staves. So I suggest the following fix:{syntaxhighlighter SPEC}
bool Measure::visible(int staffIdx) const
{
if (staffIdx >= score()->staves().size()) {
qDebug("Measure::visible: bad staffIdx: %d", staffIdx);
return false;
}
if (system() && (system()->staves()->isEmpty() || !system()->staff(staffIdx)->show()))
return false;
if (score()->staff(staffIdx)->cutaway() && isMeasureRest(staffIdx))
return false;
// NEW IF STATEMENT HERE
if (staves.size() == 0)
return false;
return score()->staff(staffIdx)->show() && staves[staffIdx]->_visible;
}{/syntaxhighlighter}
Or something along those lines. It could be included as an OR condition inside one of the existing if() statements if you prefer.
After further debugging...
Both the Box class and Measure class inherit from MeasureBase. MeasureBase does not have a .visible() property. Somehow MeasureBase objects are using the Measure class's .visible() property, even if they do not inherit from Measure. So an object of class Box is somehow calling the Measure class's ::visible() function and it doesn't have one of the class variables that Measure has and uses in that function.
My C++ is still rusty enough that I don't see how calling MeasureBase::visible() results in calling a sub-class's ::visible() function, and I don't see how to resolve this except by either:
a) creating a separate MeasureBase::visible(), which might break other things
b) basic kludge: add a QList staves variable to class Box. It will only be used to avoid crashing in this situation.
...and no, it is not the Score's staves variable or function, it is the Measure class's staves variable in measure.h.
In Measure::visibile(): Instead of checking for staves.size(), checking the Element::Type seems to work:{syntaxhighlighter SPEC}
if (type() == Element::Type::HBOX || type() == Element::Type::VBOX)
return false;{/syntaxhighlighter}
I just tested it and though the debugger doesn't show me the correct value for type(), the code runs properly.
New code:{syntaxhighlighter SPEC}
if (mb->visible()) {
Measure* m = static_cast(mb);
StaffLines* sl = m->staffLines(i);
printer.setElement(sl);
paintElement(p, sl);
}
{/syntaxhighlighter}
If byMeasure is true, the Measure* variable gets moved inside the check on mb->visible(). That plus the change to Measure::visible() solve the problem for me with these test files. Still the execution flow here is strange, as mentioned below, but I'm not familiar with the MuseScore policy on unsolved mysteries, and at least this works.
original post/reply on subject of Box, Measure, and MeasureBase classes interacting strangely re: ::visible()
Then why does this file.cpp code in saveSVG() end up calling Measure::visible() every time?
I agree. I think that this line of code in the svg'ing of staff lines should be calling Element::visible(), not Measure::visible(), especially if the MeasureBase object is a Box, not a Measure.
But regardless of whether that mystery is ever solved, those two lines of code work properly, and they seem like the best fix to me. What do you think?
I just saw your note about it not working. Here is my test file and the resulting svg. Ah, but I still have code in file.cpp too, after the initial byMeasure - that's the problem... please hold.
I just reviewed my git situation and after various edits these two changes are the only ones in my open PR branch. Looks like I reverted my other changes properly. So if you like this solution I can check in those changes and they'll be bundled in that PR.
...assuming you'll give me the green light, I've also re-indented the entire procedure for 6 spaces, as all the parts I had rewritten previously were improperly spaced. It's a small enough procedure that it wasn't a big deal, and these lines really were inconsistent all around. I can check in the code change separately, so your git compares will be simpler (you'll have a revision where there are only two lines of code changed in file.cpp, then a subsequent revision with all the indentation changes)
So it's all ready to check in if/when you want to do so.
Measure::visible(int) is very different from Element::visible(). The latter just returns the _visible member. The former is checking to see if the measure is visible *on that particular staff*, which has to do with things like, whether that measure was marked invisible for that staff in Measure Properties, but also whether it is made invisible because of "Hide empty staves", or - for the master, anyhow - whether it is invisible because it is an empoty measure and the staff is marked as a cutaway staff.
So no, I don;t think that propsoed change would do the right thing at all - it would inappropriately draw the staff lines for measures that are supposed to be invisible *for the particular staff*. That check needs to be in place in some form or other.
Forgive the multiple edits, but I'm rethinking the concept of what is the best solution.
If MeasureBase::visible(idxStaff) doesn't exist, I should not be calling it, even if the compiler lets me and for most cases it seems to function normally.
If that is the case, then the code changes should all be inside file.cpp, and they should all involve checking the Element::Type == H/VBOX prior to checking for visibility. All of a sudden that seems like the proper way to go.
(original reply)
Yes, you're right, visible(int) is different from visible(). But that still doesn't explain how an object of class Box is calling a function inside a class to which it is not related. Why is MeasureBase calling a sub-class's function when it shouldn't be? Maybe the problem is that the code shouldn't be calling a function that doesn't exist for this class... Maybe that part of this needs to be rethought...
As for the solution, I don't see how the negative result you describe would occur.
The changes inside measure.cpp only affect elements of type HBOX and VBOX. If any other code currently calls Measure::visible() from a Box object, MuseScore will crash. I foresee no negative side effects from this change in measure.cpp.
As for the change in file.cpp, that is the same logic, only better stated.
You'll need to be more specific if you still believe there is a negative outcome here.
I just checked-in the changes to file.cpp that fix this issue, essentially my original changes. Oh well, when the compiler lets you do something it's easy to forget that it might not be legit.
I did not separate the indentation changes from the substantive code changes in github, but I think we've discussed this enough that the changes should be clear. If not, let me know.
isn't called just for Box elements but for *any* measure. So if you replace the visible(i) with just visible() - which is what I thought you said you wanted to do - it will yield the wrong for staves that are only invisible on some staves but not others. You'll either get staff lines for all staves or none.
Which is to say: the change you are proposing might fix the Box case, but it will break the normal measure case if the measure is visible on some staves but not others. At least that's how it seems to me.
You misunderstood, I was not going to replace visible(i) with visible() anywhere.
But you helped me clarify that I was doing something off-kilter, so it all worked out. The changes in the pull request do the right thing, only calling Measure::visible(idxStaff) from a properly cast pointer to a Measure object, and only when that object is not an HBOX or VBOX (which can't properly be cast as Measure).
For the record, I was talking about response #19, which sure likes like it proposing eliminating "i" from the call to visible(). Maybe that was just a typo?
Comments
This issue appears around January 21/22, 2016.
- Works with this Nighlty: d54fb01
- Fails with this one: c807c0d
Not sure (verify), but the most likely here?: https://github.com/musescore/MuseScore/pull/2351/files
Stack trace 100% identical as in #105436: SVG export crashes MuseScore with files containing horizontal frames so very likely a duplicate or at least the same root cause. Here too the Crash doesn't happen when forcing byMeasure to true, i.e. not using the optimized code for creating all staves lines in one go rather than measure by measure
Very strange, just before calling saveSVG() the score gets switched to page mode (and back to continuous mode after), so this mode shouldn't be the issue at all?!?
Hah! In continuous view it is the vertical frame (shown in page mode only) that causes the crash!
So something goes wrong when that temporary switching to page mode happens.
Would you like my help on this?
I can't tell yet if this has anything to do with my code changes.
The SVG Export has always iterated over the score elements by page, and I have maintained that in my changes. I believe it does this because it is interested in the Page object's version of systems and staves, not the Score object's version. The Page version is the visually oriented version, and is in the proper paint order, etc.
Page mode is an odd thing relative to SVG Export: SVG doesn't have any concept of pages, so the export simply increments a x-axis offset and starts painting the next "page" on the same canvas. They are horizontally-spaced pseudo-pages. SVG really is "continuous mode" in its core implementation.
As far as the byMeasure code, I kept it very simple in structure, so if that's the problem it should be easy to rectify.
I have looked at it yesterday, for hours, and came up with a fix, just that fix doesn't work and I don't understand why. Will publish my repo later today and place a link here for others to see and comment.
Forcing byMeasure to true does fix it too, but my code tried to detect whether it has to be forced (because there is a vertical or horizontal frame) and that detection didn't work.
Edit: see https://github.com/Jojo-Schmitz/MuseScore/tree/svg, esp. https://github.com/Jojo-Schmitz/MuseScore/commit/aa61b7356210c744cd6f6b…
What is VBOX/HBOX's parent element? I don't see a parent() override in libmscore/box.h
The code is iterating by system, and all the other types of LAYOUT_BREAK cause the system to break. So maybe you should simply check for a LAYOUT_BREAK within this system in an alternate way, prior to the existing code for byMeasure.
I just saw your latest update, and just looked at your code. So the MeasureBase itself is an Element::Type of HBOX|VBOX? I don't see how that works inside MeasureBase.cpp/.h. Are you sure that logic is correct? Then again I don't yet see how HBOX and VBOX types are applied or returned at all (yet...) Otherwise the logic and placement of code seems correct to me:
You need to check for yet another condition where you set byMeasure = true, in addition to invisible measures.
Pretty simple. If the HBOX/VBOX is not a measure itself, then at least one more for loop will be required. But all that's relatively obvious too. At this moment I understand this less than you do, so I don't know if I'm being helpful here.
I think this is a problem for page mode too, it just doesn't crash. If there is a break in the staff lines within the system, byMeasure should be true regardless of the page/continuous mode. I imagine that if this is in page mode the exported SVG does not break the staff lines within the system, as it should.
I see you changed cs to score, line 2517, thanks for catching that. I had left it as a general open issue - it's confusing the way cs and score are used in general.
(another edit: To be absolutely clear:
- This issue has nothing to do with continuous view or page view modes. It is purely an issue relating to HBOX and VBOX element types (vertical and horizontal frames). The HBOX is always an issue because it never breaks the system. The VBOX is only an issue in continuous view, when it is invisible, because then the system doesn't break.)
(code edited - slight improvement to logic) I have found the problem: the check for HBOX and VBOX needs to happen prior to the check for .visible(). Apparently the HBOX and VBOX MeasureBase objects don't handle the visible() property well. Here is sample code:{syntaxhighlighter SPEC}
bool byMeasure = false;
for (MeasureBase* mb = s->firstMeasure(); mb != 0; mb = s->nextMeasure(mb)) {
if (mb->type() == Element::Type::HBOX
|| mb->type() == Element::Type::VBOX
|| !static_cast(mb)->visible(i)) {
byMeasure = true;
break;
}
}
if (byMeasure) { // Draw visible staff lines by measure
for (MeasureBase* mb = s->firstMeasure(); mb != 0; mb = s->nextMeasure(mb)) {
if (mb->type() != Element::Type::HBOX
&& mb->type() != Element::Type::VBOX
&& mb->visible()) {
Measure* m = static_cast(mb);
StaffLines* sl = m->staffLines(i);
printer.setElement(sl);
paintElement(p, sl);
}
}
}
{/syntaxhighlighter}
If you want, I can include this is my pending pull request, otherwise feel free to copy/paste from the code above.
OK, go ahead with your PR.
I somehow had the feeling the order matters, my suspicion was the cast, but didn't go the extra step to just try it.
Glad that my logic wasn't completely screwed.
The truly bad thing is that a MeasureBase of type HBOX or VBOX crashes when you check its .visible() property. Maybe that should be fixed at the source too, even if it always returns false or something kludgy like that. I'll update my PR later today to fix SVG Export, and I'll try to remember to indent it properly :-). Do I need to include this issue number in the PR somehow? I'll included it in the check-in description.
Guess in the contructor of BOX should set _visible to something, I guess false is the most obvious choice?
False is definitely the choice if you're going to hard code a value. These are not printable elements anyway, so as far as printing/exporting is concerned, they are always invisible.
If it's fixable by setting a value in the constructor, that sounds like a good fix to me. I think that's a fix you should implement, since it's outside my area of expertise in MuseScore, and I've never touched that file.
If you make this change, then the fix/change for this issue goes away, and there is no need to change file.cpp. What do you think about that?
edit - The only question/concern that comes to my mind relates to the way MuseScore displays/hides these BOX elements via the View menu options. But if that code used the .visible property, it would be crashing too, right?
I tried and it doesn't help
I just reverted my code and got it to crash again. The problem is in measure.cpp, line 2425, Measure::visible(), and the problem is that the Measure's list of staves is empty (measure.h, line 127, the "staves" class variable). Of course, BOX elements don't have staves. So I suggest the following fix:{syntaxhighlighter SPEC}
bool Measure::visible(int staffIdx) const
{
if (staffIdx >= score()->staves().size()) {
qDebug("Measure::visible: bad staffIdx: %d", staffIdx);
return false;
}
if (system() && (system()->staves()->isEmpty() || !system()->staff(staffIdx)->show()))
return false;
if (score()->staff(staffIdx)->cutaway() && isMeasureRest(staffIdx))
return false;
// NEW IF STATEMENT HERE
if (staves.size() == 0)
return false;
return score()->staff(staffIdx)->show() && staves[staffIdx]->_visible;
}{/syntaxhighlighter}
Or something along those lines. It could be included as an OR condition inside one of the existing if() statements if you prefer.
Guess it should be `if (score()->staves().size() == 0)`? Hmm, maybe not.
Whatever, it doesn't fix the crash
After further debugging...
Both the Box class and Measure class inherit from MeasureBase. MeasureBase does not have a .visible() property. Somehow MeasureBase objects are using the Measure class's .visible() property, even if they do not inherit from Measure. So an object of class Box is somehow calling the Measure class's ::visible() function and it doesn't have one of the class variables that Measure has and uses in that function.
My C++ is still rusty enough that I don't see how calling MeasureBase::visible() results in calling a sub-class's ::visible() function, and I don't see how to resolve this except by either:
a) creating a separate MeasureBase::visible(), which might break other things
b) basic kludge: add a QList staves variable to class Box. It will only be used to avoid crashing in this situation.
...and no, it is not the Score's staves variable or function, it is the Measure class's staves variable in measure.h.
In Measure::visibile(): Instead of checking for staves.size(), checking the Element::Type seems to work:{syntaxhighlighter SPEC}
if (type() == Element::Type::HBOX || type() == Element::Type::VBOX)
return false;{/syntaxhighlighter}
I just tested it and though the debugger doesn't show me the correct value for type(), the code runs properly.
MeasureBase inherits the _visible property and the visible() and setVisible() methods from Element.
And that change still doesn't help, it still crashes
Here is the additional change required in file.cpp:
Original code:
https://github.com/musescore/MuseScore/blob/master/mscore/file.cpp#L256…
New code:{syntaxhighlighter SPEC}
if (mb->visible()) {
Measure* m = static_cast(mb);
StaffLines* sl = m->staffLines(i);
printer.setElement(sl);
paintElement(p, sl);
}
{/syntaxhighlighter}
If byMeasure is true, the Measure* variable gets moved inside the check on mb->visible(). That plus the change to Measure::visible() solve the problem for me with these test files. Still the execution flow here is strange, as mentioned below, but I'm not familiar with the MuseScore policy on unsolved mysteries, and at least this works.
original post/reply on subject of Box, Measure, and MeasureBase classes interacting strangely re: ::visible()
Then why does this file.cpp code in saveSVG() end up calling Measure::visible() every time?
I agree. I think that this line of code in the svg'ing of staff lines should be calling Element::visible(), not Measure::visible(), especially if the MeasureBase object is a Box, not a Measure.
But regardless of whether that mystery is ever solved, those two lines of code work properly, and they seem like the best fix to me. What do you think?
I just saw your note about it not working. Here is my test file and the resulting svg. Ah, but I still have code in file.cpp too, after the initial byMeasure - that's the problem... please hold.
I just reviewed my git situation and after various edits these two changes are the only ones in my open PR branch. Looks like I reverted my other changes properly. So if you like this solution I can check in those changes and they'll be bundled in that PR.
...assuming you'll give me the green light, I've also re-indented the entire procedure for 6 spaces, as all the parts I had rewritten previously were improperly spaced. It's a small enough procedure that it wasn't a big deal, and these lines really were inconsistent all around. I can check in the code change separately, so your git compares will be simpler (you'll have a revision where there are only two lines of code changed in file.cpp, then a subsequent revision with all the indentation changes)
So it's all ready to check in if/when you want to do so.
Measure::visible(int) is very different from Element::visible(). The latter just returns the _visible member. The former is checking to see if the measure is visible *on that particular staff*, which has to do with things like, whether that measure was marked invisible for that staff in Measure Properties, but also whether it is made invisible because of "Hide empty staves", or - for the master, anyhow - whether it is invisible because it is an empoty measure and the staff is marked as a cutaway staff.
So no, I don;t think that propsoed change would do the right thing at all - it would inappropriately draw the staff lines for measures that are supposed to be invisible *for the particular staff*. That check needs to be in place in some form or other.
Forgive the multiple edits, but I'm rethinking the concept of what is the best solution.
If MeasureBase::visible(idxStaff) doesn't exist, I should not be calling it, even if the compiler lets me and for most cases it seems to function normally.
If that is the case, then the code changes should all be inside file.cpp, and they should all involve checking the Element::Type == H/VBOX prior to checking for visibility. All of a sudden that seems like the proper way to go.
(original reply)
Yes, you're right, visible(int) is different from visible(). But that still doesn't explain how an object of class Box is calling a function inside a class to which it is not related. Why is MeasureBase calling a sub-class's function when it shouldn't be? Maybe the problem is that the code shouldn't be calling a function that doesn't exist for this class... Maybe that part of this needs to be rethought...
As for the solution, I don't see how the negative result you describe would occur.
The changes inside measure.cpp only affect elements of type HBOX and VBOX. If any other code currently calls Measure::visible() from a Box object, MuseScore will crash. I foresee no negative side effects from this change in measure.cpp.
As for the change in file.cpp, that is the same logic, only better stated.
You'll need to be more specific if you still believe there is a negative outcome here.
I just checked-in the changes to file.cpp that fix this issue, essentially my original changes. Oh well, when the compiler lets you do something it's easy to forget that it might not be legit.
So this is now included in my open PR here: https://github.com/musescore/MuseScore/pull/2542
I did not separate the indentation changes from the substantive code changes in github, but I think we've discussed this enough that the changes should be clear. If not, let me know.
Well, I haven't really studied issue, and maybe I'm misunderstanding what you're proposing. But it sure looks to me like the code in question:
https://github.com/musescore/MuseScore/blob/master/mscore/file.cpp#L2569
isn't called just for Box elements but for *any* measure. So if you replace the visible(i) with just visible() - which is what I thought you said you wanted to do - it will yield the wrong for staves that are only invisible on some staves but not others. You'll either get staff lines for all staves or none.
Which is to say: the change you are proposing might fix the Box case, but it will break the normal measure case if the measure is visible on some staves but not others. At least that's how it seems to me.
You misunderstood, I was not going to replace visible(i) with visible() anywhere.
But you helped me clarify that I was doing something off-kilter, so it all worked out. The changes in the pull request do the right thing, only calling Measure::visible(idxStaff) from a properly cast pointer to a Measure object, and only when that object is not an HBOX or VBOX (which can't properly be cast as Measure).
For the record, I was talking about response #19, which sure likes like it proposing eliminating "i" from the call to visible(). Maybe that was just a typo?
yes, just a typo. it was a confused moment in time all around :-)
New PR just submitted here: https://github.com/musescore/MuseScore/pull/2552
Fixed in branch master, commit df947b3873
Fix #105436 Fix #105471: svg regressions
Fixed in branch master, commit d68ac1e9b1
Merge pull request #2552 from sidewayss/#105436-#105471-saveSVG-cleanup
Fix #105436 Fix #105471 save svg cleanup
Fixed in branch 2.0.4, commit e1b5c25a95
Fix #105436 Fix #105471: svg regressions
Also fixed in 2.0.4 branch e1b5c25a95
Automatically closed -- issue fixed for 2 weeks with no activity.