Fix for Issue 620.

Fix for Issue 620.

Well, kinda...  It fixes all of the issues merged into it, but this
could be more generic.

[pkx166h, 2011-07-20 20:22:19]

It passes make, but I get a lot of reg test issues

Mainly I can see:
+programming error: Improbable offset for stencil: -nan staff space
+Setting to zero.
+continuing, cross fingers
+programming error: Improbable offset for stencil: -nan staff space
+Setting to zero.
+continuing, cross fingers

-- on a lot of tests

actually looking down the list there are a lot of problems with this reg test
comparisons. Too many to list and I don't know how to attach them all.

[mike_apollinemike.com, 2011-07-21 06:16:27]

On Jul 20, 2011, at 10:22 PM, pkx166h@gmail.com wrote:

> It passes make, but I get a lot of reg test issues
> 
> Mainly I can see:
> +programming error: Improbable offset for stencil: -nan staff space
> +Setting to zero.
> +continuing, cross fingers
> +programming error: Improbable offset for stencil: -nan staff space
> +Setting to zero.
> +continuing, cross fingers
> 
> -- on a lot of tests
> 
> actually looking down the list there are a lot of problems with this reg
> test comparisons. Too many to list and I don't know how to attach them
> all.
> 
> http://codereview.appspot.com/4814041/


I'll be working more on the implementation today, thanks!

Cheers,
MS

[MikeSol, 2011-07-21 10:28:40]

On 2011/07/21 06:16:27, mike_apollinemike.com wrote:
> On Jul 20, 2011, at 10:22 PM, mailto:pkx166h@gmail.com wrote:
> 
> > It passes make, but I get a lot of reg test issues
> > 
> > Mainly I can see:
> > +programming error: Improbable offset for stencil: -nan staff space
> > +Setting to zero.
> > +continuing, cross fingers
> > +programming error: Improbable offset for stencil: -nan staff space
> > +Setting to zero.
> > +continuing, cross fingers
> > 
> > -- on a lot of tests
> > 
> > actually looking down the list there are a lot of problems with this reg
> > test comparisons. Too many to list and I don't know how to attach them
> > all.
> > 
> > http://codereview.appspot.com/4814041/
> 
> 
> I'll be working more on the implementation today, thanks!
> 
> Cheers,
> MS

It works now & passes regtest.  I've also added a regtest to show it in action.

Cheers,
MS

[Neil Puttock, 2011-07-23 20:56:27]

http://codereview.appspot.com/4814041/diff/6001/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (right):

