Fixes note column skylines by adding a stem tremolo pure height function.

Fixes note column skylines by adding a stem tremolo pure height function.

[MikeSol, 2011-07-18 16:11:09]

This fixes issue 1768.

[Neil Puttock, 2011-07-18 19:24:26]

Hi Mike,

I think this is too complicated; you can probably achieve the same result via a
Stem_tremolo::pure_height function.

Cheers,
Neil

[MikeSol, 2011-07-18 19:28:51]

On 2011/07/18 19:24:26, Neil Puttock wrote:
> Hi Mike,
> 
> I think this is too complicated; you can probably achieve the same result via
a
> Stem_tremolo::pure_height function.
> 
> Cheers,
> Neil

Thanks for the feedback!

How would pure height affect the calculation of horizontal skylines?  The issue
is that the StemTremolo grob is not part of the note-column's axis group, which
messes up its horizontal skyline and thus its springs in horizontal spacing
calculations.

To make it less complicated, I could nix the stem-tremolo object, add
axis-group-interface.hh to note-column.cc, and call add_element on
stem_tremolo_.  This would get rid of the new function in note-column.cc.

Cheers,
MS

[Neil Puttock, 2011-07-18 19:47:53]

On 2011/07/18 19:28:51, MikeSol wrote:

> How would pure height affect the calculation of horizontal skylines?

It should ensure the skylines are correct, assuming there's an entry for
Stem_tremolo::height in pure-conversions-alist.

It looks like the existing height function is pure since it doesn't reference
beams, so you just need to add the start/end arguments and a new
Stem_tremolo::height callback which gets the extent from the translated stencil.

Cheers,
Neil

[Neil Puttock, 2011-07-18 19:53:38]

On 2011/07/18 19:47:53, Neil Puttock wrote:
> On 2011/07/18 19:28:51, MikeSol wrote:
> 
> > How would pure height affect the calculation of horizontal skylines?
> 
> It should ensure the skylines are correct, assuming there's an entry for
> Stem_tremolo::height in pure-conversions-alist.
> 
> It looks like the existing height function is pure since it doesn't reference
> beams, so you just need to add the start/end arguments and a new
> Stem_tremolo::height callback which gets the extent from the translated
stencil.

Even simpler: remove the Y-extent default, add Stem_tremolo::pure_height and an
entry in pure-print-to-height-conversions (since ly:grob::stencil-height is the
same as getting the non-pure extent from the translated stencil).

[pkx166h, 2011-07-18 20:01:00]

Make works fine but I get a seg fault when I do a make check after a
test-baseline for the reg test comparison.

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

On 2011/07/18 20:01:00, J_lowe wrote:
> Make works fine but I get a seg fault when I do a make check after a
> test-baseline for the reg test comparison.

Same here.  stem-tremolo.ly segfaults inside rhythmic-column.cc; the
acknowledged StemTremolo doesn't have an X-parent.

[MikeSol, 2011-07-19 07:03:02]

On 2011/07/18 21:24:50, Neil Puttock wrote:
> On 2011/07/18 20:01:00, J_lowe wrote:
> > Make works fine but I get a seg fault when I do a make check after a
> > test-baseline for the reg test comparison.
> 
> Same here.  stem-tremolo.ly segfaults inside rhythmic-column.cc; the
> acknowledged StemTremolo doesn't have an X-parent.

New patchset uploaded with the pure_height fix - thanks for the tip, Neil!  Now
I have to go try to understand why it works...

Cheers,
MS

[mike_apollinemike.com, 2011-07-19 08:10:39]

On Jul 18, 2011, at 9:53 PM, n.puttock@gmail.com wrote:

> Even simpler: remove the Y-extent default, add Stem_tremolo::pure_height
> and an entry in pure-print-to-height-conversions (since
> ly:grob::stencil-height is the same as getting the non-pure extent from
> the translated stencil).
> 
> http://codereview.appspot.com/4754054/
> 

Hey Neil,

After making several round-trips around the source this morning, I can't put off
composition any longer, but I fear that all I will compose today are songs about
the Axis_group_interface if I don't understand how this works.
I believe that line 80 of note-spacing.cc is where the two skylines for the two
note columns are calculated.  How, by adding a pure-height to the Stem_tremolo,
does the Stem_tremolo make its way into this skyline?  And, if it doesn't make
its way into this skyline, how is it taken into account during horizontal
spacing?

