lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Skylines: reject steep-sloped buildings; issue 3383 (issue 9890045)


From: dak
Subject: Re: Skylines: reject steep-sloped buildings; issue 3383 (issue 9890045)
Date: Mon, 03 Jun 2013 19:03:26 +0000


https://codereview.appspot.com/9890045/diff/1/lily/skyline.cc
File lily/skyline.cc (right):

https://codereview.appspot.com/9890045/diff/1/lily/skyline.cc#newcode117
lily/skyline.cc:117: slope_ = 0.0; /* if they were both infinite, we
would get nan, not 0, from the next line */
That comment is a piece of crock since it is completely unclear what
"both" is supposed to mean and "next line" is a comparison and can't
yield nan.

https://codereview.appspot.com/9890045/diff/1/lily/skyline.cc#newcode128
lily/skyline.cc:128: else if (fabs(slope_) > 1e6)
That assumes that end - start is close to 0.  I see nothing that
precludes end - start to actually be 0, and then the above assert will
fail anyway.

https://codereview.appspot.com/9890045/diff/1/lily/skyline.cc#newcode141
lily/skyline.cc:141: return isinf (x) ? y_intercept_ : slope_ * x +
y_intercept_;
Basically, the whole representation is a bad idea numerically.  If I
understand correctly, this is not going to extrapolate, anyway.  So the
result should always be an interpolation between the left and right y
value, and instead of an "y intercept", a left border y coordinate would
make more sense.  Which is available without further ado anyway.

https://codereview.appspot.com/9890045/diff/1/lily/skyline.cc#newcode527
lily/skyline.cc:527: if (x1 < x2)
That very much looks like silently dropping zero-width buildings from
consideration again.

https://codereview.appspot.com/9890045/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]