Skip to content
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

[manifold] Difference of Cube and Complex SVG Results in Incomplete Render #4679

Closed
degroof opened this issue Jun 25, 2023 · 39 comments
Closed

Comments

@degroof
Copy link

degroof commented Jun 25, 2023

Using Manifold, a difference() between a cube and a linear_extrude()-ed complex SVG results in an incomplete render. Preview is correct but render results in incomplete model.

To Reproduce

  1. Enable Manifold
  2. Load and render the attached wodgrain.scad code

Expected behavior
Render should look like preview

Code reproducing the issue
SCAD code and SVG file included
manifold-issue.zip

Screenshots
If applicable, add screenshots to help explain your problem.
image

Environment and Version info:

  • OS: Windows 10
  • System: Intel PC 64-bit
  • OpenSCAD Version: 2023.06.24 (git 92eec2d)

Additional context
I realize this is a bit of a torture test for the rendering engine. Submitting this mostly in the event that it reveals an issue with the Manifold library.

@kintel
Copy link
Member

kintel commented Jun 28, 2023

It would be awesome to have an "scad-to-manifold" converter which spits out a pure manifold test case, to make it easier to report such issues upstream.

@donovansmith
Copy link

I am seeing the same behavior when differencing using the same specs/version and manifold.

It also occurs with DXFs not just SVGs.

Environment and Version info:
OS: Windows 10
System: Intel PC 64-bit
OpenSCAD Version: 2023.06.24 (git 92eec2d)

@pca006132
Copy link
Member

There are several bug fixes for manifold, wondering if this issue will still occur with the latest master (manifold).

It is also possible that the issue is caused by the mesh extruded from svg, not sure about that. For submitting an issue to upstream, you can just export the model for the svg extruded part (in gltf for example, as it preserves manifoldness), it should be enough for us to reproduce it.

@degroof
Copy link
Author

degroof commented Jul 20, 2023

Just tried it with OpenSCAD version 2023.07.09 (git 44fdb2c), and am getting similar results. Not sure how to export to gltf, but here's an STL file of the extruded SVG, which gives basically the same results when subtracted from a 145x100x1 cube.
woodgrain.zip

@pca006132
Copy link
Member

Reproduced now, thanks!

@alidaf
Copy link

alidaf commented Jul 21, 2023

I've had the same behaviour since manifold was introduced but note that it has also happened with text, albeit a lot less frequently. It affects complicated svg images the most, i.e. lots of small detail or islands (disconnected regions) and the un-rendered region is typically a large geometrically uniform area. It still rocks though. I can't complain with a render time in milliseconds compared to 20+ minutes, when it works!

@pca006132
Copy link
Member

pca006132 commented Jul 21, 2023

I think we might be able to get around this issue (before upstream has a fix) if we do Delaunay refinement to make the svg extruded mesh better. I think the sliver triangles are making the triangulator mad.

Alternatively, you can perform simplify to simplify the 2d polygon.

@alidaf
Copy link

alidaf commented Jul 21, 2023

I use a small offset() to mitigate the creation of open meshes with small features when using the standard solver already. It has limited effect with Manifold but then the radius (typically 0.1) needs to be quite small to maintain as much definition as possible. I'm dealing with logos and text that need the detail to be preserved.

@pca006132
Copy link
Member

There is a simplifyPath function in clipper2 (the 2d library openscad is using) that can simplify the 2d contour.

This function removes vertices that are less than the specified epsilon distance from an imaginary line that passes through its 2 adjacent vertices. Logically, smaller epsilon values will be less aggressive in removing vertices than larger epsilon values.

I checked the woodgrain mesh, and indeed there are triangles where the distance between a point and the edge is very small: min side lenth 0.002837, min height: 0.000322, precision: 0.000700, where the 0.0007 is the value of epsilon manifold is working with. Those triangles do not contribute to the details of the mesh as their areas are so small they are virtually just a line. I am not sure if openscad can do the simplification here, I think I will add this to the upstream later for better robustness and performance.

@pca006132
Copy link
Member

Btw, offset is orthogonal to simplifyPath, I think it should not remove the number of edges in general.

@alidaf
Copy link

alidaf commented Jul 21, 2023

Ah, ok. I'm not using the woodgrain mesh in any case. My images are far less detailed. OP should have precedence on this but I have attached an stl of one of my example images extruded if it helps.
manifold.stl.zip

