Re: Gets vertical skylines from grob stencils (issue 5626052)

[k-ohara5a5a]

This is barely slower than master on an orchestral score and parts
  patch                 master
  real    16m16.228s    real      15m54.212s
  user    15m30.543s    user      15m5.920s
  sys     0m10.624s     sys       0m10.649s

but piano music (Chopin op45) takes 25% longer

  real    0m16.245s     real      0m12.862s
  user    0m15.573s     user      0m12.232s
  sys     0m0.254s      sys       0m0.217s

patch                 master
real    16m16.228s    real      15m54.212s
user    15m30.543s    user      15m5.920s
sys     0m10.624s     sys       0m10.649s

real    0m16.245s     real      0m12.862s
user    0m15.573s     user      0m12.232s
sys     0m0.254s      sys       0m0.217s

We should clean up the code, most importantly because having operations
that do not progress toward the goal is quite confusing, and
time-consuming, for any code maintainer.


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

http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode945
lily/axis-group-interface.cc:945: feature present in
tuplet-number-outside-staff-priority.
On 2012/08/17 17:16:25, MikeSol wrote:
> On 2012/08/17 08:12:56, Keith wrote:
> If you comment out the rider business, the vertical-skylines will not
be correct
> for axis-group-interface.

That is very subtle, very minor, changes only the debug output, not what
would normally be printed from that example, and is different from the
comment indicates.

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

http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc#newcode697
lily/axis-group-interface.cc:697: bumps.push_back (dir *
vertical_skyline_forest[dir][j][d].distance ((*pair)[-d]));
When d = dir, the direction we intend to move, this tells us how far we
need to move, but when d = -dir, this tells us by how far we have
encroached into the next obstacle, which we will eventually need to hop
over.

What you really want to put on the bumps list is how far we need to move
to hope over this next obstacle.  But we only tested the relevant
skylines for 'intersection' ... if only we had stached them distance().

http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc#newcode701
lily/axis-group-interface.cc:701: bumps.push_back (dir *
vertical_skyline_forest[dir][j][d].distance (to_flip));
This finds the distance to move such that the top skyline of the new
object just encloses, Russian egg doll style, one of the already-placed
skylines.  We never want that to be the final position, so don't put it
on the bumps list.

http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc#newcode732
lily/axis-group-interface.cc:732: pair->raise (min_bump);
  printf("bumping %.3f\n", min_bump);
shows that for a simple case
  \relative f {f'^"mouse" c'' f,,^"EEEEK" d''}
we bump around quite a lot:
"mouse"
bumping 0.567
bumping 1.894
"EEEK"
bumping 0.443
bumping 0.048
bumping 0.095
bumping 0.191
bumping 0.382
bumping 0.459
bumping 1.344
bumping 0.161
bumping 0.322
bumping 0.578

http://codereview.appspot.com/5626052/