[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Thread skyline construction through stencil interpretation (issue 58
|
From: |
hanwenn |
|
Subject: |
Re: Thread skyline construction through stencil interpretation (issue 581960043 by address@hidden) |
|
Date: |
Mon, 27 Apr 2020 14:16:02 -0700 |
https://codereview.appspot.com/581960043/diff/561710044/lily/include/lazy-skyline-pair.hh
File lily/include/lazy-skyline-pair.hh (right):
https://codereview.appspot.com/581960043/diff/561710044/lily/include/lazy-skyline-pair.hh#newcode23
lily/include/lazy-skyline-pair.hh:23: #include <vector>
On 2020/04/27 19:56:59, hahnjo wrote:
> last please :)
Done.
https://codereview.appspot.com/581960043/diff/561710044/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):
https://codereview.appspot.com/581960043/diff/561710044/lily/stencil-integral.cc#newcode188
lily/stencil-integral.cc:188: if (i)
On 2020/04/27 19:56:59, hahnjo wrote:
> i > 0
Done.
https://codereview.appspot.com/581960043/diff/561710044/lily/stencil-integral.cc#newcode245
lily/stencil-integral.cc:245: for (vsize i = 0; i < 8; i += 2)
On 2020/04/27 19:56:59, hahnjo wrote:
> hm, can we avoid hardcoding the size twice - or even get rid of it
completely? I
> think you can write
> Offset points[] = { ... };
> and
> i < sizeof(points) / sizeof(points[0])
Done.
https://codereview.appspot.com/581960043/diff/561710044/lily/stencil-integral.cc#newcode253
lily/stencil-integral.cc:253: vector<Offset> points;
On 2020/04/27 20:31:55, dak wrote:
> Seems unused.
Done.
https://codereview.appspot.com/581960043/diff/561710044/lily/stencil-integral.cc#newcode275
lily/stencil-integral.cc:275: if (i)
On 2020/04/27 19:56:59, hahnjo wrote:
> i > 0
Done.
https://codereview.appspot.com/581960043/diff/561710044/lily/stencil-integral.cc#newcode309
lily/stencil-integral.cc:309: Offset ps[4] = {
On 2020/04/27 19:56:59, hahnjo wrote:
> you should be able to remove the explicit 4 here
Done.
https://codereview.appspot.com/581960043/diff/561710044/lily/stencil-integral.cc#newcode323
lily/stencil-integral.cc:323: for (int i = 0; i < 4; i++)
On 2020/04/27 19:56:59, hahnjo wrote:
> (not here though because the parameter is a pointer now - sigh,
sometimes C and
> C++ are very frustrating...)
>
> Instead how about passing a Bezier directly to this function?
honestly, I don't understand the concern. It's not like we're ever going
to change how many control points a Bezier curve has.
https://codereview.appspot.com/581960043/