Re: Avoids calling pure calculations too far downstream and avoids triggering quanting too far upstream (issue 12951044)

[Mike Solomon]

On 15 août 2013, at 08:35, address@hidden 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.cc#newcode274
> 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.cc#newcode284
> 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/