Checks for grobs with circular parentage in the regtests.

Checks for grobs with circular parentage in the regtests.

Eliminates all circular parenting cases from the code base, allowing
for null pointers to be parents but issuing a programming error (this
should only happen for staves that will never see the light of day due
to bad code).

[MikeSol, 2011-07-15 20:11:38]

This gets rid of all circular parentage in LilyPond (at least all that's in the
regtests...).

[hanwenn, 2011-07-16 01:39:48]

http://codereview.appspot.com/4747045/diff/2001/lily/grob.cc
File lily/grob.cc (right):

http://codereview.appspot.com/4747045/diff/2001/lily/grob.cc#newcode528
lily/grob.cc:528: Grob::in_own_family_tree (Grob *g, Grob *orig)
I think this should take an axis argument, and check only one axis.

http://codereview.appspot.com/4747045/diff/2001/lily/include/grob.hh
File lily/include/grob.hh (right):

http://codereview.appspot.com/4747045/diff/2001/lily/include/grob.hh#newcode140
lily/include/grob.hh:140: static bool in_own_family_tree(Grob *g, Grob *orig);
this needs a small comment; could probably be normal method as well.

I think 

  a->has_ancestor(b)

would be more clear.

http://codereview.appspot.com/4747045/diff/2001/lily/pitched-trill-engraver.cc
File lily/pitched-trill-engraver.cc (right):

http://codereview.appspot.com/4747045/diff/2001/lily/pitched-trill-engraver.c...
lily/pitched-trill-engraver.cc:124: trill_group_->translate_axis ((unsmob_pitch
(scm_pitch)->steps () + c0 ) * 0.5,
you can't do typography (positioning) in any engraver.  This wil mess up
positioning with modified staff sizes.

[pkx166h, 2011-07-16 14:25:05]

passes make, but some reg tests are different. Trillspanner ones (trill is
shorter than before). And he 'Black box' test is showing up,

see: http://code.google.com/p/lilypond/issues/detail?id=1766#c2

for screenshots.

[mike_apollinemike.com, 2011-07-16 15:31:33]

On Jul 16, 2011, at 3:39 AM, hanwenn@gmail.com wrote:

> 
> http://codereview.appspot.com/4747045/diff/2001/lily/grob.cc
> File lily/grob.cc (right):
> 
> http://codereview.appspot.com/4747045/diff/2001/lily/grob.cc#newcode528
> lily/grob.cc:528: Grob::in_own_family_tree (Grob *g, Grob *orig)
> I think this should take an axis argument, and check only one axis.
> 
> http://codereview.appspot.com/4747045/diff/2001/lily/include/grob.hh
> File lily/include/grob.hh (right):
> 
>
http://codereview.appspot.com/4747045/diff/2001/lily/include/grob.hh#newcode140
> lily/include/grob.hh:140: static bool in_own_family_tree(Grob *g, Grob
> *orig);
> this needs a small comment; could probably be normal method as well.
> 
> I think
> 
>  a->has_ancestor(b)
> 
> would be more clear.
> 
> http://codereview.appspot.com/4747045/diff/2001/lily/pitched-trill-engraver.cc
> File lily/pitched-trill-engraver.cc (right):
> 
>
http://codereview.appspot.com/4747045/diff/2001/lily/pitched-trill-engraver.c...
> lily/pitched-trill-engraver.cc:124: trill_group_->translate_axis
> ((unsmob_pitch (scm_pitch)->steps () + c0 ) * 0.5,
> you can't do typography (positioning) in any engraver.  This wil mess up
> positioning with modified staff sizes.
> 

I'm gonna pull this patch out of consideration - both you and Neil are right
about checking parents on an axis-by-axis basis.  My desire to do this comes
from an incomplete understanding of what "parent" means in LilyPond speak.  I
now understand that grob A can be grob B's X parent while grob B is grob A's Y
parent.  In this case, the approach in my tuplet patch holds because I only
check along the Y_AXIS (as Carl suggested), which eliminates the recursive
problem.

Cheers,
MS

[hanwenn, 2011-07-18 00:50:15]

On Sat, Jul 16, 2011 at 12:31 PM, mike@apollinemike.com
<mike@apollinemike.com> wrote:

>
> I'm gonna pull this patch out of consideration - both you and Neil are right
about checking parents on an axis-by-axis basis.  My desire to do this comes
from an incomplete understanding of what "parent" means in LilyPond speak.  I
now understand that grob A can be grob B's X parent while grob B is grob A's Y
parent.  In this case, the approach in my tuplet patch holds because I only
check along the Y_AXIS (as Carl suggested), which eliminates the recursive
problem.


Also, code that actually creates loops would never finish compiling.  We do

  obj->relative_coordinate(system, axis)

in some place, and if there were a loop, this would never exit.

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