Sorry for the many questions - usually I can grok these things on my own, but
this is proving to be elusive (as are all things having to do with `pure' in
LilyPond, which I understand in only the most fleeting and obtuse way).

Cheers,
MS

[Neil Puttock, 2011-07-19 16:11:36]

On 19 July 2011 09:10, mike@apollinemike.com <mike@apollinemike.com> wrote:

> After making several round-trips around the source this morning, I can't put
off composition any longer, but I fear that all I will compose today are songs
about the Axis_group_interface if I don't understand how this works.
> I believe that line 80 of note-spacing.cc is where the two skylines for the
two note columns are calculated.  How, by adding a pure-height to the
Stem_tremolo, does the Stem_tremolo make its way into this skyline?  And, if it
doesn't make its way into this skyline, how is it taken into account during
horizontal spacing?

I'm afraid I don't know why it works. I just suggested it based on
seeing how similar collisions have been solved in the past (e.g., for
ez-noteheads).

Cheers,
Neil

[Neil Puttock, 2011-07-19 16:26:05]

http://codereview.appspot.com/4754054/diff/14001/lily/stem-tremolo.cc
File lily/stem-tremolo.cc (right):

http://codereview.appspot.com/4754054/diff/14001/lily/stem-tremolo.cc#newcode151
lily/stem-tremolo.cc:151: Stem_tremolo::pure_height (SCM smob, SCM, SCM)
SCM /* start */, SCM /* end */

I spoke too soon here.  This function isn't pure (despite the comment noting
that it's unsafe to get the true slope from the beam) since it calls
translated_stencil () which will look at the beam:

\relative c' {
  c8:16 c c c' 
}

-> programming error: minimise_least_squares ():  Nothing to minimise
This means that vertical spacing is triggered
before line breaking

(and an unsloped beam)

untranslated_stencil () is probably OK here (it's already used for
ly:stem-tremolo::width).

[pkx166h, 2011-07-19 21:03:20]

Make works ok, but I get some warnings in the reg test

--snip--

@@ -9,6 +9,36 @@
   a'4
      :4 c:8 a:16 c:32 a a: a2:
 Preprocessing graphical objects...
+programming error: minimise_least_squares ():  Nothing to minimise
+This means that vertical spacing is triggered
+before line breaking
+
+continuing, cross fingers
+programming error: minimise_least_squares ():  Nothing to minimise
+This means that vertical spacing is triggered
+before line breaking
+
+continuing, cross fingers
+programming error: minimise_least_squares ():  Nothing to minimise
+This means that vertical spacing is triggered
+before line breaking
+
+continuing, cross fingers
+programming error: minimise_least_squares ():  Nothing to minimise
+This means that vertical spacing is triggered
+before line breaking
+
+continuing, cross fingers
+programming error: minimise_least_squares ():  Nothing to minimise
+This means that vertical spacing is triggered
+before line breaking
+
+continuing, cross fingers
+programming error: minimise_least_squares ():  Nothing to minimise
+This means that vertical spacing is triggered
+before line breaking
+
+continuing, cross fingers
 Calculating line breaks... 
 Drawing systems... 
 Writing header field `texidoc' to
`/home/jlowe/lilypond-git/build/out/lybook-testdb/09/lily-c7e028e5.`texidoc' to
`/home/jlowe/lilypond-git/build/out/lybook-testdb/09/lily-c7e028e5.texidoc'...

---

also two reg tests show changes

See attached images on 

http://code.google.com/p/lilypond/issues/detail?id=1768

[MikeSol, 2011-07-20 14:03:49]

I went back to my own plan after recent grob impurities - this passes regtests &
causes slight differences in horizontal spacing in a couple tremolo related
regtests that seem for the better.  If someone could confirm the regtest check,
I'd be much obliged.

Cheers,
MS

[MikeSol, 2011-07-20 14:04:19]

On 2011/07/20 14:03:49, MikeSol wrote:
> I went back to my own plan after recent grob impurities - this passes regtests
&
> causes slight differences in horizontal spacing in a couple tremolo related
> regtests that seem for the better.  If someone could confirm the regtest
check,
> I'd be much obliged.
> 
> Cheers,
> MS

Errata - should read "I went back to my old plan" - I don't own any plans.

Cheers,
MS

[Neil Puttock, 2011-07-20 15:32:20]

On 2011/07/20 14:03:49, MikeSol wrote:
> I went back to my own plan after recent grob impurities - this passes regtests
&
> causes slight differences in horizontal spacing in a couple tremolo related
> regtests that seem for the better.  If someone could confirm the regtest
check,
> I'd be much obliged.

It still segfaults here.  stem_tremolo_ is uninitialized.

I still think this version is too complicated; did you try emending the
pure-height function as I suggested?

Cheers,
Neil

[mike_apollinemike.com, 2011-07-20 15:37:07]

On Jul 20, 2011, at 5:32 PM, n.puttock@gmail.com wrote:

> On 2011/07/20 14:03:49, MikeSol wrote:
>> I went back to my own plan after recent grob impurities - this passes
> regtests &
>> causes slight differences in horizontal spacing in a couple tremolo
> related
>> regtests that seem for the better.  If someone could confirm the
> regtest check,
>> I'd be much obliged.
> 
> It still segfaults here.  stem_tremolo_ is uninitialized.
> 
> I still think this version is too complicated; did you try emending the
> pure-height function as I suggested?
> 
> Cheers,
> Neil
> 

I'm starting to doubt the viability of my local build...the regtests went off
without a hitch.
I'll look into emending the pure height function.

Cheers,
MS

[Neil Puttock, 2011-07-20 16:04:23]

On 20 July 2011 16:36, mike@apollinemike.com <mike@apollinemike.com> wrote:

> I'm starting to doubt the viability of my local build...the regtests went off
without a hitch.

I wouldn't worry about that.  Uninitialized members often work fine;
it might depend on your architecture.

> I'll look into emending the pure height function.

I've just tested both versions, and there are minor differences in
spacing (though nothing serious AFAICT).

Cheers,
Neil

[MikeSol, 2011-07-20 20:21:41]

On 2011/07/20 16:04:23, Neil Puttock wrote:
> On 20 July 2011 16:36, mailto:mike@apollinemike.com
<mailto:mike@apollinemike.com> wrote:
> 
> > I'm starting to doubt the viability of my local build...the regtests went
off
> without a hitch.
> 
> I wouldn't worry about that.  Uninitialized members often work fine;
> it might depend on your architecture.
> 
> > I'll look into emending the pure height function.
> 
> I've just tested both versions, and there are minor differences in
> spacing (though nothing serious AFAICT).
> 
> Cheers,
> Neil

The Stem_tremolo::pure_height function is now pure.  Works AFAICT.  I'm hitting
the sack (longest LilyPond day ever...I <3 glissando stems) - if anyone is up
for a late night regtest, go for it.

Cheers,
MS

[mike_apollinemike.com, 2011-07-21 07:59:22]

On Jul 20, 2011, at 10:21 PM, mtsolo@gmail.com wrote:

> On 2011/07/20 16:04:23, Neil Puttock wrote:
>> On 20 July 2011 16:36, mailto:mike@apollinemike.com
> <mailto:mike@apollinemike.com> wrote:
> 
>> > I'm starting to doubt the viability of my local build...the regtests
> went off
>> without a hitch.
> 
>> I wouldn't worry about that.  Uninitialized members often work fine;
>> it might depend on your architecture.
> 
>> > I'll look into emending the pure height function.
> 
>> I've just tested both versions, and there are minor differences in
>> spacing (though nothing serious AFAICT).
> 
>> Cheers,
>> Neil
> 

Passes regtests.
As this patch has an impact on horizontal spacing, I'd like input from a couple
other people before I push.

I'm attaching all the examples that diverged in the regtests - please have a
look at them and let me know what you think.

Cheers,
MS

[pkx166h, 2011-07-24 11:06:43]

Passes make and reg tests show some differences - nothing significantly wrong

see
http://code.google.com/p/lilypond/issues/detail?id=1768#c6

for screenshots

(couldn't see where mikes attached differences were - so redid them again myself
for the tracker in case people were not seeing what had changed).

[MikeSol, 2011-07-27 08:40:04]

On 2011/07/24 11:06:43, J_lowe wrote:
> Passes make and reg tests show some differences - nothing significantly wrong
> 
> see
> http://code.google.com/p/lilypond/issues/detail?id=1768#c6
> 
> for screenshots
> 
> (couldn't see where mikes attached differences were - so redid them again
myself
> for the tracker in case people were not seeing what had changed).

Pushed as 922d59a242646c2488738d77263580427bd7764b.

Cheers,
MS

[MikeSol, 2011-07-27 08:42:32]

On 2011/07/27 08:40:04, MikeSol wrote:
> On 2011/07/24 11:06:43, J_lowe wrote:
> > Passes make and reg tests show some differences - nothing significantly
wrong
> > 
> > see
> > http://code.google.com/p/lilypond/issues/detail?id=1768#c6
> > 
> > for screenshots
> > 
> > (couldn't see where mikes attached differences were - so redid them again
> myself
> > for the tracker in case people were not seeing what had changed).
> 
> Pushed as 922d59a242646c2488738d77263580427bd7764b.
> 
> Cheers,
> MS

ERRATA
Pushed as def419dfd961fee6fd0bf49321ae130143b618af.

Cheers,
MS

[mike_apollinemike.com, 2011-08-11 11:34:54]

On Jul 19, 2011, at 6:11 PM, Neil Puttock wrote:

> On 19 July 2011 09:10, mike@apollinemike.com <mike@apollinemike.com> wrote:
> 
>> After making several round-trips around the source this morning, I can't put
off composition any longer, but I fear that all I will compose today are songs
about the Axis_group_interface if I don't understand how this works.
>> I believe that line 80 of note-spacing.cc is where the two skylines for the
two note columns are calculated.  How, by adding a pure-height to the
Stem_tremolo, does the Stem_tremolo make its way into this skyline?  And, if it
doesn't make its way into this skyline, how is it taken into account during
horizontal spacing?
> 
> I'm afraid I don't know why it works. I just suggested it based on
> seeing how similar collisions have been solved in the past (e.g., for
> ez-noteheads).

I figured out why it works - I figured I'd post this to the list in case anyone
else ever wants to mess around with pure properties.

The StemTremolo is added to the paper column's element grob array via the
axis-group-engraver because it does not have an axis-group-parent-Y.
Then, when horizontal spacing happens, its pure height function is passed
through for its print function (separation-item.cc).

I may write a "pure" tutorial for the contributor's guide.  It has taken me a
long time to figure out how "pure" works in lilypond, and if other people are as
mystified as I was when I started trying to figure this stuff out, then I think
an addition to the CG would help.

Cheers,
MS

[Neil Puttock, 2011-08-11 17:03:25]

On 11 August 2011 12:34, mike@apollinemike.com <mike@apollinemike.com> wrote:

> I figured out why it works - I figured I'd post this to the list in case
anyone else ever wants to mess around with pure properties.
>
> The StemTremolo is added to the paper column's element grob array via the
axis-group-engraver because it does not have an axis-group-parent-Y.

I think you mean the VerticalAxisGroup's elements array.

The StemTremolo is added to a PaperColumn's elements array (and gets
the column as axis-group-parent-X) in the Paper_column_engraver.

> Then, when horizontal spacing happens, its pure height function is passed
through for its print function (separation-item.cc).

I think I understand: before you added the print-to-height conversion,
the original height callback (ly:stem-tremolo::height) wasn't
pure-relevant; this resulted in Item::pure_height () returning an
empty extent, causing the StemTremolo to be left out of the skyline.
This didn't matter unless the spacing was really tight.

> I may write a "pure" tutorial for the contributor's guide.  It has taken me a
long time to figure out how "pure" works in lilypond, and if other people are as
mystified as I was when I started trying to figure this stuff out, then I think
an addition to the CG would help.

Sounds great.

Cheers,
Neil

[mike_apollinemike.com, 2011-08-11 22:31:23]

On Aug 11, 2011, at 7:03 PM, Neil Puttock wrote:

> On 11 August 2011 12:34, mike@apollinemike.com <mike@apollinemike.com> wrote:
> 
>> I figured out why it works - I figured I'd post this to the list in case
anyone else ever wants to mess around with pure properties.
>> 
>> The StemTremolo is added to the paper column's element grob array via the
axis-group-engraver because it does not have an axis-group-parent-Y.
> 
> I think you mean the VerticalAxisGroup's elements array.
> 
> The StemTremolo is added to a PaperColumn's elements array (and gets
> the column as axis-group-parent-X) in the Paper_column_engraver.
> 

Yes, you're absolutely right.

>> Then, when horizontal spacing happens, its pure height function is passed
through for its print function (separation-item.cc).
> 
> I think I understand: before you added the print-to-height conversion,
> the original height callback (ly:stem-tremolo::height) wasn't
> pure-relevant; this resulted in Item::pure_height () returning an
> empty extent, causing the StemTremolo to be left out of the skyline.
> This didn't matter unless the spacing was really tight.
> 

Yup!

Cheers,
MS