Natural accidentals are not remembered within measure

[chbauman]

When there is an accidental, it is not remembered for subsequent notes within the same measure.
This can leads to redundant naturals or missing sharps / flats.

As an example, the following was rendered by MuseScore:

Now when rendered by OSMD it looks as follows:

Which is not correct.

[chbauman]

The relevant musicxml file is in the following .zip:

Debug.zip

(Attaching .musicxml directly is now working)

[sschmidTU]

Actually your sample is working fine for me (without transposition):

What settings and version of OSMD did you use to get your result?
I would also guess that this was the same before our release of 1.7.6 today, because the changes shouldn't have affected transposition, but I could be wrong of course.

[chbauman]

I can still reproduce the original issue with version opensheetmusicdisplay@1.7.6.

Did you use the pull request branch? Because there I added some more changes that fixed some of the newly introduced problems with the first commits.

But I think there was still the issue with the double natural in "Dichterliebe" that remained and I did not have the time yet to look further into that.

I extended my debugging score to the following: (MuseScore)

OSMD 1.7.6:

Branch chbauman:patch-1:

[sschmidTU]

I meant that I can't reproduce it in the public demo, which is base OSMD 1.7.6.

https://opensheetmusicdisplay.github.io/demo/

[chbauman]

Oh you're right it looks like this:

There is still a redundant flat tho.

But I guess there may be issues on my application that uses OSMD and not OSMD itself.

[sschmidTU]

Do you use Transpose? There's an issue where sometimes accidentals change if you change Transpose after render()

[fablau]

Simon, I think the problem appears when you transpose the music, let's say to -2, then revert back to 0. In that case, all accidentals are marked on each note, even though they are in the key signatures. Naturals are always marked.

Look at this example, the first shot below is how the music is shown by default:

Here below, the same music is transposed correctly one tone lower (-2):

Then, I transpose it back to its original key (tranposition to 0):

Is that something that can be fixed?

[sschmidTU]

Right, that makes sense, good find.
I mean, you can work around it by just loading it again when you switch back to the original key signature. Since load() takes almost no time, it's not really a drawback other than having to implement the workaround.
As for the root causes, it would need a deeper look. The transposing and accidental code is tricky, I've tried to improve it a few times in the past. It's hard to catch all edge cases.

[fablau]

I understand. The only problem with "reloading" the file is that if you have hidden any instruments from the score, they'll reappear, and you may need to hide them again. And I think the same would happen to any other modification you may have done to the score. So the operation may become cumbersome. It'd be nice to have the transposing feature to just work correctly without having to reload the file ;)

[ammatwain]

Hi @sschmidTU , hi @fablau !
By removing the condition "transposeHalftones !== 0" from the function "createGraphicalMeasure" and function "checkNoteForAccidental" in the file MusicSheetCalculator.ts, this workaround is no longer necessary, provided that a TransposeCalculator handles it correctly.
You see, the crucial point for me is this:
In particular (in createGraphicalMeasure) in the group of conditions where the value of "transposeHalftones" is evaluated, it is also checked whether TransposeCalculator is an existing object.
Well, I believe that when the TransposeCalculator is brought into play, the TransposeCalculator must dance 🚀.
By the way, this modification is present in the pull request #1420 of the ExtendedTransposeCalculator in my fork :)

[fablau]

@ammatwain you are absolutely right! I just tried to remove that clause and it works just fine! I hope there are not "side effects" by removing that check.... @sschmidTU can you confirm that? This would solve a great deal of problems ;)

Thank you!

[ammatwain]

Grazie a te, Fabrizio!
Okay, let me tell you the problems I encountered while working on my TransposeCalculator extension.

The main idea was to make it work by directly passing the key signature as a value (then I added octaves as requested by Simon). The issue was transposing from key "x" to key C (zero). In this case, TransposeCalculator didn't react.

So, I started "dirtying up" the values passed to Transpose. Instead of passing 0, I passed 0.00000000000001, instead of 1, I passed 1.00000000000001, and so on. This way, the OSMD routines always triggered TransposeCalculator. (Of course, the methods of TransposeCalculator cleaned up the value with Math.floor() in the first instruction).

Essentially, this was a workaround to bypass the condition checks I found in three routines (hopefully, there are no others). Here they are listed and commented:

MusicSheetCalculator.createGraphicalMeasure() {
 ...
   if ( /* transposeHalftones !== 0 && */
 ...

MusicSheetCalculator.checkNoteForAccidental() {
...
   if (/* transposeHalftones !== 0 && */
   graphicalNote.sourceNote.ParentStaffEntry.ParentStaff.ParentInstrument.MidiInstrumentId !== MidiInstrument.Percussion
   // On this line, it's me adding the condition that TransposeCalculator exists.
   && MusicSheetCalculator.transposeCalculator
   ) {
...

MusicSystemBuilder.transposeKeyInstruction() {
...
        if (/* transposeHalftones !== keyInstruction.isTransposedBy && */
...

In the latest version I'm working on, I removed the "dirtying up" from the value passed to Transpose and also removed the conditions that were blocking TransposeCalculator.

Certainly, those conditions were put there cautiously, probably to allow the sheet to regenerate without TransposeCalculator's supervision. However, as you can see, they end up causing problems themselves.

P.S. I have updated the demo of ExtendedTransposeCalculator to version 1.8.2 of OSMD and with the modifications described above. If you want to test it, I remind you that the link is:
https://ammatwain.github.io/extended-transpose-calculator/

Until we write again :)
Amedeo

[ammatwain]

Hello @sschmidTU , is there a variable I can use to know that the score has just been loaded and it's the first time it has been rendered? Something like firstRenderCompleted: boolean?

[sschmidTU]

@ammatwain No, unfortunately not, because usually it doesn't matter and the developer can easily track it, unless of course you're within OSMD. So, we could add such a variable. I would instead make it renderCount: number though, a bit more flexible.

[ammatwain]

renderCount: number would be perfect and could be used in various situations. I'm asking you this because the idea of letting the renderer work in peace during the initial loading, without the intervention of TransposeCalculator, is good and absolutely justified!

[sschmidTU]

agreed! I'll think about where to put it. Probably in osmd.graphic. Or EngravingRules, since that's what almost all objects have access to, unlike graphic.

[ammatwain]

I was thinking that renderCount could be useful to be accessible to the three functions mentioned above:

MusicSheetCalculator.createGraphicalMeasure()
MusicSheetCalculator.checkNoteForAccidental()
MusicSystemBuilder.transposeKeyInstruction()

This way, these functions would have something consistent to prevent TransposeCalculator from taking action!

And, of course, it will also be easily accessible to the user/programmer, who may want to use it for various reasons!

Thank you, Simon!

[fablau]

Awesome stuff guys! And Great job with the "ETC Transpose Calculator" @ammatwain, I love it ;)

[ammatwain]

Thank you so much, @fablau Fabrizio!

[sschmidTU]

osmd.EngravingRules.RenderCount was added in 1e16c6d.