Uses the pure-from-neighbor-interface for NonMusicalPaperColumn horizontal spacing.

Uses the pure-from-neighbor-interface for NonMusicalPaperColumn horizontal spacing.

[MikeSol, 2011-11-25 05:27:53]

This is the second of the three-part pure-from-neighbor-interface code rewrite. 
It uses extra-spacing-height for certain important non-musical grobs in order to
prevent other grobs for passing over or under them via the
pure-from-neighbor-interface.

The last step, after this is pushed, will be changing the BarLine
extra-spacing-height function to block all accidentals from passing over
barlines.

Cheers,
MS

[MikeSol, 2011-11-25 05:31:57]

Sorry, forgot to mention that for the time being this only applies cleanly to
the staging branch.  It'll apply cleanly to master once master is
fast-forwarded.

[pkx166h, 2011-11-25 20:58:06]

Passes Make, all reg test diffs here

http://code.google.com/p/lilypond/issues/detail?id=2053#c1

James

[pkx166h, 2011-11-25 20:58:29]

On 2011/11/25 05:31:57, MikeSol wrote:
> Sorry, forgot to mention that for the time being this only applies cleanly to
> the staging branch.  It'll apply cleanly to master once master is
> fast-forwarded.

I fast forwarded staging ;)

[Keith, 2011-11-26 01:08:19]

looks good to me, 
 after putting back skyline-vertical-padding for just one more round.

http://codereview.appspot.com/5432070/diff/1/input/regression/lyrics-pass-und...
File input/regression/lyrics-pass-under-bar.ly (right):

http://codereview.appspot.com/5432070/diff/1/input/regression/lyrics-pass-und...
input/regression/lyrics-pass-under-bar.ly:5: the bar line.
Can't put this in yet, because the BarLines don't yet distinguish between items
from their home staff, and other staves like Lyrics.

http://codereview.appspot.com/5432070/diff/1/scm/define-grobs.scm
File scm/define-grobs.scm (left):

http://codereview.appspot.com/5432070/diff/1/scm/define-grobs.scm#oldcode1413
scm/define-grobs.scm:1413: (skyline-vertical-padding . 0.6)
Can't take this out yet, or ledgers will go over bar lines.

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

