diagrams / diagrams-contrib

User-contributed extensions to diagrams
BSD 3-Clause "New" or "Revised" License
26 stars 31 forks source link

union does not produce expected result when used with difference #78

Open bacchanalia opened 5 years ago

bacchanalia commented 5 years ago

In the following code ex3 should be the union of the the star with the ring, but instead union just returns the equivilent of ex2.

{-# LANGUAGE TypeFamilies, FlexibleContexts #-}
import Diagrams.Prelude
import Diagrams.Backend.Cairo.CmdLine
import Diagrams.TwoD.Polygons
import qualified Diagrams.TwoD.Path.Boolean as B

rose r1 r2 = polygon $ with
  & polyType   .~ PolyPolar (replicate 7 $ 1/8 @@ turn) (cycle [r1, r2])
  & polyOrient .~ NoOrient

ex1, ex2, ex3 :: Diagram Cairo
ex1 = stroke $ B.union Winding $ rose 1 3 <> circle 2                                     -- works
ex2 = stroke $ rose 1 3 <> B.difference Winding (circle 2) (circle 1.9)                   -- works
ex3 = stroke $ B.union Winding $ rose 1 3 <> B.difference Winding (circle 2) (circle 1.9) -- broken

main = mainWith (ex1 ||| ex2 ||| ex3)
fryguybob commented 5 years ago

I think something must be a little off in the direction of the paths that come from B.difference. If I reverse the result of the B.difference it works:

ex3 = stroke $ B.union Winding $ rose 1 3 <> (reversePath $ B.difference Winding (circle 2) (circle 1.9))
byorgey commented 5 years ago

Ah, right @fryguybob , that jogged my memory. There was this similar bug that is now fixed: https://github.com/diagrams/diagrams-contrib/issues/67 but in general it seems this is a known issue with cubicbezier: https://github.com/kuribas/cubicbezier/issues/6 .

bacchanalia commented 5 years ago

Since there hasn't been any movement on this on the cubicbezier side, I'd like to fix this on the diagrams side, but I'd like some feedback on what the desired behaviour is. I can think of two reasonable interpretations:

  1. (simpler) Force all exterior loops to be counterclockwise, reversing interior paths as needed.
  2. (more expensive) For each exterior loop, if any point on it corresponds to a point on a counterclockwise input loop, force it to be counterclockwise, otherwise force it to be clockwise, reversing interior paths as needed.
bacchanalia commented 5 years ago

I think option (2) actually requires the improvements to segmentSegment discussed in diagrams-lib issue #323 in order to be able to map sections of output looks back to input loops.