Fix issues 1259 and 1433 (\breakDynamicSpan and a spanner's style=#'none over a line break)

Fix  issues 1259 and 1433 (\breakDynamicSpan and a spanner's style=#'none over a line break)

This changes the way DynamicLineSpanners are handled when \breakDynamicSpan
is called or when the DynamicTextSpanner's style=#'none (i.e. no line should
be used and thus the spanner should also not unneccessarily shift the dynamic
text spanner up/down).

So far, this was handled by simply ending the DynamicLineSpanner prematurely,
while leaving its children at their full length. As a consequence, the child
spanners were no longer fully contained in the DynamicLineSpanner, which
caused several problem at line breaks.
    
This patch changes it as follows:
-) All spanner breaks are handled by a flag set on the spanner itself
-) The breakDynamicSpan events are handled by the dynamic engraver, which will set that flag
-) When a DynamicTextSpanner with style=#'none or a \breakDynamicSpan
   is encountered, I also set that flag immediately after spanner creation
-) From then on, no new dynamics are added to the line spanner and no
   new support points are added that would otherwise shift the spanner
 	 if there is a very high/low note or articulation. If the spanner creates
 	 a grob (like a hairpin), that grob would still be shifted as a grob
 	 to prevent collisions, though.
-) When the current child spanner is ended, we also end the DynamicLineSpanner
   (and store it in a temporary variable so that we can properly end it
 	 in stop_translation_timestep). If a new dynamic is encountered, it will
 	 then create a completely new DynamicLineSpanner, which provides the
 	 independent alignment that we want.

This fixes both issues 1259 and 1433:
http://code.google.com/p/lilypond/issues/detail?id=1259
http://code.google.com/p/lilypond/issues/detail?id=1433

[Neil Puttock, 2011-06-26 11:34:20]

Hi Reinhold,

I'm afraid this fails `make check'.  dynamics-alignment-no-line-linebreak.ly
weirdly works directly without a hitch, but crashes under test conditions:

Processing `./1e/lily-b9dc92b9.ly'
Parsing...
Renaming input to: `dynamics-alignment-no-line-linebreak.ly'
Interpreting music...
Preprocessing graphical objects...
Calculating line breaks...
Drawing systems...
Writing header field `texidoc' to `./1e/lily-b9dc92b9.texidoc'...
Writing ./1e/lily-b9dc92b9-1.signature
Writing ./1e/lily-b9dc92b9-2.signatureUnbound variable: open-file
Unbound variable: open-file
ERROR: In procedure ly_grob_extent:
ERROR: Wrong type argument in position 2 (expecting common refpoint): #<Grob
System >

BTW, did you notice that the altered regtest you posted still doesn't work
properly?  The final diminuendo spanner is still suicided due to invalid bounds.

Cheers,
Neil

[Reinhold, 2011-06-26 11:56:32]

On 2011/06/26 11:34:20, Neil Puttock wrote:
> Hi Reinhold,
> 
> I'm afraid this fails `make check'.  dynamics-alignment-no-line-linebreak.ly
> weirdly works directly without a hitch, but crashes under test conditions:

Hmm, okay, it seems that -ddump-signatures also has problems with spanners
without a parent...

lilypond -ddump-signatures=#t dynamics-alignment-no-line-linebreak.ly
reinhold@curie:~/lilypond/lilypond/input/regression$ lilypond
-ddump-signatures=#t dynamics-alignment-no-line-linebreak.ly
GNU LilyPond 2.15.3
»dynamics-alignment-no-line-linebreak.ly« wird verarbeitet
Analysieren...
Interpretation der Musik...
Vorverarbeitung der grafischen Elemente...
Ideale Seitenanzahl wird gefunden...
Musik wird auf eine Seite angepasst...
Systeme erstellen...
Layout nach »dynamics-alignment-no-line-linebreak.ps« ausgeben...
Writing dynamics-alignment-no-line-linebreak-1.signature
Writing dynamics-alignment-no-line-linebreak-2.signatureERROR: In procedure
ly_grob_extent:
ERROR: Wrong type argument in position 2 (expecting common refpoint): #<Grob
System >


> BTW, did you notice that the altered regtest you posted still doesn't work
> properly?  The final diminuendo spanner is still suicided due to invalid
bounds.

