On 2020/04/24 17:12:48, hanwenn wrote: > https://codereview.appspot.com/547980044/diff/567490043/lily/include/skyline.hh > File lily/include/skyline.hh (right): > > https://codereview.appspot.com/547980044/diff/567490043/lily/include/skyline.hh#newcode152 > lily/include/skyline.hh:152: } > On 2020/04/24 16:33:04, hahnjo wrote: > > Why do you need all of this in the header file? As far as I can see, nobody > else > > is calling these methods, so the argument of inlinining does not apply. > > trimmed this a bit. > > https://codereview.appspot.com/547980044/diff/567490043/lily/skyline-pair.cc > File lily/skyline-pair.cc (left): > > https://codereview.appspot.com/547980044/diff/567490043/lily/skyline-pair.cc#oldcode100 > lily/skyline-pair.cc:100: Skyline_pair::print_points () const > On 2020/04/24 16:33:04, hahnjo wrote: > > You should not delete a function without removing the declaration in > > lily/include/skyline-pair.hh. Also this seems unrelated to the performance > > improvements. > > the print() function prints the building in terms of y-intercept at x=0.0 + > slope. Since we don't store the y-intercept anymore, that is useless now. The > points are also more convenient for debugging, which is the only objective of > this function. > > https://codereview.appspot.com/547980044/diff/567490043/lily/skyline.cc > File lily/skyline.cc (right): > > https://codereview.appspot.com/547980044/diff/567490043/lily/skyline.cc#newcode780 > lily/skyline.cc:780: Skyline::to_segments (Axis horizon_axis) const > On 2020/04/24 16:33:04, hahnjo wrote: > > What's the advantage of this code over the old method? > > the buildings making up the skylines have become non-contiguous, so you can't > simply connect all the points any more. (If you'd do that, there would be > diagonal line segments printed in debug output that aren't really there.) > > https://codereview.appspot.com/547980044/diff/567490043/scm/define-grobs.scm > File scm/define-grobs.scm (right): > > https://codereview.appspot.com/547980044/diff/567490043/scm/define-grobs.scm#newcode1657 > scm/define-grobs.scm:1657: (stencil . ,ly:paper-column::print) > On 2020/04/24 16:33:04, hahnjo wrote: > > Is this change intentional? And intentionally in this review? > > thanks, no. I was already wondering why the cell count shot up in the profile > :-)
grmbl.- and now the speed difference is gone again. I hate my autoscaling CPU. https://codereview.appspot.com/547980044/
