[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Use Interval for representing skyline X coordinates (issue 571980043
From: |
hanwenn |
Subject: |
Re: Use Interval for representing skyline X coordinates (issue 571980043 by address@hidden) |
Date: |
Fri, 17 Apr 2020 04:23:38 -0700 |
On 2020/04/16 21:44:00, Dan Eble wrote:
> LGTM
>
>
https://codereview.appspot.com/571980043/diff/581920043/lily/skyline.cc
> File lily/skyline.cc (right):
>
>
https://codereview.appspot.com/571980043/diff/581920043/lily/skyline.cc#newcode164
> lily/skyline.cc:164: if (ret >= x_[LEFT] && ret <= x_[RIGHT] &&
!std::isinf
> (ret))
> Is this x_.contains (ret) && ...?
>
>
https://codereview.appspot.com/571980043/diff/581920043/lily/skyline.cc#newcode223
> lily/skyline.cc:223: if (b.x_[RIGHT] < c.x_[RIGHT]) /* finish with b
*/
> Hmm... I wonder if there are Interval operations that would made this
section
> more readable. I'm not asking you to do anything, just wondering.
>
>
https://codereview.appspot.com/571980043/diff/581920043/lily/skyline.cc#newcode292
> lily/skyline.cc:292: assert (b.x_[RIGHT] >= b.x_[LEFT]);
> Is this !b.x_.is_empty ()?
>
>
https://codereview.appspot.com/571980043/diff/581920043/lily/skyline.cc#newcode565
> lily/skyline.cc:565: i->x_[LEFT] += s;
> Is this i->x_ += s?
you're right about many of those, but I'll leave it for a follow-up
change. There might be subtleties in handling >= vs > . I also want to
take a closer look at the skyline code; I think it may be allocating too
much data
https://codereview.appspot.com/571980043/
- Use Interval for representing skyline X coordinates (issue 571980043 by address@hidden), jonas . hahnfeld, 2020/04/16
- Re: Use Interval for representing skyline X coordinates (issue 571980043 by address@hidden), dak, 2020/04/16
- Re: Use Interval for representing skyline X coordinates (issue 571980043 by address@hidden), hanwenn, 2020/04/16
- Re: Use Interval for representing skyline X coordinates (issue 571980043 by address@hidden), nine . fierce . ballads, 2020/04/16
- Re: Use Interval for representing skyline X coordinates (issue 571980043 by address@hidden), hanwenn, 2020/04/16
- Re: Use Interval for representing skyline X coordinates (issue 571980043 by address@hidden), nine . fierce . ballads, 2020/04/16
- Re: Use Interval for representing skyline X coordinates (issue 571980043 by address@hidden),
hanwenn <=