Hmm, you are right. If the spanner ends on the first note after a line break,
there are still problems...


Cheers,
Reinhold

[reinhold_kainhofer.com, 2011-06-26 12:02:48]

Am Sonntag, 26. Juni 2011, 13:34:20 schrieben Sie:
> I'm afraid this fails `make check'.
> dynamics-alignment-no-line-linebreak.ly weirdly works directly without a
> hitch, but crashes under test conditions:
> 
> Processing `./1e/lily-b9dc92b9.ly'
> Parsing...
> Renaming input to: `dynamics-alignment-no-line-linebreak.ly'
> Interpreting music...
> Preprocessing graphical objects...
> Calculating line breaks...
> Drawing systems...
> Writing header field `texidoc' to `./1e/lily-b9dc92b9.texidoc'...
> Writing ./1e/lily-b9dc92b9-1.signature
> Writing ./1e/lily-b9dc92b9-2.signatureUnbound variable: open-file
> Unbound variable: open-file
> ERROR: In procedure ly_grob_extent:
> ERROR: Wrong type argument in position 2 (expecting common refpoint):
> #<Grob System >

Hmm, again the problem is that the parts after a line break of broken dynamic 
spanners (text / hairpin) do not have any parent set any more... Now, the 
function write-system-signature (subfunction found-grob, stencil.scm) calls 
ly:grob-extent, which checks for the common refpoint of the spanner and its 
parent (which is 0)...

So, I more and more get the impression that the approach of simply killing the 
line spanner and letting its child dynamic spanners live on is the wrong 
approach to \breakDynamicSpan and spanner's style=#'none...

Cheers,
Reinhold



-- 
------------------------------------------------------------------
Reinhold Kainhofer, reinhold@kainhofer.com, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

[Neil Puttock, 2011-06-26 12:25:15]

On 26 June 2011 13:02, Reinhold Kainhofer <reinhold@kainhofer.com> wrote:

> Hmm, again the problem is that the parts after a line break of broken dynamic
> spanners (text / hairpin) do not have any parent set any more... Now, the
> function write-system-signature (subfunction found-grob, stencil.scm) calls
> ly:grob-extent, which checks for the common refpoint of the spanner and its
> parent (which is 0)...

I think you're misreading this.  The common refpoint is a System; it
doesn't have a parent since it's the top-level grob.  The problem is
that the broken spanner is expecting a different System.

> So, I more and more get the impression that the approach of simply killing the
> line spanner and letting its child dynamic spanners live on is the wrong
> approach to \breakDynamicSpan and spanner's style=#'none...

Yep, it definitely appears to be too naive.

Cheers,
Neil

[Reinhold, 2011-06-28 12:25:22]

On 2011/06/26 12:25:15, Neil Puttock wrote:
> On 26 June 2011 13:02, Reinhold Kainhofer <mailto:reinhold@kainhofer.com>
wrote:
> > So, I more and more get the impression that the approach of simply killing
the
> > line spanner and letting its child dynamic spanners live on is the wrong
> > approach to \breakDynamicSpan and spanner's style=#'none...
> 
> Yep, it definitely appears to be too naive.

I think I've now found the proper solution to the problem: Keep the line spanner
alive, but don't add new support points and break it at the end of the current
spanner. New patchset with updated description is up.

Cheers,
Reinhold

[pkx166h, 2011-06-28 15:03:40]

Passes reg tests - dynamics-alignment-breaker.ly is obviously different as the
reg test itself has been changed.

[Reinhold, 2011-06-28 16:05:26]

On 2011/06/28 15:03:40, J_lowe wrote:
> Passes reg tests - dynamics-alignment-breaker.ly is obviously different as the
> reg test itself has been changed.

Yes, I removed that second \breakDynamicSpan, because I wanted to check that
while the first cresc is broken, the second line spanner is really left intact
and the decrescendo and the \p line up.
I'll remove the line break, though, because that's tested in the *-linebreak.ly
test case

[pkx166h, 2011-07-03 00:03:07]

