Irev-Dev / Round-Anything

A set of OpenSCAD utilities for adding radii and fillets, that embodies a robust approach to developing OpenSCAD parts.
https://kurthutten.com/blog/round-anything-a-pragmatic-approach-to-openscad-design
MIT License
504 stars 47 forks source link

Invert flipped faces for polyRoundExtrude #10

Closed nickcoutsos closed 4 years ago

nickcoutsos commented 4 years ago

I've recently started playing around with this library and noticed an issue with a difference operation after switching one of my designs from polygin(polyRound(...)) to polyRoundExtrude.

Here's the sample script to illustrate the problem:

use <Round-Anything/polyround.scad>

difference() {
  translate([0, 0, -2.5]) polyRoundExtrude([
    [0, 10, 5],
    [10, 0, 5],
    [0, -10, 5],
    [-10, 0, 5]
  ]);

  sphere(d=10);
}

In OpenSCAD I see the following

Screen Shot 2020-10-03 at 10 45 11 PM Screen Shot 2020-10-03 at 10 25 33 PM

The left is Preview and the right is Thrown Together. If I attempt to render the result I get the following in the console:

Parsing design (AST generation)... Compiling design (CSG Tree generation)... Rendering Polygon Mesh using CGAL... ERROR: CGAL error in CGAL_Nef_polyhedron3(): CGAL ERROR: assertion violation! Expr: e->incident_sface() != SFace_const_handle() File: /Users/kintel/code/OpenSCAD/libraries/install/include/CGAL/Nef_S2/SM_const_decorator.h Line: 330 Geometries in cache: 1229 Geometry cache size in bytes: 29986496 CGAL Polyhedrons in cache: 109 CGAL cache size in bytes: 102138824 Total rendering time: 0 hours, 0 minutes, 0 seconds Rendering finished.

From what I understand about extrudePolygonWithRadius, it uses the same operation for the top and bottom geometry, but mirrors the coordinates in the bottom geometry along the Z axis. This works for preview, but results in inverted faces because the coordinates are no longer in the correct order.

This PR introduces a couple of helper functions to reverse the order of vertices in a list of faces and applies it to the side faces, bottom faces, and bottom cap. I notice offsetPolygonPoints gives some consideration to clockwise vs counter-clockwise but I'm not clear on how that gets used or if it would affect the extrude functionality.

Hope this helps! I came across your library a little while back and was really impressed, thanks for all the hard work on it :)

Irev-Dev commented 4 years ago

Thanks @nickcoutsos I'll have a look at this soon.

nickcoutsos commented 4 years ago

Ah, okay, that complicates things. Do you feel it would be enough to stipulate that the points are given in clockwise order? I suppose the order could be conditionally reversed internally with CWorCCW but concave polygons might require testing the entire thing (and checking the convexity param? I might be getting in a little over my head)

nickcoutsos commented 4 years ago

Looks like CWorCCW takes care of more than I thought and just needed to be called from polyRoundExtrude to see if the list of radiiPoints needs to be reversed. I'm reusing the reverse function there and renamed it for clarity.

Irev-Dev commented 4 years ago

I would be happy to make you a collaborator for this repo if you're interested @nickcoutsos. Let me know.

nickcoutsos commented 4 years ago

Sure, that sounds cool! Do you have any code style preferences? There doesn't seem to be any guidelines for OpenSCAD and I tend towards JavaScript Standard Style.

Irev-Dev commented 4 years ago

Sure, we can go with that standard as much as it's applicable. I agree with having a standard, but I'd much prefer if we were able to lint with a commit hook. Hmm I did find this, wonder if it could be hooked up to a commit hook https://github.com/Maxattax97/openscad-format. Probably not worth the effort at this point.