Haskell-Things / ImplicitCAD

A math-inspired CAD program in haskell. CSG, bevels, and shells; 2D & 3D geometry; 2D gcode generation...
https://implicitcad.org/
GNU Affero General Public License v3.0
1.39k stars 142 forks source link

A bug in RefineSegs.hs #234

Open p4l1ly opened 4 years ago

p4l1ly commented 4 years ago

Hi devolopers and maintainers of this great project,

I have come across a bug in the refine function, which generates broken faces in some models. For now, I don't understand the full idea of the function so I don't come with a fix proposal, so I provide the reproduction code (which creates an internal helic involute gear):

module Main where

import Prelude
import Graphics.Implicit
import Graphics.Implicit.Definitions

involute x y = - (x * cos angle + y * sin angle - 1)
  where angle = sqrt$ abs$ x**2 + y**2 - 1

circ x y = x**2 + y**2 - 1

invcirc :: SymbolicObj2
invcirc = flip implicit ((-5, -5), (5, 5)) $ \(x, y) -> maximum
  [ min (involute x y) (maximum [x - 1, y - 1.5, -x - 3])
  , -y
  , circ (x/4.9) (y/4.9)
  ]

double :: a -> (a, a)
double x = (x, x)

involuteGear :: ℝ -> ℕ -> ℝ -> ℝ-> SymbolicObj2
involuteGear pressureAngle toothCount modulus gapExpansion = scale scale_ $
  intersect
    [ circle (1 + 2/toothCountℝ)
    , union$ [circle (1 - 2*5/4/toothCountℝ)] ++
        [ rotate ((fromℕtoℝ i)*toothAngle) $
            intersect [toothHalf, rotate toothAngle$ scale (1, -1) toothHalf]
        | i <- [0..toothCount - 1]
        ]
    ]
  where
    toothCountℝ = fromℕtoℝ toothCount
    scale_ = double$ modulus * toothCountℝ / 2

    toothHalf = rotate (halfGapAngle)$ scale (double involuteScale) invcirc
    involuteScale = cos pressureAngle

    toothAngle = 2 * pi / toothCountℝ
    halfGapAngle = (toothAngle / 2 - 2 * (tan pressureAngle - pressureAngle)) / 2
      + gapExpansion

main = writeSTL 1 "test.stl" obj
  where
    pressureAngle = 30*pi/180
    modulus = 2
    gearProfile toothCount gapExpansion =
      involuteGear pressureAngle toothCount modulus gapExpansion

    gearLike toothCount profile = extrudeRM
      0
      (Right$ \z -> abs$ 360/toothCount' - 360/toothCount'/5*2*z)
      (Left 1)
      (Left (0, 0))
      profile
      (Left 5)
      where toothCount' = fromℕtoℝ toothCount

    gear toothCount gapExpansion = gearLike toothCount$
      gearProfile toothCount gapExpansion

    obj = gearLike 54$ difference [circle$ 54*modulus/2 + 10, gearProfile 54 (-0.003)]

The broken faces were no more there when I removed the refine function application here, which can be used as a simple workaround to this bug:

-    in map (refine res obj) . filter notPointLine $ case (x1y2 <= 0, x2y2 <= 0,
-                                                          x1y1 <= 0, x2y1 <= 0) of
+    in filter notPointLine $ case (x1y2 <= 0, x2y2 <= 0, x1y1 <= 0, x2y1 <= 0) of
julialongtin commented 4 years ago

Good catch! do you mind if i add your example code to the examples in implicitcad? It would help me know people depend on that part of the API during release testing.

Thanks again!

p4l1ly commented 4 years ago

Sure you can add it as an example, I will also publish an involute gear library soon :) And... am I using some obsolete part of the API? Is there some more recent approach of modelling? I followed the examples and it looks like I should just use the basic primitives combined with implicit if the primitives are not enough. Then output via writeSTL.

julialongtin commented 4 years ago

So far as i know, you're using the up to date part. I just don't have any examples testing it that thoroughly, hense why i asked to add yours. :)

On Thu, Nov 21, 2019 at 8:55 PM p4l1ly notifications@github.com wrote:

Sure you can add it as an example, I will also publish an involute gear library soon :) And... am I using some obsolete part of the API? Is there some more recent approach of modelling? I followed the examples and it looks like I should just use the basic primitives combined with implicit if the primitives are not enough, then output via writeSTL.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/colah/ImplicitCAD/issues/234?email_source=notifications&email_token=AEAMAAQ62OV677NJGKBIXG3QU3YVZA5CNFSM4JOKYOKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE3TZVQ#issuecomment-557268182, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEAMAAQDYMBNFTPWIGKHTUTQU3YVZANCNFSM4JOKYOKA .

julialongtin commented 4 years ago

After poking at this, the change reduces the polygon count of all objects, making them slightly smaller (read: less accurate). I'm going to have to dig deep into the example you've given me directly, and see if i can find the root cause.

still, thanks. this at least points me in the right direction.

On Thu, Nov 21, 2019 at 8:57 PM Julia Longtin julia.longtin@wire.com wrote:

So far as i know, you're using the up to date part. I just don't have any examples testing it that thoroughly, hense why i asked to add yours. :)

On Thu, Nov 21, 2019 at 8:55 PM p4l1ly notifications@github.com wrote:

Sure you can add it as an example, I will also publish an involute gear library soon :) And... am I using some obsolete part of the API? Is there some more recent approach of modelling? I followed the examples and it looks like I should just use the basic primitives combined with implicit if the primitives are not enough, then output via writeSTL.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/colah/ImplicitCAD/issues/234?email_source=notifications&email_token=AEAMAAQ62OV677NJGKBIXG3QU3YVZA5CNFSM4JOKYOKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE3TZVQ#issuecomment-557268182, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEAMAAQDYMBNFTPWIGKHTUTQU3YVZANCNFSM4JOKYOKA .

p4l1ly commented 4 years ago

It is true, but I think it's not a very big deal because the accuracy can be still controlled by the resolution parameter. If you do e.g. 3D printing, it is OK just to pick a resolution a bit smaller than the resolution of your 3D printer and you don't care about further accuracy.

p4l1ly commented 4 years ago

I can see that the refine function uses some derivations but my function is not differentiable.