On 2011/06/28 16:05:26, Reinhold wrote:
> On 2011/06/28 15:03:40, J_lowe wrote:
> > Passes reg tests - dynamics-alignment-breaker.ly is obviously different as
the
> > reg test itself has been changed.
> 
> Yes, I removed that second \breakDynamicSpan, because I wanted to check that
> while the first cresc is broken, the second line spanner is really left intact
> and the decrescendo and the \p line up.
> I'll remove the line break, though, because that's tested in the
*-linebreak.ly
> test case

Reinhold,

I re-did the reg test, the only difference is the
'dynamics-alignment-breaker.ly' as I said before.

I haven't done a comparison with the difference between the last patch and this
one, only as base check,

It looks ok (again it is different as expected) but for completeness (and
because i cannot do it here) I have attached a png of the differences on issues
1259 and 1433:

[Neil Puttock, 2011-07-03 20:14:56]

LGTM.

http://codereview.appspot.com/4630070/diff/3002/input/regression/dynamics-ali...
File input/regression/dynamics-alignment-breaker-linebreak.ly (right):

http://codereview.appspot.com/4630070/diff/3002/input/regression/dynamics-ali...
input/regression/dynamics-alignment-breaker-linebreak.ly:12:
c1_\dim\breakDynamicSpan
redundant \breakDynamicSpan

http://codereview.appspot.com/4630070/diff/3002/input/regression/dynamics-ali...
input/regression/dynamics-alignment-breaker-linebreak.ly:15:
c,,1_\dim\breakDynamicSpan
redundant \breakDynamicSpan

http://codereview.appspot.com/4630070/diff/3002/input/regression/dynamics-ali...
File input/regression/dynamics-alignment-breaker.ly (right):

http://codereview.appspot.com/4630070/diff/3002/input/regression/dynamics-ali...
input/regression/dynamics-alignment-breaker.ly:19: %   \breakDynamicSpan
remove

http://codereview.appspot.com/4630070/diff/3002/lily/dynamic-align-engraver.cc
File lily/dynamic-align-engraver.cc (right):

http://codereview.appspot.com/4630070/diff/3002/lily/dynamic-align-engraver.c...
lily/dynamic-align-engraver.cc:93: programming_error ("Already have a
force-ended DynamicLineSpanner.");
"already have a force-ended DynamicLineSpanner"

http://codereview.appspot.com/4630070/diff/3002/lily/dynamic-align-engraver.c...
lily/dynamic-align-engraver.cc:163: || (end && d == RIGHT && !line->get_bound
(RIGHT)))
indent

[Reinhold, 2011-07-03 20:27:15]

http://codereview.appspot.com/4630070/diff/3002/input/regression/dynamics-ali...
File input/regression/dynamics-alignment-breaker-linebreak.ly (right):

http://codereview.appspot.com/4630070/diff/3002/input/regression/dynamics-ali...
input/regression/dynamics-alignment-breaker-linebreak.ly:12:
c1_\dim\breakDynamicSpan
On 2011/07/03 20:14:56, Neil Puttock wrote:
> redundant \breakDynamicSpan

I don't think so. This \breakDynamicSpan breaks the \dim, while the previous on
breaks only the \<. The dim will create a new DynamicLineSpanner...

http://codereview.appspot.com/4630070/diff/3002/input/regression/dynamics-ali...
input/regression/dynamics-alignment-breaker-linebreak.ly:15:
c,,1_\dim\breakDynamicSpan
On 2011/07/03 20:14:56, Neil Puttock wrote:
> redundant \breakDynamicSpan

See above.

[Neil Puttock, 2011-07-03 20:44:48]

On 3 July 2011 21:27,  <reinhold.kainhofer@gmail.com> wrote:
>
>
http://codereview.appspot.com/4630070/diff/3002/input/regression/dynamics-ali...
> File input/regression/dynamics-alignment-breaker-linebreak.ly (right):
>
>
http://codereview.appspot.com/4630070/diff/3002/input/regression/dynamics-ali...
> input/regression/dynamics-alignment-breaker-linebreak.ly:12:
> c1_\dim\breakDynamicSpan
> On 2011/07/03 20:14:56, Neil Puttock wrote:
>>
>> redundant \breakDynamicSpan
>
> I don't think so. This \breakDynamicSpan breaks the \dim, while the
> previous on breaks only the \<. The dim will create a new
> DynamicLineSpanner...
>
>
http://codereview.appspot.com/4630070/diff/3002/input/regression/dynamics-ali...
> input/regression/dynamics-alignment-breaker-linebreak.ly:15:
> c,,1_\dim\breakDynamicSpan
> On 2011/07/03 20:14:56, Neil Puttock wrote:
>>
>> redundant \breakDynamicSpan
>
> See above.

