Re: Issue 4274: Fix a cyclic dependency in Rest_collision (issue 193590043 by address@hidden)

[k-ohara5a5a]

Are you sure there is a cycle?

> Rest_collision::calc_positioning_done now uses the pure
> height of rests because the height of certain rests depends
> on their position on the staff, which Rest_collision is
> responsible for adjusting.

Rest_collision does use the height of the rests to determine the height
of the rests, but I do not see where calling the Rest::height() function
can call Rest_collision() cyclically.

This calculation seems to be of the type
  h = 3;
  if (moon is full)
   h = h + 2;

Much of LilyPond layout code is done through function calls, so there is
often risk of cycles like
  int h(void) {
    int ret = 3;
    if (moon is full)
      ret = h() + 2;
    return ret;
  }
but I don't see how that can happen here.

Is there some LilyPond input that causes infinite recursion or gives the
error message from the trap for cyclic dependencies ?


https://codereview.appspot.com/193590043/diff/1/lily/rest-collision.cc
File lily/rest-collision.cc (right):

https://codereview.appspot.com/193590043/diff/1/lily/rest-collision.cc#newcode195
lily/rest-collision.cc:195: Real diff = d * ((last_y - y) /
staff_space);
The idea here seemed to be that any outer-voice rests are offset based
on inner-voice rests, which were just placed by the translate() call
above.   pure_height() seems to freshly look up the stencils so it would
ignore any previous positioning.  The result is used in the translate()
below which offsets the rests relative to their most recent position, so
the old code using the most recent positions seems correct.

https://codereview.appspot.com/193590043/