Re: Minor cleanups in stencil-integral.cc (issue 579630043 by address@hidden)

[hanwenn]

On 2020/04/24 22:04:52, dak wrote:
> On 2020/04/24 21:33:12, Carl wrote:
> > On 2020/04/24 21:19:57, dak wrote:
> > > On 2020/04/24 21:18:12, dak wrote:
> > > >
> > >
> >
>
https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc
> > > > File lily/stencil-integral.cc (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc#newcode465
> > > > lily/stencil-integral.cc:465: // more convoluted, but it's
fairly hot
> path.
> > > > Sorry for not being clear: the question was not why this change
was
> > effective
> > > in
> > > > saving time, but why it was valid.  When thickness is zero, you
only
> update
> > > the
> > > > upper skyline.  Why would the lower skyline no longer need
updating?
> > > 
> > > Well, other way round, but apart from that the question stands.
> > 
> > When thickness is zero, the upper and lower curves are the same. 
Either one
> > completes the skyline.
> 
> Of course they are the same.  That is not the question.  The question
is why I
> am considering the identical curve for the minimum skyline while I
don't let it
> participate in the maximum skyline any more.
> 
> If everything is of thickness null, the maximum skyline will be
non-existent
> while the minimum skyline is there.  While the skylines should be
identical
> rather than only one of them being there.

the Direction d is up/down, but note that it calculates d *
normal(curve-direction), so you get to two curves that correspond to the
outer and inner curve of a stroked bezier.

The curve has no particular orientation relative to the axes, so
-assuming the previous code was correct- it can't be that the UP points
are for the UP skyline.

You can see this happening here near the bottom of the function: the
points are line segments, and then each line segment is interpreted as a
box. The boxes are then added to the curve.

I think this all can be more succinctly expressed by just calculating
the curve once and fattening the resulting box by .5 thickness on all
sides, but all the same, it would be better to get rid of the boxes
entirely, and work from curves directly to skylines. 

I have plans to do this, but they require overhaul of calling
conventions in the stencil -> {box,building} -> skyline pipeline.

https://codereview.appspot.com/579630043/