Have you removed them and compared the output?  They're identical, and
both snippets create five DynamicLineSpanner grobs.

[Reinhold, 2011-07-03 23:09:21]

On 2011/07/03 20:44:48, Neil Puttock wrote:
> On 3 July 2011 21:27,  <mailto:reinhold.kainhofer@gmail.com> wrote:
> > I don't think so. This \breakDynamicSpan breaks the \dim, while the
> > previous on breaks only the \<. The dim will create a new
> > DynamicLineSpanner...
> 
> Have you removed them and compared the output?  They're identical, and
> both snippets create five DynamicLineSpanner grobs.

Thanks for spotting that. I'll have to investigate tomorrow why this
\breakDynamicSpan doesn't have an effect. It properly breaks the spanner if it
is placed right before the \p, but not if I put it immediately after the \dim...
I.e. the following works:

  c'1 c1^\<\breakDynamicSpan c'' \break
  c,,1_\dim
  f,1\breakDynamicSpan\p \break

but this does not:

  c'1 c1^\<\breakDynamicSpan c'' \break
  c,,1_\dim\breakDynamicSpan
  f,1\p \break

[Carl, 2011-07-05 05:32:58]

LGTM, but I haven't explored the strange behavior you've identified.

Carl

[Reinhold, 2011-07-06 15:07:51]

On 2011/07/03 23:09:21, Reinhold wrote:
> I'll have to investigate tomorrow why this
> \breakDynamicSpan doesn't have an effect. It properly breaks the spanner if it
> is placed right before the \p, but not if I put it immediately after the
\dim...
> I.e. the following works:
> 
>   c'1 c1^\<\breakDynamicSpan c'' \break
>   c,,1_\dim
>   f,1\breakDynamicSpan\p \break
> 
> but this does not:
> 
>   c'1 c1^\<\breakDynamicSpan c'' \break
>   c,,1_\dim\breakDynamicSpan
>   f,1\p \break

I now debugged that problem and the issue is that ALL events created by
\breakDynamicSpan are sent to the engraver before any dynamic event is sent. In
particular, for c1\<\breakDynamicSpan the order of events heard by the dynamic
span engraver is:
1) break event
2) crescendo start event

Similarly, the \dim\breakDynamicSpan (with the implicit \! for the \<) is
actually observed as:
1) break event
2) crescendo end event
3) decrescendo (dim) start event

So, even though the user places the break after the \dim (because he wants to
break the dim), lilypond observes it as if it was placed before the \dim!


In short, there is no difference if you write 
c\<\breakDynamicSpan or c\breakDynamicSpan\< (because you want to break the
previous spanner)

I guess, we'll have to define how \breakDynamicSpan is supposed to work
semantically. Should it
1) End a spanner that is ended at the current moment, but not a spanner that is
created at the current moment, or should it
2) end a spanner that newly created at the current moment, but not a spanner
that was running up to the current moment

In particular, there are several cases:

a) c1\<\breakDynamicSpan c\!   => It should break the newly created \< in all
cases
b) c1\< c\breakDynamicSpan\!   => It should break the existing \<
c) c1\< c\!\breakDynamicSpan   => It should break the (no longer) existing \<
?!?
d) c1\< c\breakDynamicSpan\> c\! => Should this break the \< or the \>????
e) c1\< c\>\breakDynamicSpan c\! => Should this break the \< or the \>????

From an input perspective, d) and e) should work differently (i.e. the order of
events SHOULD matter), but the problem is that the engraver receives the events
reordered, with the break event always being the first. Is there any way around
this?

Cheers,
Reinhold

[Neil Puttock, 2011-07-07 16:15:54]

On 6 July 2011 16:07,  <reinhold.kainhofer@gmail.com> wrote:

> I now debugged that problem and the issue is that ALL events created by
> \breakDynamicSpan are sent to the engraver before any dynamic event is
> sent. In particular, for c1\<\breakDynamicSpan the order of events heard
> by the dynamic span engraver is:
> 1) break event
> 2) crescendo start event

