[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Gets vertical skylines from grob stencils (issue 5626052)
From: |
mtsolo |
Subject: |
Re: Gets vertical skylines from grob stencils (issue 5626052) |
Date: |
Sat, 14 Jul 2012 18:33:10 +0000 |
http://codereview.appspot.com/5626052/diff/85004/lily/skyline.cc
File lily/skyline.cc (right):
http://codereview.appspot.com/5626052/diff/85004/lily/skyline.cc#newcode292
lily/skyline.cc:292: Real p1 = left->end_ * left->slope_ +
left->y_intercept_;
On 2012/07/14 14:01:08, joeneeman wrote:
use left->height (left_end_) instead of calculating it by hand
Done.
http://codereview.appspot.com/5626052/diff/85004/lily/skyline.cc#newcode293
lily/skyline.cc:293: Real p2 = i->start_ * i->slope_ + i->y_intercept_;
On 2012/07/14 14:01:08, joeneeman wrote:
i->height (i->start_)
Done.
http://codereview.appspot.com/5626052/diff/85004/lily/skyline.cc#newcode302
lily/skyline.cc:302: while (dirty);
On 2012/07/14 14:01:08, joeneeman wrote:
I don't understand this do while (dirty) part. It seems to me that
unless you
have two empty segments in a row, you only need one pass. On the other
hand, if
you have two empty segments in a row then this function will fail
anyway because
you will end up setting center->slope_ and center->y_intercept_ both
to
-infinity, which is bound to start creating NaN sooner or later.
I'll rewrite this so that it combines all interior skylines with an
infinite y intercept and then only does one pass.
http://codereview.appspot.com/5626052/diff/90001/lily/include/skyline.hh
File lily/include/skyline.hh (right):
http://codereview.appspot.com/5626052/diff/90001/lily/include/skyline.hh#newcode58
lily/include/skyline.hh:58: NOT_ENOUGH_INFO
On 2012/07/14 14:01:08, joeneeman wrote:
I wish I could convince you to think of a skyline as a region instead
of just
the boundary of that region. Once you think of it that way, it becomes
clear
that this information can be easily obtained from the Skyline_pair,
using the
existing distance function.
Suppose s and t are Skyline_pairs.
max(s[UP].distance(t[DOWN]), s[DOWN].distance(t[UP])) = -infinity
means the objects don't overlap horizontally at all, so it's
meaningless to talk
about which one is higher (I think this is what you're calling
NOT_ENOUGH_INFO).
min(s[UP].distance(t[DOWN]), s[DOWN].distance(t[UP])) > 0
means that the objects intersect.
s[UP].distance(t[DOWN]) <= 0 and s[DOWN].distance(t[UP]) > 0
means that s is below t
t[UP].distance(s[DOWN]) <= 0 and t[DOWN].distance(s[UP]) > 0
means that t is below s
This logic uses two calls to distance, which is expensive, whereas using
the e-num gets this info from one call to distance. I can use the
method you propose, but don't you think that'd cause a performance hit?
http://codereview.appspot.com/5626052/diff/90001/lily/skyline.cc
File lily/skyline.cc (right):
http://codereview.appspot.com/5626052/diff/90001/lily/skyline.cc#newcode796
lily/skyline.cc:796: */
On 2012/07/14 14:01:08, joeneeman wrote:
Since padding doesn't seem to be involved in this function, you should
delete
this comment.
Done.
http://codereview.appspot.com/5626052/
- Re: Gets vertical skylines from grob stencils (issue 5626052), (continued)
- Re: Gets vertical skylines from grob stencils (issue 5626052), Janek Warchoł, 2012/07/03
- Re: Gets vertical skylines from grob stencils (issue 5626052), address@hidden, 2012/07/04
- Re: Gets vertical skylines from grob stencils (issue 5626052), Janek Warchoł, 2012/07/04
- Re: Gets vertical skylines from grob stencils (issue 5626052), address@hidden, 2012/07/04
- Re: Gets vertical skylines from grob stencils (issue 5626052), Janek Warchoł, 2012/07/04
- Re: Gets vertical skylines from grob stencils (issue 5626052), address@hidden, 2012/07/05
- Re: Gets vertical skylines from grob stencils (issue 5626052), Janek Warchoł, 2012/07/05
- Re: Gets vertical skylines from grob stencils (issue 5626052), Phil Holmes, 2012/07/04
Re: Gets vertical skylines from grob stencils (issue 5626052), dak, 2012/07/09
Re: Gets vertical skylines from grob stencils (issue 5626052), joeneeman, 2012/07/14
Re: Gets vertical skylines from grob stencils (issue 5626052),
mtsolo <=