http://codereview.appspot.com/5432070/diff/1/scm/define-grobs.scm#newcode1091
scm/define-grobs.scm:1091: (KeyCancellation
This one, too

[Keith, 2011-11-26 04:33:15]

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

http://codereview.appspot.com/5432070/diff/1/scm/define-grobs.scm#newcode2282
scm/define-grobs.scm:2282: (extra-spacing-height . (-1.0 . 1.0))
delete the second extra-spacing-height

[pkx166h, 2011-11-26 13:33:53]

Passes Make - reg tests here:
http://lilypond-stuff.1065243.n5.nabble.com/Tracker-2053-26-November-td502367...

Jam

[Keith, 2011-11-26 20:55:50]

Mike, Maybe change-clefs should not get the full extra-spacing-height by
default.
Engravers tend to tuck change-clefs in with the least possible disruption of the
note spacing.

Also, the test 'dots.ly' shows a ledger line sliding under a bar line.
I think the (-1 . +1) extra-spacing height for the BarLine is not quite enough. 
LilyPond gives ledger line columns only the minimum necessary extra-spacing
height, which I think is correct because traditionally and practically people
slide neighboring accidentals under ledger lines.

Also, there are still two definitions for extra-spacing height in TimeSignature.

[mike_apollinemike.com, 2011-11-26 22:48:36]

On Nov 26, 2011, at 9:56 PM, Keith OHara wrote:

> Mike, Maybe change-clefs should not get the full extra-spacing-height by
default.

I could do this if I could call Item::break_visible before line breaking (which
would let me know which clefs are change clefs as opposed to
beginning-of-line-clefs).  However, I remember Neil saying that this was a bad
idea, although I don't remember why.  Any ideas why this call isn't Kosher?

> Engravers tend to tuck change-clefs in with the least possible disruption of
the note spacing.
> 
> Also, the test 'dots.ly' shows a ledger line sliding under a bar line.
> I think the (-1 . +1) extra-spacing height for the BarLine is not quite
enough.  LilyPond gives ledger line columns only the minimum necessary
extra-spacing height, which I think is correct because traditionally and
practically people slide neighboring accidentals under ledger lines.
> 

Perhaps a stupid question, but why does LilyPond even take ledger lines into
account if they have no vertical or horizontal extent?  How do they factor into
the spacing calculations?

> Also, there are still two definitions for extra-spacing height in
TimeSignature.<DebussyToccataPourPiano.Peters.png><DebussyPArab.Peters.png><DebussyPArab.DurandFils.png><Cahier_dEsquisses.SchottFrères.png><DebussyToccataPourPiano.Fromont.png>

Will fix.  I see that you too have found in Debussy a gold mine of
NonMusicalPaperColumn quandaries.  I am convinced that he anticipated the
existence of LilyPond and purposefully composed music to complicate our lives.

Cheers,
MS

[Keith, 2011-11-27 00:21:00]

On Sat, 26 Nov 2011 14:48:29 -0800, mike@apollinemike.com
<mike@apollinemike.com> wrote:

> On Nov 26, 2011, at 9:56 PM, Keith OHara wrote:
>
>> Mike, Maybe change-clefs should not get the full extra-spacing-height by
default.
>
> I could do this if I could call Item::break_visible before line breaking
(which would let me know which clefs are change clefs as opposed to
beginning-of-line-clefs).  However, I remember Neil saying that this was a bad
idea, although I don't remember why.  Any ideas why this call isn't Kosher?
>

Oops. I was thinking the smaller change clef was a different "grob", but it is
not.

'break_visible' calls break_status_dir() to look up whether an item is visible
based on the results of line-breaking, so you don't want that.  Now I see that
the clef:calc_glyph_name() does the same.

I think all the Clefs that exist before line-breaking should be change clefs,
with the begin-of-line clefs added afterwards. Probably best if no Clefs use the
pure-from-neighbor height, certainly not CueClefs.

The distinction between beginning-of-line and middle-of-line spacing is made
with 'space-alist'.  Most of the spacing based on the meaning, rather than
extent, of symbols is done with that system.
Actually, your "don't-hang-over-me" rule would fit there nicely as a new type in
a 'space-alist' to go alongside "minimum-space", "semi-fixed-space" etc.   It
could be "no-overlap-space" and use the extent of the Item and the next-note
column rather than their skylines.

> Perhaps a stupid question, but why does LilyPond even take ledger lines into
account if they have no vertical or horizontal extent?  How do they factor into
the spacing calculations?
>

Neighboring note-columns are allowed to tuck under ledger lines, but not within
the ledger lines.   {e'''8 cis'' e''' fis'' }
The NoteHead has extra-spacing-height to enforce this rule.

[mike_apollinemike.com, 2011-11-27 10:17:36]

On Nov 27, 2011, at 1:21 AM, Keith OHara wrote:

> On Sat, 26 Nov 2011 14:48:29 -0800, mike@apollinemike.com
<mike@apollinemike.com> wrote:
> 
>> On Nov 26, 2011, at 9:56 PM, Keith OHara wrote:
>> 
>>> Mike, Maybe change-clefs should not get the full extra-spacing-height by
default.
>> 
>> I could do this if I could call Item::break_visible before line breaking
(which would let me know which clefs are change clefs as opposed to
beginning-of-line-clefs).  However, I remember Neil saying that this was a bad
idea, although I don't remember why.  Any ideas why this call isn't Kosher?
>> 
> 
> Oops. I was thinking the smaller change clef was a different "grob", but it is
not.
> 
> 'break_visible' calls break_status_dir() to look up whether an item is visible
based on the results of line-breaking, so you don't want that.  Now I see that
the clef:calc_glyph_name() does the same.
> 

But break_status_dir doesn't seem to be contingent on line breaking (again, I
could be wrong).  It actually seems to be the opposite - line breaking can't
happen w/o break_status_dir being called first.