@degroof
Copy link
Author

degroof commented Jul 21, 2023

OK. I think I've got a simpler example that might help narrow down the issue. I created a model with 2 objects separated by a very narrow curved gap. When this is differenced using Manifold, I get a similar render issue.

Note: It doesn't matter if the "thin gap" object is created with or without manifold, or whether it's created in code or imported from STL.

The non-manifold render is on the left; manifold on the right.
image

I've attached the "thin gap" STL model for use within manifold.

I've also included the SCAD source here. And, yes, the values for "t" and "$fn" are ridiculous. Increasing "t" or decreasing "$fn" slightly will result in a correctly-rendered model, so this code is just over the line where things go wrong.

The code serves no practical purpose other than possibly a torture test for OpenSCAD. 😆

w=4; //width of square and cylinder
t=.00002; //gap between objects
$fn=500;


//cube with a quarter circle knocked out
module oc()
{
  difference()
  {
    translate([w/2,w/2,0])
    cube([w,w,1.1],center=true);
    cylinder(d=w,h=1.2,center=true);
  }
}

//partial cube and cylinder separated by a thin gap
module thinGap()
{
  union()
  {
    oc();
    cylinder(d=w-t,h=1.1,center=true);
  }
}

//difference between a cube and thinGap
module diff()
{
  difference()
  {
    cube([10,10,1],center=true);
    thinGap();
  }
}

//oc();
//thinGap();
diff();

thinGap.zip

@astrolemonade
Copy link

astrolemonade commented Aug 12, 2023

I experience the same issue on nightly(build from source commit: 2835e35 latest as of 12.08.2023)

Without manifold:
image

Time: ~12 minutes

With manifold:
image

Time: ~5 seconds

Script:

$fn=350; // ignore this since it is just for testing purposes
module hole_plate(size, hole_dm, hole_margin, hole_count = [ 2, 2 ])
{
    difference()
    {
        cube(size);
        abs_margin = hole_margin + hole_dm / 2;
        x_hole_dist = (size.x - 2 * abs_margin) / (hole_count.x - 1);
        y_hole_dist = (size.y - 2 * abs_margin) / (hole_count.y - 1);
        x_values = [abs_margin:x_hole_dist:size.x - abs_margin + 0.1];
        y_values = [abs_margin:y_hole_dist:size.y - abs_margin + 0.1];
        // holes
        for (x = x_values, y = y_values)
            translate([ x, y, -1 ]) cylinder(d = hole_dm, h = size.z + 2);
    }
}
hole_plate([ 100, 50, 5 ], 6, 4);
translate([ 0, 60, 0 ]) hole_plate(size = [ 50, 50, 5 ], hole_dm = 3, hole_margin = 2);
translate([ 60, 60, 0 ]) hole_plate([ 50, 50, 5 ], hole_dm = 5, hole_margin = 5);
translate([ 110, 0, 0 ]) hole_plate([ 100, 50, 5 ], 6, 4);
translate([ 0, -60, 0 ]) hole_plate(size = [ 50, 50, 5 ], hole_dm = 3, hole_margin = 2, hole_count = [ 10, 10 ]);

Files:
hole.zip

58M  hole_plate-cgal.stl
9.4M hole_plate-manifold.stl

Reducing $fn to 300 solves the rendering issue. I wonder what happens.

@pca006132
Copy link
Member

@astrolemonade It is a problem with manifold's triangulator, more specifically about how we handle small line segments. We are now tracking this issue in elalish/manifold#502. Btw, thanks for the simple repro!

@pca006132
Copy link
Member

pca006132 commented Aug 14, 2023

should be fixed in the upstream

note that we may need to wait a bit to update the submodule, see elalish/manifold#529

@t-paul
Copy link
Member

t-paul commented Aug 26, 2023

Updating turns out to be a bit difficult. Looks like none of the build environments have a TBB new enough to compile latest Manifold code.

@pca006132
Copy link
Member

Updating turns out to be a bit difficult. Looks like none of the build environments have a TBB new enough to compile latest Manifold code.

What is the compilation error exactly? Is it missing the concurrent containers?
Also, maybe you can just uninstall tbb for those CI environments, we will now check for tbb and compile from source if there is no tbb installed (the compilation is pretty fast)

