|
From: | nine . fierce . ballads |
Subject: | Re: Use Interval for representing skyline X coordinates (issue 571980043 by address@hidden) |
Date: | Thu, 16 Apr 2020 14:43:56 -0700 |
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? https://codereview.appspot.com/571980043/
[Prev in Thread] | Current Thread | [Next in Thread] |