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

union of 5 simple objects crashes browser #598

Closed
SebiTimeWaster opened this issue Jun 7, 2020 · 11 comments
Closed

union of 5 simple objects crashes browser #598

SebiTimeWaster opened this issue Jun 7, 2020 · 11 comments
Labels

Comments

@SebiTimeWaster
Copy link

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;
};
@z3dev
Copy link
Member

z3dev commented Jun 9, 2020

@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.

@SebiTimeWaster
Copy link
Author

SebiTimeWaster commented Jun 9, 2020

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.

@z3dev
Copy link
Member

z3dev commented Sep 25, 2020

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 }))
    );

@SebiTimeWaster
Copy link
Author

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.

@hrgdavor
Copy link
Contributor

hrgdavor commented Sep 4, 2021

There is now an ongoing attempt to fix this in #898 ... with original code from this bug report converted to V2.

@platypii
Copy link
Contributor

platypii commented Feb 25, 2022

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 Non-manifold objects generated from union #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

@hrgdavor
Copy link
Contributor

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.

@SebiTimeWaster
Copy link
Author

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

@z3dev
Copy link
Member

z3dev commented Aug 13, 2022

@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...?

@SebiTimeWaster
Copy link
Author

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.

@hrgdavor
Copy link
Contributor

hrgdavor commented Sep 2, 2022

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

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

No branches or pull requests

4 participants