[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: make_draw_bezier_boxes: save work if thickness == 0.0 (issue 5517300
|
From: |
Han-Wen Nienhuys |
|
Subject: |
Re: make_draw_bezier_boxes: save work if thickness == 0.0 (issue 551730043 by address@hidden) |
|
Date: |
Fri, 24 Apr 2020 23:05:54 +0200 |
On Fri, Apr 24, 2020 at 8:56 PM <address@hidden> wrote:
>
>
> https://codereview.appspot.com/551730043/diff/583770043/lily/stencil-integral.cc
> File lily/stencil-integral.cc (right):
>
> https://codereview.appspot.com/551730043/diff/583770043/lily/stencil-integral.cc#newcode496
> lily/stencil-integral.cc:496: break;
> I think that is less readable than desirable. Instead how about putting
>
> if (th == 0.0)
> break;
>
> behind the push? That makes very obvious that only one point is being
> pushed without further cleverness.
Yeah, that's better.
> Either way you'll be checking thickness twice if its non-zero. The
> trivial way to avoid that would be to simply unfold the loop. Like
>
> points[DOWN].push_back (scm_transform (trans, curve.control_[0] + DOWN *
> normal);
> if (th != 0.0)
> points[UP].push_back (scm_transform (trans, curve.control_[0] + UP *
> normal;
the UP/DOWN manipulation has been a frequent source of errors in cut &
paste code, so I'd like to keep the loops.
> Written like that, it actually becomes far from trivial to see why this
> optimisation would be valid, so maybe add a comment explaining it for
> the sake of human readers?
The thickness check is cheap. The expensive bit is calculating
directions and bezier points. I added a comment.
--
Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen