Avoids calling pure calculations too far downstream and avoids triggering quanting too far upstream

Avoids calling pure calculations too far downstream and avoids triggering quanting too far upstream

[Keith, 2013-08-15 05:35:49]

Of course this is two commits.

I do not know of any reason to distinguish the concept of 'cross-staff' from
your new 
height_depends_on_grobs_from_another_vertical_
      axis_group()
In most cases, 'cross-staff' means exactly that the height (Y-extent) depends on
staff-spacing.

Could you mark the relevant object as 'cross-staff' when the cross-staff item is
added to it 'support', rather than search through its support later?

I am completely stumped how skipping some objects in computing skylines for
staff-spacing has anything to do with quanting.

https://codereview.appspot.com/12951044/diff/1/lily/side-position-interface.cc
File lily/side-position-interface.cc (right):

https://codereview.appspot.com/12951044/diff/1/lily/side-position-interface.c...
lily/side-position-interface.cc:274: // In the case of a stem, we will find a
note head as well
The comments are out of date

https://codereview.appspot.com/12951044/diff/1/lily/side-position-interface.c...
lily/side-position-interface.cc:284: && !is_direction (e->get_property_data
("direction")))))
Now you skip *all* stems whose direction is still uncertain, while before you
skipped only cross-staff stems of uncertain direction.
That seems fine, but makes a lot more sense if the decisions about stems are all
together in the next "if()continue" down below.

[MikeSol, 2013-08-15 05:50:10]

Changes comments

[mike7, 2013-08-15 05:51:36]

On 15 août 2013, at 08:35, k-ohara5a5a@oco.net wrote:

> Of course this is two commits.
> 

probably three (axis-group-interface.cc and two independent changes in
side-position-interface.cc).

> I do not know of any reason to distinguish the concept of 'cross-staff'
> from your new
> height_depends_on_grobs_from_another_vertical_
>      axis_group()
> In most cases, 'cross-staff' means exactly that the height (Y-extent)
> depends on staff-spacing.
> 

cross-staff is a non-exhaustive subset of this.

your original fix to 3363 was the right way to go, but it would retrigger 3385,
even with my fix.  i prefer to keep it and fix the bug that it reveals, which is
what this patch does.

to fix 3385, we need for beam quanting not to be triggered by vertical axis
group skyline calculations, which requires weeding out both cross-staff-grobs
and all grobs whose heights depend on cross-staff grobs.  this includes the
ottava bracket, which we agreed not to mark as cross-staff.

> Could you mark the relevant object as 'cross-staff' when the cross-staff
> item is added to it 'support', rather than search through its support
> later?
> 

see above: this would mean that ottava brackets are marked as cross-staff, which
we don't want.

> I am completely stumped how skipping some objects in computing skylines
> for staff-spacing has anything to do with quantizing.

skipping means that quanting is not triggered.

> 

>
https://codereview.appspot.com/12951044/diff/1/lily/side-position-interface.cc
> File lily/side-position-interface.cc (right):
> 
>
https://codereview.appspot.com/12951044/diff/1/lily/side-position-interface.c...
> lily/side-position-interface.cc:274: // In the case of a stem, we will
> find a note head as well
> The comments are out of date

ok

>
https://codereview.appspot.com/12951044/diff/1/lily/side-position-interface.c...
> lily/side-position-interface.cc:284: && !is_direction
> (e->get_property_data ("direction")))))
> Now you skip *all* stems whose direction is still uncertain, while
> before you skipped only cross-staff stems of uncertain direction.
> That seems fine, but makes a lot more sense if the decisions about stems
> are all together in the next "if()continue" down below.
> 

I prefer to keep this as is, as they are two conceptually different things.  The
first "if()continue" tests to make sure that certain things don't happen in pure
lookups.  The second excludes stems that aren't conceptually supports because
they don't point in the same direction as the supported grob.  I've changed the
comments a bit to better reflect this.

> https://codereview.appspot.com/12951044/

[Keith, 2013-08-15 05:51:56]

On 2013/08/15 05:35:49, Keith wrote:

> In most cases, 'cross-staff' means exactly that the height (Y-extent) depends
on
> staff-spacing.

Oh.  It is the ottava bracket from the example for issue 3385 that is not
cross-staff, 
but whose positioning references a cross-staff beam.

But if we change the pitches to raise that beam, the ottava bracket does not in
fact move in response (either before or after this patch).

If we had ever succeeded in moving spanners in response to cross-staff grobs
that they are side-positioned over, we should probably mark those spanners as
cross-staff.

If the spanners have never successfully responded to the position of cross-staff
support, we should just skip cross-staff items from the support.

[mike7, 2013-08-15 05:55:03]

On 15 août 2013, at 08:51, k-ohara5a5a@oco.net wrote:

> On 2013/08/15 05:35:49, Keith wrote:
> 
>> In most cases, 'cross-staff' means exactly that the height (Y-extent)
> depends on
>> staff-spacing.
> 
> Oh.  It is the ottava bracket from the example for issue 3385 that is
> not cross-staff,
> but whose positioning references a cross-staff beam.
> 
> But if we change the pitches to raise that beam, the ottava bracket does
> not in fact move in response (either before or after this patch).
> 
> If we had ever succeeded in moving spanners in response to cross-staff
> grobs that they are side-positioned over, we should probably mark those
> spanners as cross-staff.
> 
> If the spanners have never successfully responded to the position of
> cross-staff support, we should just skip cross-staff items from the
> support.
> 

This is a great idea, but I'm not sure the best way to implement it.
It's not possible at the engraver stage when the grobs are originally put in the
list.
And it's kinda kludgy to check for it every time we extract the grob set.

But it is a great idea if in fact the spanner is not responding to these objects
at all.

Cheers,
MS

[Keith, 2013-08-15 06:03:28]

On 2013/08/15 05:51:36, mike7 wrote:
> On 15 août 2013, at 08:35, mailto:k-ohara5a5a@oco.net wrote:
>
> > Could you mark the relevant object as 'cross-staff' when the cross-staff
> > item is added to it 'support', rather than search through its support
> > later?
> 
> see above: this would mean that ottava brackets are marked as cross-staff,
which
> we don't want.
> 
Well, the thing we did not want was to have ottava brackets ignored for
staff-spacing.
The bracket overlaps the next staff the same way, whether you call it
'cross-staff' or
function_to_deal_with_one_special_case()

chUp = \change Staff = "upper"
chDn = \change Staff = "lower"
\new PianoStaff << \new Staff = "lower" { s1 }
\new Staff = "upper"
{ \relative c' { \ottava #1 c'8 \chDn c \chUp c \chDn c \chUp f''2 } } >>

[mike7, 2013-08-15 06:12:15]

On 15 août 2013, at 09:03, k-ohara5a5a@oco.net wrote:

> On 2013/08/15 05:51:36, mike7 wrote:
>> On 15 août 2013, at 08:35, mailto:k-ohara5a5a@oco.net wrote:
> 
>> > Could you mark the relevant object as 'cross-staff' when the
> cross-staff
>> > item is added to it 'support', rather than search through its
> support
>> > later?
> 
>> see above: this would mean that ottava brackets are marked as
> cross-staff, which
>> we don't want.
> 
> Well, the thing we did not want was to have ottava brackets ignored for
> staff-spacing.
> The bracket overlaps the next staff the same way, whether you call it
> 'cross-staff' or
> function_to_deal_with_one_special_case()
> 
> chUp = \change Staff = "upper"
> chDn = \change Staff = "lower"
> \new PianoStaff << \new Staff = "lower" { s1 }
> \new Staff = "upper"
> { \relative c' { \ottava #1 c'8 \chDn c \chUp c \chDn c \chUp f''2 } }
> 

Ok, so, the bracket is ignoring the cross-staff stems that support it.
Do you know where in side-position-interface.cc this ignoring happens?

Cheers,
MS

[Keith, 2013-08-15 06:12:28]

> > If the spanners have never successfully responded to the position of
> > cross-staff support, we should just skip cross-staff items from the
> > support.
> > 
> 
> This is a great idea, but I'm not sure the best way to implement it.
> It's not possible at the engraver stage when the grobs are originally put in
the
> list.
> And it's kinda kludgy to check for it every time we extract the grob set.
>

https://codereview.appspot.com/12951044/diff/5001/lily/side-position-interfac...
File lily/side-position-interface.cc (right):

https://codereview.appspot.com/12951044/diff/5001/lily/side-position-interfac...
lily/side-position-interface.cc:305: aligns_to_cross_staff |= cross_staff;
You already check all the 'support' for cross-staffitude every time you unpack
the list.  I am beginning to doubt that the special case for
aligns_to_cross_staff has any effect.  

If the code has not succeeded in considering cross-staff support, skip them up
above
if (!me->"cross_staff" || e-> "cross-staff" ) continue;

https://codereview.appspot.com/12951044/diff/5001/lily/side-position-interfac...
lily/side-position-interface.cc:373: dim.set_minimum_height (dim.max_height ());
Really?  If a spanner lies over one cross-staff grob, draw a box around *all*
the grobs in the support, whether cross-staff or not ?

[Keith, 2013-08-15 06:55:13]

On 2013/08/15 06:12:15, mike7 wrote:
> On 15 août 2013, at 09:03, mailto:k-ohara5a5a@oco.net wrote:
> > Well, the thing we did not want was to have ottava brackets ignored for
> > staff-spacing.
> > The bracket overlaps the next staff the same way,

> Ok, so, the bracket is ignoring the cross-staff stems that support it.
> Do you know where in side-position-interface.cc this ignoring happens?
> 
If you mean the stems in the upper staff, the ottava-bracket engraver ignores
them.
I think we want the ottava bracket to ignore them.

[Keith, 2013-08-15 07:31:05]

On 2013/08/15 06:12:28, Keith wrote:
> 
> If the code has not succeeded in considering cross-staff support, skip them up
> above
> if (!me->"cross_staff" || e-> "cross-staff" ) continue;
> 

I meant this to be an .AND.

if (!me->"cross_staff" && e->"cross-staff" ) continue;