http://codereview.appspot.com/4814041/diff/6001/scm/define-grob-properties.sc...
scm/define-grob-properties.scm:1000: (core-interfaces ,list "Core interfaces to
be used for positioning.")
Yuck.

If you have to do it like this, I think it would be better to define a new
interface which you add to the relevant grobs (though even that seems dubious)

[mike_apollinemike.com, 2011-07-24 08:55:40]

On Jul 23, 2011, at 10:56 PM, n.puttock@gmail.com wrote:

> 
> http://codereview.appspot.com/4814041/diff/6001/scm/define-grob-properties.scm
> File scm/define-grob-properties.scm (right):
> 
>
http://codereview.appspot.com/4814041/diff/6001/scm/define-grob-properties.sc...
> scm/define-grob-properties.scm:1000: (core-interfaces ,list "Core
> interfaces to be used for positioning.")
> Yuck.
> 
> If you have to do it like this, I think it would be better to define a
> new interface which you add to the relevant grobs (though even that
> seems dubious)
> 

Why is it a bad thing to do it this way?  Currently, the Beam_collision_engraver
implements dynamic filtering based on interface, and I don't think there's a
problem with that (it is the only way to make it ignore certain grobs on the
fly).
Creating a new interface would be OK but would make it harder to filter out
interfaces on the fly (people would have to override a grob's "meta" property,
which seems hard).

The problem at hand is that the PaperColumn groups together a slew of grobs
indiscriminately that elements then get positioned against.  If we want only
certain grobs in this paper column to count in the positioning calculations, I
think it makes sense to to do it like this.

~Mike

[Neil Puttock, 2011-07-24 21:22:36]

On 24 July 2011 09:55, mike@apollinemike.com <mike@apollinemike.com> wrote:

> Why is it a bad thing to do it this way?  Currently, the
Beam_collision_engraver implements dynamic filtering based on interface, and I
don't think there's a problem with that (it is the only way to make it ignore
certain grobs on the fly).

I don't like the name.  We already have `core interfaces'; they're
grob-interface, item-interface and spanner-interface.

> Creating a new interface would be OK but would make it harder to filter out
interfaces on the fly (people would have to override a grob's "meta" property,
which seems hard).

Do you have a scenario whereby a user might want to override this?

Cheers,
Neil

[mike_apollinemike.com, 2011-07-25 11:12:47]

On Jul 24, 2011, at 11:22 PM, Neil Puttock wrote:

> On 24 July 2011 09:55, mike@apollinemike.com <mike@apollinemike.com> wrote:
> 
>> Why is it a bad thing to do it this way?  Currently, the
Beam_collision_engraver implements dynamic filtering based on interface, and I
don't think there's a problem with that (it is the only way to make it ignore
certain grobs on the fly).
> 
> I don't like the name.  We already have `core interfaces'; they're
> grob-interface, item-interface and spanner-interface.
> 

Ah, OK.  I can change it. How about horizontal-alignment-interfaces?

>> Creating a new interface would be OK but would make it harder to filter out
interfaces on the fly (people would have to override a grob's "meta" property,
which seems hard).
> 
> Do you have a scenario whereby a user might want to override this?
> 

No, but I think the question should be "are we 100% sure that a user wouldn't
want to override this?"  As I am not an authority on the subject and I just
picked the interfaces populating the core-interfaces list based on personal
predilections, I cannot guarantee that this list is not something that shouldn't
be overridden in certain instances.  When in doubt, I try to leave as much of my
code as possible open to Scheme overrides.

Cheers,
MS

[MikeSol, 2011-07-27 16:45:02]

A new version of this patch is up w/ the name of the property changed to
bound-alignment-interfaces.

Cheers,
MS

[pkx166h, 2011-07-27 19:32:24]

Passes Make and reg tests

[Graham Percival (old account), 2011-08-03 19:43:52]

cannot apply to master due to fixcc.

[MikeSol, 2011-08-05 00:21:45]

On 2011/08/03 19:43:52, Graham Percival wrote:
> cannot apply to master due to fixcc.

New patchset uploaded - should apply.
Neil - whenever you get the chance, I'd like to hear more of what you have to
say about it.  If you think that custom engravers would be a better idea here
(or even non-custom engravers), I'd like to fully discuss it on the list.

Cheers,
MS

[Neil Puttock, 2011-08-14 20:41:18]

On 2011/08/05 00:21:45, MikeSol wrote:

> Neil - whenever you get the chance, I'd like to hear more of what you have to
> say about it.  If you think that custom engravers would be a better idea here
> (or even non-custom engravers), I'd like to fully discuss it on the list.

I think an engraver approach would be too complicated for this.

Ideally, I'd like a solution which doesn't require a new property, but that's
probably not feasible.

Cheers,
Neil

[Neil Puttock, 2011-08-14 21:01:24]

http://codereview.appspot.com/4814041/diff/24001/lily/axis-group-interface.cc
File lily/axis-group-interface.cc (right):

http://codereview.appspot.com/4814041/diff/24001/lily/axis-group-interface.cc...
lily/axis-group-interface.cc:82:
Axis_group_interface::relative_maybe_bound_group_extent (vector<Grob*> const
&elts,
vector<Grob *>

http://codereview.appspot.com/4814041/diff/24001/lily/axis-group-interface.cc...
lily/axis-group-interface.cc:109: vector<Grob*> new_elts;
vector<Grob *>

http://codereview.appspot.com/4814041/diff/24001/lily/axis-group-interface.cc...
lily/axis-group-interface.cc:118: if (new_elts.size () == 0)
!new_elts.size ()

http://codereview.appspot.com/4814041/diff/24001/lily/axis-group-interface.cc...
lily/axis-group-interface.cc:846: "bound-alignment-interfaces "
tab

http://codereview.appspot.com/4814041/diff/24001/lily/axis-group-interface.cc...
lily/axis-group-interface.cc:847: "default-staff-staff-spacing "
tab

http://codereview.appspot.com/4814041/diff/24001/lily/hairpin.cc
File lily/hairpin.cc (right):

http://codereview.appspot.com/4814041/diff/24001/lily/hairpin.cc#newcode144
lily/hairpin.cc:144: Interval e = (Axis_group_interface::has_interface (b)
tab

http://codereview.appspot.com/4814041/diff/24001/lily/include/axis-group-inte...
File lily/include/axis-group-interface.hh (right):

http://codereview.appspot.com/4814041/diff/24001/lily/include/axis-group-inte...
lily/include/axis-group-interface.hh:50: static Interval
relative_maybe_bound_group_extent (vector<Grob*> const &list,
vector<Grob *>

http://codereview.appspot.com/4814041/diff/24001/lily/include/axis-group-inte...
lily/include/axis-group-interface.hh:51: Grob *common, Axis, bool);
indent

http://codereview.appspot.com/4814041/diff/24001/lily/line-spanner.cc
File lily/line-spanner.cc (right):

http://codereview.appspot.com/4814041/diff/24001/lily/line-spanner.cc#newcode112
lily/line-spanner.cc:112: Interval extent = (Paper_column::has_interface
(bound_grob)
if you change this to Axis_group_interface::has_interface () you should be able
to remove the special case in separation-item.cc for arpeggios (and restore the
old code which was replaced for issue 591 in rhythmic-column-engraver.cc)

http://codereview.appspot.com/4814041/diff/24001/lily/line-spanner.cc#newcode118
lily/line-spanner.cc:118: details);
tabs

[MikeSol, 2011-08-17 09:49:24]

Pushed as ac7aef03ab9d459a6ea6f03d9c127be150871dd4.

Cheers,
MS