@t-paul
Copy link
Member

t-paul commented Aug 26, 2023

Link to build logs: https://github.com/openscad/openscad/runs/16230648150

Looks like it's not always the same error as I assumed first. MXE (windows builds on Linux) does have an old TBB version:

/mxe/usr/lib/gcc/x86_64-w64-mingw32.static.posix/11.2.0/include/c++/pstl/parallel_backend_tbb.h:29:6: error: #error Intel(R) Threading Building Blocks 2018 is required; older versions are not supported.
   29 | #    error Intel(R) Threading Building Blocks 2018 is required; older versions are not supported.

I can have a look if the compile from source works here. There's already an issue asking for an upstream TBB update too mxe/mxe#2984.

The AppImage build actually fails due to Manifold injecting -Werror into the build process which is not ideal as this tends to break builds in not so common environments. I already had to patch that in some other build as it caused a build fail in code we don't directly control. AppImage is a bit special as this needs to be built on an older system, basically the minimum it's meant to support. That said, at least the actual issue flagged here, I should be able to fix and see if it goes further.

/root/workspace/src/geometry/manifold/manifold-applyops-minkowski.cc:102:37: error: moving a local object in a return statement prevents copy elision [-Werror=pessimizing-move]
  102 |         return std::move(part_points);
...
cc1plus: all warnings being treated as errors

@pca006132
Copy link
Member

  1. We can disable some code for older versions of tbb. But in general I think we should try to use more recent versions of TBB, as it provides some mulithreaded containers that are pretty useful and have some bug fixes.
  2. I will fix the problem with -Werror. This is supposed to be internal to manifold code, and I think I know where the problem comes from.

@t-paul
Copy link
Member

t-paul commented Aug 29, 2023

Thanks @pca006132, compilation is now green on all platforms. We still need to solve an issue with a test case generating STL output in a different order.

See #4728.

@t-paul
Copy link
Member

t-paul commented Nov 7, 2023

Should be fixed via #4728.

@t-paul t-paul closed this as completed Nov 7, 2023
@astrolemonade
Copy link

Hey, I still experience this after the fix: 5f8f6e4 . Can you have another look, please?

I experience the same issue on nightly(build from source commit: 2835e35 latest as of 12.08.2023)

Without manifold: image

Time: ~12 minutes

With manifold: image

Time: ~5 seconds

Script:

$fn=350; // ignore this since it is just for testing purposes
module hole_plate(size, hole_dm, hole_margin, hole_count = [ 2, 2 ])
{
    difference()
    {
        cube(size);
        abs_margin = hole_margin + hole_dm / 2;
        x_hole_dist = (size.x - 2 * abs_margin) / (hole_count.x - 1);
        y_hole_dist = (size.y - 2 * abs_margin) / (hole_count.y - 1);
        x_values = [abs_margin:x_hole_dist:size.x - abs_margin + 0.1];
        y_values = [abs_margin:y_hole_dist:size.y - abs_margin + 0.1];
        // holes
        for (x = x_values, y = y_values)
            translate([ x, y, -1 ]) cylinder(d = hole_dm, h = size.z + 2);
    }
}
hole_plate([ 100, 50, 5 ], 6, 4);
translate([ 0, 60, 0 ]) hole_plate(size = [ 50, 50, 5 ], hole_dm = 3, hole_margin = 2);
translate([ 60, 60, 0 ]) hole_plate([ 50, 50, 5 ], hole_dm = 5, hole_margin = 5);
translate([ 110, 0, 0 ]) hole_plate([ 100, 50, 5 ], 6, 4);
translate([ 0, -60, 0 ]) hole_plate(size = [ 50, 50, 5 ], hole_dm = 3, hole_margin = 2, hole_count = [ 10, 10 ]);

Files: hole.zip

58M  hole_plate-cgal.stl
9.4M hole_plate-manifold.stl

Reducing $fn to 300 solves the rendering issue. I wonder what happens.

@t-paul
Copy link
Member

t-paul commented Nov 7, 2023

Thanks for testing, I'll reopen for investigation...

@t-paul t-paul reopened this Nov 7, 2023
@kintel
Copy link
Member

kintel commented Nov 8, 2023

Fwiw, it works for me:

Screenshot 2023-11-07 at 19 32 02

