-
Notifications
You must be signed in to change notification settings - Fork 83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Non convex polygons #31
base: master
Are you sure you want to change the base?
Conversation
toPoint (Vertex x) = x | ||
|
||
instance Eq Vertex where | ||
(Vertex (x1,y1)) == (Vertex (x2,y2)) = abs (x1-x2) + abs (y1-y2) < 0.0001 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using an arbitrary constant to determine whether two edges are 'close enough' means that the library will not work at different scales. In the current library there is nothing preventing the user from defining their picture with small valued coordinates, eg (0.00001, 0.00001) then scaling it before rendering it to the screen. With the constant it seems that triangulation of such pictures will not work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there way to reformulate the algorithm so that it does not need this constant?
toPoint (Vertex x) = x | ||
|
||
instance Eq Vertex where | ||
(Vertex (x1,y1)) == (Vertex (x2,y2)) = abs (x1-x2) + abs (y1-y2) < 0.0001 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there way to reformulate the algorithm so that it does not need this constant?
instance Eq Edge where | ||
Edge{start = p0, end = p1} == Edge{start = p2, end = p3} | ||
= p0 == p2 && p1 == p3 | ||
instance Ord Edge where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you describe what the ordering relation on edges is?
Just (_, py) = intersectHelper p0 p1 x | ||
Just (_, qy) = intersectHelper p2 p3 x | ||
in if py == qy then (x1-x)*(y3-py) > (x3-x)*(y1-py) else py < qy | ||
compare x y |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is already in the Haskell Prelude.
@@ -0,0 +1,388 @@ | |||
module Graphics.Gloss.Geometry.Triangulation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment describing where a maintainer could read more about the algorithm implemented in this module. If we find a bug in this code we'll need to read the original paper to understand how the algorithm works.
-- | Turns @Path@ into a graph in witch egdes can intersect. You can think of it as a drawing of a @Path@. | ||
makeInitialVertexSet :: Path -> Vertices | ||
|
||
makeInitialVertexSet [] = undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't have uses of 'undefined' in this code, as if the user ever causes this to be evaluated there will be no description of what went wrong. If it should not be possible to cause this error via the public API then use the error function with a message describing what went wrong in the implementation.
-- | Accumulator function used for events processing. | ||
accumulatorFunction :: (AccumulatorType , Events) -> (Vertex, (Set.Set Edge, Set.Set Edge)) -> (AccumulatorType, Events) | ||
|
||
accumulatorFunction ((edges, vertices), events) (_, (startOf, endOf)) = let deletedEdges = edges `Set.difference` (startOf `Set.union` endOf) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll need to reformat the code so it fits in 100 columns at most. Try to follow the coding style in rest of the Gloss library.
= Vertex Point | ||
deriving (Show) | ||
|
||
-- | Utility function to convert @Vertex@ to @Point@ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to say "Utility function" in these comments, just write "Convert a Vertex to a Point". We can already see that it is a utility function.
breakUpToSimplePolygons path = traversePolygonGraph Nothing [] (\_ _ _ -> True) Set.empty (polygonGraph path) | ||
|
||
-- | Traverse planar graph and calculates simple polygons that bound inner fealds of given graph. | ||
traversePolygonGraph :: Maybe (Vertex, Vertex) -> [Path] -> (Vertices -> Vertex -> Vertex -> Bool) -> Set.Set (Vertex, Vertex) -> Vertices -> [Path] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add haddock comments that describe what each of these parameters do. For example, what does the deciderFn do?
|
||
-- | Makes a planar graph with fealds bouded by x-monotone polygons and a set of inner edges. | ||
makeXMonotoneGraph :: Path -> (Vertices, EdgesWithHelpers, Set.Set (Vertex,Vertex)) | ||
makeXMonotoneGraph points = foldl accFn (makeInitialVertexSet points, Map.empty, Set.empty) (sortBy (\(_,a,_) (_,b,_) -> compare (eventCootdinates a) (eventCootdinates b)) $ zip3Tail $ markVerteces points) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reformat this code so it fits in at most 100 columns. Try to follow the coding style in the existing Gloss library.
We implemented decomposition to simple polygons and triangulation of simple polygons.