Skip to content

Commit

Permalink
bug fix: don't explicitly draw final segment of a loop if it is straight
Browse files Browse the repository at this point in the history
This was causing strange miter joins in some situations.  Fixes #38.
  • Loading branch information
Brent Yorgey committed Jan 17, 2014
1 parent 3089fb6 commit 0f5e399
Showing 1 changed file with 15 additions and 7 deletions.
22 changes: 15 additions & 7 deletions src/Diagrams/Backend/Cairo/Internal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -295,15 +295,23 @@ instance Renderable (Segment Closed R2) Cairo where
= C . liftC $ C.relCurveTo x1 y1 x2 y2 x3 y3

instance Renderable (Trail R2) Cairo where
render _ t = flip withLine t $ renderT . lineSegments
render _ = withTrail renderLine renderLoop
where
renderT segs =
C $ do
mapM_ renderC segs
liftC $ when (isLoop t) C.closePath
renderLine ln = C $ do
mapM_ renderC (lineSegments ln)

when (isLine t) (ignoreFill .= True)
-- remember that we saw a Line, so we will ignore fill attribute
-- remember that we saw a Line, so we will ignore fill attribute
ignoreFill .= True

renderLoop lp = C $ do
case loopSegments lp of
-- let closePath handle the last segment if it is linear
(segs, Linear _) -> mapM_ renderC segs

-- otherwise we have to draw it explicitly
_ -> mapM_ renderC (lineSegments . cutLoop $ lp)

This comment has been minimized.

Copy link
@fryguybob

fryguybob Jan 17, 2014

Member

I think in this case we should still be throwing away the end point that cutLoop gives and use the start point instead. The same issue can occur with cubic segments so we should solve it there too.

This comment has been minimized.

Copy link
@byorgey

byorgey Jan 17, 2014

Member

Ah, good point. Can we come up with a contrived example that causes the bad behavior? (I think we should still fix it even if we can't, but it would make it a lot easier to tell when we've properly fixed it...)

This comment has been minimized.

Copy link
@byorgey

byorgey Jan 17, 2014

Member

And incidentally, I think what we should really do is modify cutLoop directly, so that it guarantees to always give back exactly equal start and end points.

This comment has been minimized.

Copy link
@fryguybob

fryguybob Jan 17, 2014

Member

An easy example that should give the same result is just replacing the last segment in the triangle with cubic that has the handles colinear with the original line. A glance at cutLoop didn't seem to me like the general solution was possible as the end need to be specified in terms of an offset. But I would love it if is.

This comment has been minimized.

Copy link
@byorgey

byorgey Jan 17, 2014

Member

Ah, no, you're right, I was confused for a minute about what cutLoop is doing. I tried making an example that exhibits the problem with a bezier but haven't been able to yet.


liftC C.closePath

instance Renderable (Path R2) Cairo where
render _ p = C $ do
Expand Down

0 comments on commit 0f5e399

Please sign in to comment.