I'm a bit puzzled as to why you're talking about dynamic events here.
The Dynamic_align_engraver only listens to break-span-event; it
acknowledges dynamic grobs which have been created by
New_dynamic_engraver.   Event broadcasting happens before translation
creates (and thus announces) grobs for acknowledgment.

Cheers,
Neil

[reinhold_kainhofer.com, 2011-07-07 16:37:09]

Am Donnerstag, 7. Juli 2011, 18:15:54 schrieben Sie:
> On 6 July 2011 16:07,  <reinhold.kainhofer@gmail.com> wrote:
> > I now debugged that problem and the issue is that ALL events created by
> > \breakDynamicSpan are sent to the engraver before any dynamic event is
> > sent. In particular, for c1\<\breakDynamicSpan the order of events heard
> > by the dynamic span engraver is:
> > 1) break event
> > 2) crescendo start event
> 
> I'm a bit puzzled as to why you're talking about dynamic events here.
> The Dynamic_align_engraver only listens to break-span-event; it
> acknowledges dynamic grobs which have been created by
> New_dynamic_engraver.

Ah, that's a very good observation, which I have failed to make... And of 
course, it explains everything.
The problem now is just, how can we handle this in the dynamic align engraver? 

Cheers,
Reinhold
-- 
------------------------------------------------------------------
Reinhold Kainhofer, reinhold@kainhofer.com, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

[pkx166h, 2011-07-20 21:04:49]

Make is fine, the same reg test that showed a difference last time now shows
that the text 'dim...' has been replaced by hairpins, is this what is wanted?

See attached at

http://code.google.com/p/lilypond/issues/detail?id=1433#c5

[reinhold_kainhofer.com, 2011-07-20 21:16:31]

Am Mittwoch, 20. Juli 2011, 23:04:49 schrieben Sie:
> Make is fine, the same reg test that showed a difference last time now
> shows that the text 'dim...' has been replaced by hairpins, is this what
> is wanted?

Yes, I replaced them, because the text spanner's extender line didn't look 
good at all.

Cheers,
Reinhold
-- 
------------------------------------------------------------------
Reinhold Kainhofer, reinhold@kainhofer.com, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

[Neil Puttock, 2011-07-21 18:48:50]

http://codereview.appspot.com/4630070/diff/17001/input/regression/dynamics-al...
File input/regression/dynamics-alignment-breaker-linebreak.ly (right):

http://codereview.appspot.com/4630070/diff/17001/input/regression/dynamics-al...
input/regression/dynamics-alignment-breaker-linebreak.ly:5: dynamic spanner is
across a line break.
crosses a line break

http://codereview.appspot.com/4630070/diff/17001/input/regression/dynamics-al...
input/regression/dynamics-alignment-breaker-linebreak.ly:9: % Both lines should
give the same output, although the \breakDynamicSpan
should go in texidoc

Edit: actually, I'm a bit confused since there are three lines and they're all
different

http://codereview.appspot.com/4630070/diff/17001/lily/dynamic-align-engraver.cc
File lily/dynamic-align-engraver.cc (right):

http://codereview.appspot.com/4630070/diff/17001/lily/dynamic-align-engraver....
lily/dynamic-align-engraver.cc:49: Spanner *ended_line_; // Spanner was manually
broken, create a new one, but store the old so we can properly finish it.
comment is a bit long

