Patch for Issue 897 - Key signature should be inherited by all Voices (no matter their accidental style)

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Patch for Issue 897 - Key signature should be inherited by all Voices (no matter their accidental style)

Owen Tuz
The issue: http://code.google.com/p/lilypond/issues/detail?id=897&q=label%3Afrog&colspec=ID%20Type%20Status%20Priority%20Stars%20Owner%20Summary

This was a fun one. I spent hours looking through the source learning how scm input works, then moved one line of code and it seems to work :)
I've just moved the accidental_engraver down to the Voice context - best of all, someone had already left a comment in engraver_init.ly suggesting various engravers be moved, so I can't claim much of the credit (I've left the others where they were for now, but based on this I guess it could be worth considering).

If someone could verify that this does indeed fix the issue and not create anything new and horrible, that'd be great. There's ly code to reproduce the issue at the link above.

Thanks,
 Owen



0001-Moved-accidental_engraver-to-Voice-context.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Patch for Issue 897 - Key signature should be inherited by all Voices (no matter their accidental style)

Neil Puttock
On 22 December 2010 15:16, Owen Tuz <[hidden email]> wrote:

> If someone could verify that this does indeed fix the issue and not create
> anything new and horrible, that'd be great. There's ly code to reproduce the
> issue at the link above.

Have you run a regresson test check?  That's the quickest way of
checking it doesn't break anything.

I'm not sure whether moving the engraver will affect any accidental
rules adversely, but it *will* have a detrimental effect on accidental
placement: the Accidental_engraver creates one AccidentalPlacement
grob per timestep in a stave, so if you move it to the Voice context,
each voice's accidentals will be oblivious to any other accidentals,
causing collisions.

Here's a simple example showing the problem:

\version "2.13.44"

\relative c' {
  << { cis1 } \\ { ais } >>
}

\layout {
  \context {
    \Staff
    \remove "Accidental_engraver"
  }
  \context {
    \Voice
    \consists "Accidental_engraver"
  }
}

Cheers,
Neil

---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|

Re: Patch for Issue 897 - Key signature should be inherited by all Voices (no matter their accidental style)

Owen Tuz
In reply to this post by Owen Tuz


On Wed, Dec 22, 2010 at 11:20 PM, <[hidden email]> wrote:
Hi Owen,

Your patch affects the output of the attached file in an undesirable way.
This snippet was extracted from the regression test 'collisions.ly', and,
after applying your patch, the accidentals collide in a single column.

We may well need to move engravers, but that ought be done with care.

Kind regards,
Felipe

PS: I am having trouble posting at the Frogs, so I am sending you
a private mail. Please, pass the snippet forward, so others can
help  you solving this issue.

Thanks Felipe and Neil. I had a feeling it couldn't be that simple!
I'm still a bit new to this but I should have thought to run the regtests >.< I'll have a look and see what I can come up with.

Cheers (and merry Christmas if that's your thing)
 Owen