Direction
Item::break_status_dir () const
{
  if (original ())
    {
      Item *i = dynamic_cast<Item *> (original ());

      return (i->broken_to_drul_[LEFT] == this) ? LEFT : RIGHT;
    }
  else
    return CENTER;
}

The only function call within this function is to original (), which itself is a
simple getter that just returns a pointer to original_.
broken_to_drul_ is populated with copy_breakable_items (), which itself is
called via discretionary_processing (), which is called first in
System::pre_processing () (before any of the springs and rods calculations take
place).  So, by the time any pure height approximation is required, it seems
that the breaking of items into pieces would already be done and that a function
call to Item::break_status_dir would be safe.

> I think all the Clefs that exist before line-breaking should be change clefs,
with the begin-of-line clefs added afterwards. Probably best if no Clefs use the
pure-from-neighbor height, certainly not CueClefs.
> 

If my logic above holds, then this can be accomplished while preserving one grob
(Clef).

> The distinction between beginning-of-line and middle-of-line spacing is made
with 'space-alist'.  Most of the spacing based on the meaning, rather than
extent, of symbols is done with that system.
> Actually, your "don't-hang-over-me" rule would fit there nicely as a new type
in a 'space-alist' to go alongside "minimum-space", "semi-fixed-space" etc.   It
could be "no-overlap-space" and use the extent of the Item and the next-note
column rather than their skylines.
> 

I've toyed around with this idea - it would take me a while to figure out how to
integrate it into Staff_spacing::get_spacing, but it seems completely
appropriate.  Perhaps a last 2011 or first 2012 project.

>> Perhaps a stupid question, but why does LilyPond even take ledger lines into
account if they have no vertical or horizontal extent?  How do they factor into
the spacing calculations?
>> 
> 
> Neighboring note-columns are allowed to tuck under ledger lines, but not
within the ledger lines.   {e'''8 cis'' e''' fis'' }
> The NoteHead has extra-spacing-height to enforce this rule.
> 

Ah, all is clear.  Note_head::include_ledger_line_height goes to 1 unit away
from the staff.  So the extra spacing height for bar lines should be something
like 1.01.  I'll give that a shot.

Cheers,
MS

[pkx166h, 2011-11-27 12:05:23]

Passes make. Reg tests all here:

http://lilypond-stuff.1065243.n5.nabble.com/Tracker-2053-27-November-2011-td5...

James

[pkx166h, 2011-11-27 18:31:45]

Passes Make. Reg tests

http://lilypond-stuff.1065243.n5.nabble.com/Tracker-2053-27-November-2011-2-t...

James

[Keith, 2011-11-27 19:28:23]

On Sun, 27 Nov 2011 02:17:27 -0800, mike@apollinemike.com
<mike@apollinemike.com> wrote:

> But break_status_dir doesn't seem to be contingent on line breaking (again, I
could be wrong).  It actually seems to be the opposite - line breaking can't
happen w/o break_status_dir being called first.
>

That seems to be correct.
I have trouble following the object-oriented stuff, so I depended too much on
the variable names and assumed 'broken' referred to line-breaking.  Following
your description, 'broken' here does seem to mean "copied to make three copies"
so I see how you add height to only the copy that might appear at begin-of-line.

>> Actually, your "don't-hang-over-me" rule would fit there nicely as a new type
in a 'space-alist' to go alongside "minimum-space", "semi-fixed-space" etc.   It
could be "no-overlap-space" and use the extent of the Item and the next-note
column rather than their skylines.
>>
>
> I've toyed around with this idea - it would take me a while to figure out how
to integrate it into Staff_spacing::get_spacing, but it seems completely
appropriate.  Perhaps a last 2011 or first 2012 project.

I might have made a bad suggestion there.  The space-alists might be applied
after columns are built up to include all Staves, Lyrics, etc., so it might not
be easy to apply the "don't-hang-over-me" to notes and decorations only.  Also,
it might not be the natural way to prevent ledgered notes from hanging over the
/following/ bar, because note-columns don't need space-alists for any other
reason.

[Carl, 2011-11-28 01:31:33]

LGTM