http://codereview.appspot.com/4630070/diff/17001/lily/dynamic-align-engraver....
lily/dynamic-align-engraver.cc:93: programming_error ("Already have a
force-ended DynamicLineSpanner.");
"already have a force-ended DynamicLineSpanner"

http://codereview.appspot.com/4630070/diff/17001/lily/dynamic-align-engraver....
lily/dynamic-align-engraver.cc:134: // TODO: Compare the direction of the
existing spanner with
remove

(issue #1111)

http://codereview.appspot.com/4630070/diff/17001/lily/dynamic-align-engraver....
lily/dynamic-align-engraver.cc:189: bool end = line_ && (running_.empty ());
&& running_.empty ();

http://codereview.appspot.com/4630070/diff/17001/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (right):

http://codereview.appspot.com/4630070/diff/17001/scm/define-grob-properties.s...
scm/define-grob-properties.scm:780: (spanner-broken ,boolean? "Indicates whether
dynamics spanner
internal?

I'd remove any reference to dynamics here, since this is generic enough to be
useful elsewhere (e.g. pedals)

http://codereview.appspot.com/4630070/diff/17001/scm/define-grobs.scm
File scm/define-grobs.scm (right):

http://codereview.appspot.com/4630070/diff/17001/scm/define-grobs.scm#newcode812
scm/define-grobs.scm:812: (spanner-broken . #f)
remove

[Reinhold, 2011-07-22 13:57:57]

Uploaded new patch that fixes all of Neil's concerns and clean up the regtests a
bit. AFAICT, this should now be the final patch.

http://codereview.appspot.com/4630070/diff/17001/input/regression/dynamics-al...
File input/regression/dynamics-alignment-breaker-linebreak.ly (right):

http://codereview.appspot.com/4630070/diff/17001/input/regression/dynamics-al...
input/regression/dynamics-alignment-breaker-linebreak.ly:5: dynamic spanner is
across a line break.
On 2011/07/21 18:48:50, Neil Puttock wrote:
> crosses a line break

Changed, thanks.

http://codereview.appspot.com/4630070/diff/17001/input/regression/dynamics-al...
input/regression/dynamics-alignment-breaker-linebreak.ly:9: % Both lines should
give the same output, although the \breakDynamicSpan
On 2011/07/21 18:48:50, Neil Puttock wrote:
> should go in texidoc
> 
> Edit: actually, I'm a bit confused since there are three lines and they're all
> different

I have now completely redone the regtests, splitting them up into several
smaller tests. In this test I was actually checking three different things at
once (line breaks, exact position of \breakDyanmicSpan, and that the break does
not have an effect on subsequent spanners).

http://codereview.appspot.com/4630070/diff/17001/input/regression/dynamics-al...
File input/regression/dynamics-alignment-breaker.ly (left):

http://codereview.appspot.com/4630070/diff/17001/input/regression/dynamics-al...
input/regression/dynamics-alignment-breaker.ly:12: \dimTextDim
Added that again, so make check should not show and difference to before.

http://codereview.appspot.com/4630070/diff/17001/lily/dynamic-align-engraver.cc
File lily/dynamic-align-engraver.cc (right):

http://codereview.appspot.com/4630070/diff/17001/lily/dynamic-align-engraver....
lily/dynamic-align-engraver.cc:49: Spanner *ended_line_; // Spanner was manually
broken, create a new one, but store the old so we can properly finish it.
On 2011/07/21 18:48:50, Neil Puttock wrote:
> comment is a bit long

Shortened

http://codereview.appspot.com/4630070/diff/17001/lily/dynamic-align-engraver....
lily/dynamic-align-engraver.cc:93: programming_error ("Already have a
force-ended DynamicLineSpanner.");
On 2011/07/21 18:48:50, Neil Puttock wrote:
> "already have a force-ended DynamicLineSpanner"

Changed

http://codereview.appspot.com/4630070/diff/17001/lily/dynamic-align-engraver....
lily/dynamic-align-engraver.cc:134: // TODO: Compare the direction of the
existing spanner with
On 2011/07/21 18:48:50, Neil Puttock wrote:
> remove
> 
> (issue #1111)

Actually, I explicitly added it because of issue #1111 to mark the possible
entry point for a fix.

http://codereview.appspot.com/4630070/diff/17001/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (right):

http://codereview.appspot.com/4630070/diff/17001/scm/define-grob-properties.s...
scm/define-grob-properties.scm:780: (spanner-broken ,boolean? "Indicates whether
dynamics spanner
On 2011/07/21 18:48:50, Neil Puttock wrote:
> internal?

Okay, made it internal.

> I'd remove any reference to dynamics here, since this is generic enough to be
> useful elsewhere (e.g. pedals)

Good idea.

[Neil Puttock, 2011-07-23 22:05:19]

Hi Reinhold,

This LGTM, but I think it would be better to fold the patch for issue #1111 into
this, otherwise you're going to have to reformulate some of regression tests so
they don't use explicit directions.

Cheers,
Neil