@pca006132
Copy link
Member

@astrolemonade are you sure you updated the submodule after pulling the changes?

@astrolemonade
Copy link

astrolemonade commented Nov 8, 2023

It turns out I did the git submodule update --init --recursive but forgot about the git pull step. After doing that and rebuilding OpenSCAD I confirm that the issue is no longer happening. One thing I discovered after the update is that the rendering takes more time than before+the rendering blocks the interface randomly(you can't interact with UI elements or rotate the viewport).

The viewport became terrible slow.

output.mp4

@t-paul
Copy link
Member

t-paul commented Nov 8, 2023

Closing as fix is confirmed. The preview is unrelated to ManifoldT here were some changes in that area, please check Preferences -> Features and maybe enable the changed display options there.

@t-paul t-paul closed this as completed Nov 8, 2023
@astrolemonade
Copy link

Closing as fix is confirmed. The preview is unrelated to ManifoldT here were some changes in that area, please check Preferences -> Features and maybe enable the changed display options there.

I don't have any display options changed in the Preferences -> Features, I don't even see a display settings there.

@pca006132
Copy link
Member

@astrolemonade there were some recent changes in rendering code, maybe you can open another issue for it

@t-paul
Copy link
Member

t-paul commented Nov 8, 2023

Hmm, looks like the change is not merged yet. So let me rephrase: Please check that all of the vertex-object-* options are enabled.

@kintel
Copy link
Member

kintel commented Nov 8, 2023

@astrolemonade How much time does rendering take now?

@pca006132
Copy link
Member

I got 8.5s for this particular example. It is slow because the triangulator is now using ear clipping and is O(n^2), which can get pretty slow when there are 300k vertices. We can potentially use collider to reduce the complexity, do parallelization, or try to optimize the memory access.

Btw not sure if the rendering being slow means manifold or OpenGL rendering.

@pca006132
Copy link
Member

That said, the updated manifold version should be much faster in general. For example comparing with the older version (in #4533), condensed-matter.scad:fn=100 completed in 14.5s instead of 29s, minbosl.scad:ultrafine completed in 18.9s instead of 27.5s, smoothed-antennas.scad completed in 18.9s instead of 26.3s.

@astrolemonade
Copy link

astrolemonade commented Nov 8, 2023

@astrolemonade How much time does rendering take now?

49 seconds compared to the old 6 seconds before the manifold upgrade.

Hmm, looks like the change is not merged yet. So let me rephrase: Please check that all of the vertex-object-* options are enabled.

I enabled them and the viewport behaves faster again.

I have all the vertex* options enabled, predictable-output, manifold, fast-csg, fast-csg-safer enabled.

@kintel
Copy link
Member

kintel commented Nov 8, 2023

49 seconds feels like you're running a debug build. If you build yourself: cmake -DCMAKE_BUILD_TYPE=Release

@kintel
Copy link
Member

kintel commented Nov 8, 2023

Oh, and you should disable the two fast-csg options I think as they potentially conflict with Manifold

@astrolemonade
Copy link

That was it, it was a debug build and I had those 2 options (fast-csg) enabled. After rebuilding it in release mode and disable those 2 I got 9 seconds which is pretty great.

@donovansmith
Copy link

The Windows 2023.11.08 nightly build appears to crash or hang with manifold enabled on render/preview (dependent on file). 2023.11.05 build does not — so it appears to be related to this push?

The 2023.11.08 Linux AppImage appears to work and fix the original issue.

The exception codes I've noted in Event Viewer "Application Error" and ".NET Runtime" entries have been:

  1. 0xc0000374
  2. 0xc0000005

I've tried:

  1. EXE and Zip versions
  2. Three different machines Win 10 (Phys), Win 10, (Phys) Win 11 (VM) (Two have never had OpenSCAD use)
  3. Wiping HKEY_CURRENT_USER\SOFTWARE\OpenSCAD settings in registry (For machine w/ previous OpenSCAD use)
  4. Enabling/Disabling vertex-* entries
  5. fast-csg options were not checked

You should be able to replicate with even a simple basic model like the included Basic > logo.scad example file.

@kintel
Copy link
Member

kintel commented Nov 8, 2023

@donovansmith This probably deserves a new issue, as the fix in question (#4728) relates to a number of other issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants