Removes cross-staff beams from the collision grob array

Removes cross-staff beams from the collision grob array

[MikeSol, 2011-04-23 11:35:25]

This may be killing a fly with a sledgehammer.

Another alternative may be to create an internal property for beams called
"automatic" that is set to true if a beam is an autobeam and then apply the fix
in this patch only to beams that are cross staff _and_ automatically created (in
issue 1632, the problem goes away if you use manual beams).

[Neil Puttock, 2011-04-23 11:54:59]

On 2011/04/23 11:35:25, MikeSol wrote:

> Another alternative may be to create an internal property for beams called
> "automatic" that is set to true if a beam is an autobeam

You can use Grob_info::event_cause () for this since manual beams are the only
beams whose cause is a stream event.

[MikeSol, 2011-04-23 12:16:55]

On 2011/04/23 11:54:59, Neil Puttock wrote:
> On 2011/04/23 11:35:25, MikeSol wrote:
> 
> > Another alternative may be to create an internal property for beams called
> > "automatic" that is set to true if a beam is an autobeam
> 
> You can use Grob_info::event_cause () for this since manual beams are the only
> beams whose cause is a stream event.

Done.

[hanwenn, 2011-04-24 01:03:23]

http://codereview.appspot.com/4432063/diff/3001/lily/beam-collision-engraver.cc
File lily/beam-collision-engraver.cc (right):

http://codereview.appspot.com/4432063/diff/3001/lily/beam-collision-engraver....
lily/beam-collision-engraver.cc:110: && !covered_grobs_[j].event_cause ())))
rather than doing this kind of decision in the engraver, can you do it in the
typesetting backend itself?  You could make cross-staff beams ignore just
stems/beams and continue doing collisions for clefs, accidentals, etc.

[MikeSol, 2011-04-24 01:18:07]

On 2011/04/24 01:03:23, hanwenn wrote:
>
http://codereview.appspot.com/4432063/diff/3001/lily/beam-collision-engraver.cc
> File lily/beam-collision-engraver.cc (right):
> 
>
http://codereview.appspot.com/4432063/diff/3001/lily/beam-collision-engraver....
> lily/beam-collision-engraver.cc:110: && !covered_grobs_[j].event_cause ())))
> rather than doing this kind of decision in the engraver, can you do it in the
> typesetting backend itself?  You could make cross-staff beams ignore just
> stems/beams and continue doing collisions for clefs, accidentals, etc.

Would this need to be done in both beam-quanting.cc and beam.cc?

In general, I am reticent to make grob-by-grob exceptions to a procedure that
should seemingly be grob-agnostic.

I have a semi-idea for how to fix this, but it'll take me a couple days to make
it into a full idea.  The gist is to move the beam-collision-engraver up to the
score level and then populate the covered-grob list based on the beam's staves
by using grobs that share a staff symbol with a given stem around that stem's
rank.  This way, cross-staff beams can take grobs from all staves to which they
belong into consideration.

[MikeSol, 2011-04-24 12:04:25]

On 2011/04/24 01:18:07, MikeSol wrote:
> On 2011/04/24 01:03:23, hanwenn wrote:
> >
>
http://codereview.appspot.com/4432063/diff/3001/lily/beam-collision-engraver.cc
> > File lily/beam-collision-engraver.cc (right):
> > 
> >
>
http://codereview.appspot.com/4432063/diff/3001/lily/beam-collision-engraver....
> > lily/beam-collision-engraver.cc:110: && !covered_grobs_[j].event_cause ())))
> > rather than doing this kind of decision in the engraver, can you do it in
the
> > typesetting backend itself?  You could make cross-staff beams ignore just
> > stems/beams and continue doing collisions for clefs, accidentals, etc.
> 
> Would this need to be done in both beam-quanting.cc and beam.cc?
> 
> In general, I am reticent to make grob-by-grob exceptions to a procedure that
> should seemingly be grob-agnostic.
> 
> I have a semi-idea for how to fix this, but it'll take me a couple days to
make
> it into a full idea.  The gist is to move the beam-collision-engraver up to
the
> score level and then populate the covered-grob list based on the beam's staves
> by using grobs that share a staff symbol with a given stem around that stem's
> rank.  This way, cross-staff beams can take grobs from all staves to which
they
> belong into consideration.

I have a test patch on:
http://codereview.appspot.com/4430063

More to come on the devel list...

[hanwenn, 2011-04-24 13:42:08]

On Sat, Apr 23, 2011 at 10:18 PM,  <mtsolo@gmail.com> wrote:
> On 2011/04/24 01:03:23, hanwenn wrote:
>
>
http://codereview.appspot.com/4432063/diff/3001/lily/beam-collision-engraver.cc
>>
>> File lily/beam-collision-engraver.cc (right):
>
>
>
http://codereview.appspot.com/4432063/diff/3001/lily/beam-collision-engraver....
>>
>> lily/beam-collision-engraver.cc:110: && !covered_grobs_[j].event_cause
>
> ())))
>>
>> rather than doing this kind of decision in the engraver, can you do it
>
> in the
>>
>> typesetting backend itself?  You could make cross-staff beams ignore
>
> just
>>
>> stems/beams and continue doing collisions for clefs, accidentals, etc.
>
> Would this need to be done in both beam-quanting.cc and beam.cc?

beam.cc already has a filtering pass in shift_positition_to_valid().
If you add the filtering there it will work for beam-quanting.cc too

In general, I think it is better for the backend to deal with this, as
the backend should be robust against 'invalid' configuration by the
engraver system anyway.

> In general, I am reticent to make grob-by-grob exceptions to a procedure
> that should seemingly be grob-agnostic.

it's already not grob-agnostic: it is aware of the difference between
beams and stems.

are you sure you're not just less hesitant about changing the engraver
than the backend because you wrote it yourself, and are therefore more
familiar with it?

> I have a semi-idea for how to fix this, but it'll take me a couple days
> to make it into a full idea.  The gist is to move the
> beam-collision-engraver up to the score level and then populate the
> covered-grob list based on the beam's staves by using grobs that share a
> staff symbol with a given stem around that stem's rank.  This way,
> cross-staff beams can take grobs from all staves to which they belong
> into consideration.

For getting x-staff beaming completely right, I agree that the
engraver needs to move up.

I'm a bit hesitant about bolting this onto the current design.

What are the performance characteristics for the beam-coll engraver at
staff level for large scores (eg. 50 staves)?  The looping (all beams
x all grobs that start around a beams start point) seems suboptimal to
me.

Another option:

  map<context*, vector<grob*>>  context_grobs_  // sorted

and a

 map<grob*, context*> covered_ // staff origin for each grob

ie. a sorted list per context, and then

  int min, max;
  for (b : beams) { // beams is sorted
    set<context*> contexts = ...  // get beam's contexts

    for (cont : context_grobs_) {
      // update min/max indices so they cover b
    }

    for (c : contexts) {
      grobs = context_grobs_[c]
      start, end  = .. . // binary search to start point/end point of
beam; use a global minimum/maximum to initate search
      for (cov : grobs[start : .. ])  {
        // consider adding cov to b.
      }
    }
  }

on second thought, this may be too tricky for a first cut.


>
> http://codereview.appspot.com/4432063/
>



-- 
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen