OpenJSCAD.org icon indicating copy to clipboard operation
OpenJSCAD.org copied to clipboard

union of 5 simple objects crashes browser

Open SebiTimeWaster opened this issue 4 years ago • 8 comments

Expected Behavior

that it works

Actual Behavior

when i try to union the 5 elements the attached below script generated or click on "generate stl" (i presume it does a union in the background) the browser tab is not responding anymore until it crashes

Steps to Reproduce the Problem

  1. generate 5 objects with attached below script
  2. click "generate stl"
  3. crash

Specifications

  • Version: website
  • Platform: newest ubuntu
  • Environment: newest chromium
function main() {
    const testObj = union(
        translate([5, 5, 5], sphere({ r: 5, fn: 60 })),
        translate([0, 0, 10], cube({ size: 10 })),
        translate([5, 5, 20], cylinder({ r1: 1, r2: 5, h: 10, fn: 60 }))
    );

    return bendObj(testObj, 20, 5);
}

const bendObj = (obj, objRadius, segments = 1) => {
    const slices = [];

    if (segments === 1) {
        slices.push(bend(obj, objRadius));
    } else {
        const objSize = obj.getBounds();
        const objW = Math.abs(objSize[0].x - objSize[1].x);
        const objD = Math.abs(objSize[0].y - objSize[1].y);
        const objH = Math.abs(objSize[0].z - objSize[1].z);
        const objSliceHeight = objH / segments;

        for (let x = 0; x < segments; x++) {
            const bentObj = bend(
                intersection(
                    translate([0, 0, x * -objSliceHeight], obj),
                    translate([-5, -5, 0], cube({ size: [objW + 10, objD + 10, objSliceHeight] }))
                ),
                objRadius
            );
            const angleAlpha = (x * objSliceHeight) / objRadius;

            slices.push(translate([0, -objRadius, 0], rotate([(angleAlpha * 360) / (Math.PI * 2), 0, 0], translate([0, objRadius, 0], bentObj))));
        }
    }

    return slices;
};

const bend = (obj, objRadius) => {
    const csg = new CSG();

    obj.polygons.forEach((polygon) => {
        const vertices = [];

        polygon.vertices.forEach((vertice) => {
            const vertex = vertice.pos;
            const angleAlpha = vertex.z / objRadius;
            const newRadius = objRadius + vertex.y;
            const posY = Math.cos(angleAlpha) * newRadius - objRadius;
            const posZ = Math.sin(angleAlpha) * newRadius;

            vertices.push(new CSG.Vertex(new CSG.Vector3D(vertex.x, posY, posZ)));
        });

        csg.polygons.push(new CSG.Polygon(vertices));
    });

    csg.isCanonicalized = false;
    csg.isRetesselated = false;
    return csg;
};

SebiTimeWaster avatar Jun 07 '20 19:06 SebiTimeWaster

@SebiTimeWaster interesting... I have a few comments.

Math.cos() and Math.sin() expect angles as per radians (0 - 2*PI). And the JSCAD V1 rotate() expects angles as per degrees (0 - 360). So, you need to be careful with those calculations.

The bend() function doesn't look correct. I suggest that you test that function heavy. Incorrect calculations on vertices will result in a solid with gaps and / or incorrect polygons.

z3dev avatar Jun 09 '20 05:06 z3dev

The bend() function doesn't look correct. I suggest that you test that function heavy. Incorrect calculations on vertices will result in a solid with gaps and / or incorrect polygons.

can you elaborate on what does not look correct?

at least optically i cannot see any gaps or wrong rotated faces.

btw, when i do 1, 2, 3, 4 or 6 slices instead of 5, 7, 8, 9 or 10 it works in under 2 seconds, so i presume it is an endless loop / memory leak problem. maybe it happens when a cut directly hits a vertice or something like that.

SebiTimeWaster avatar Jun 09 '20 17:06 SebiTimeWaster

Here's something to ponder.. 👍 Try using this test object, and bend it. The result is totally different.

    const testObj = union(
        translate([50, 50, 50], sphere({ r: 50, fn: 60 })),
        translate([0, 0, 100], cube({ size: 100 })),
        translate([50, 50, 200], cylinder({ r1: 10, r2: 50, h: 100, fn: 60 }))
    );

z3dev avatar Sep 25 '20 06:09 z3dev

yeah, it looks fine and renders in a reasonable time ~10secs, when the parameters are correctly adjusted (segmentation):

image

if you run this with a very low segmentation it produces wrong faces since the curvature of the object is too high for something low poly like a cube. low poly objects is the reason the segments parameter exists.

SebiTimeWaster avatar Jan 27 '21 22:01 SebiTimeWaster

There is now an ongoing attempt to fix this in https://github.com/jscad/OpenJSCAD.org/pull/898 ... with original code from this bug report converted to V2.

hrgdavor avatar Sep 04 '21 10:09 hrgdavor

Did some investigation here. The short version is that this bend function creates a geometry with gaps in it. So when you pass a non-solid geometry to a union function, weird things will happen.

To understand what's happening, first I took the script that @hrgdavor made in #898 which was the V2 port of the bug here. It generates a geometry with gaps in it. I simplified the example, replacing the sphere and cylinder with a cuboid. And I reduced the number of slices to 1 so that I could just use the bend function directly:

const jscad = require('@jscad/modeling')
const { cuboid } = jscad.primitives
const { union } = jscad.booleans

function main() {
  let out = union(
    cuboid({ size: [10, 10, 10], center: [0, 0, 10] }),
    cuboid({ size: [1, 1, 10], center: [0, 5, 15] }),
  )
  out = bend(out, 20)
  return out
}

const bend = (obj, objRadius) => { 
  const csg = jscad.geometries.geom3.create()

  obj.polygons.forEach((polygon) => {
    const vertices = []

    polygon.vertices.forEach((vertex) => {
      const angleAlpha = vertex[2] / objRadius
      const newRadius = objRadius + vertex[1]
      const posY = Math.cos(angleAlpha) * newRadius - objRadius
      const posZ = Math.sin(angleAlpha) * newRadius

      vertices.push([vertex[0], posY, posZ])
    })

    csg.polygons.push({vertices})
  })
  return csg
}

module.exports = { main }

This shows the bend function (which is not from jscad) just naively maps vertices to new locations based on a curve. It does not take into account adjacent faces and whether there might be non-manifold vertices which will "split apart" during the transformation.

issue598

There are a couple ways to look at this:

  1. Constructing a non-manifold geometry and performing operations on it is undefined behavior, and can produce unexpected results.
  2. We should detect non-manifold geometry and throw an error. I like this option if we can do it cheaply. But it would almost certainly cause a lot of currently-working models to break.
  3. The actual problem is arguably the first union call. It produces a solid where one face has an edge where the adjacent face has a vertex splitting the same edge. The inputs to union are both valid manifold cuboids. The output of union violates the manifold condition that "every edge is contained in exactly two faces". If the output of the union were manifold, by splitting the adjacent face edge, the geometry would be more "correct" in the sense that it would be more robust to floating point errors, and certain transformations (such as this bend function) would work correctly. See also #807.

In my opinion, the ideal fix would be that the union call (and the BSP trees underlying it), would output a manifold geometry with an additional vertex on the side face if needed:

issue598-fix

platypii avatar Feb 25 '22 22:02 platypii

Personally, I was thinking of doing the bend function without booleans, by creating new mesh with polygons split manually, and then applying the bend without changing their association.

hrgdavor avatar Feb 25 '22 23:02 hrgdavor

it would be nice to have a native bend function in jscad, my code was just a workaround.

SebiTimeWaster avatar Feb 26 '22 21:02 SebiTimeWaster

@SebiTimeWaster not sure what to do now... there's still some hope for a native bend function but that will probably have limitations. How about creating a new issue with some of your thoughts about what the new bend function should be able to do...?

z3dev avatar Aug 13 '22 11:08 z3dev

i dont know how to further proceed here. if im in the mood i might try to fix it (one way could be to cut at all nodes z points, but on complex shapes that would produce incredibly thin slices), but the chances for success are probably rather slim.

SebiTimeWaster avatar Aug 25 '22 15:08 SebiTimeWaster

Way to do it would be by manually "slicing" the mesh and producing new mesh without using jscad booleans.

  • take a shape
  • increase poly count by "splitting" the polygons on z-axis

image

split should just increase number of points in polygons to have more detailed z axis image

then just move points by rotating the space (here is my lame attempt of showing it in photoshop) image

this type of movement of points will not mess with the shape solidity or cause things to overlap

hrgdavor avatar Sep 02 '22 11:09 hrgdavor