zalo / CascadeStudio

A Full Live-Scripted CAD Kernel in the Browser
https://zalo.github.io/CascadeStudio/
MIT License
994 stars 122 forks source link

RuntimeError: function signature mismatch #4

Closed ancorasir closed 3 years ago

ancorasir commented 3 years ago

I use the following code to generate a list of TopoDS_Shape brepList

function ForEachShell(shape, callback) {
  let shell_index = 0;
  let anExplorer = new oc.TopExp_Explorer(shape, oc.TopAbs_SHELL);
  for (anExplorer.Init(shape, oc.TopAbs_SHELL); anExplorer.More(); anExplorer.Next()) {
    callback(shell_index++, anExplorer.Current());
  }
}

Then modify one of the shape in brepList like brepList[0] = Rotate(axis = [0, 1, 0], degrees = 1, brepList[0] ); and the following code to make an compound

let scene = new oc.TopoDS_Compound();
let sceneBuilder = new oc.BRep_Builder();
sceneBuilder.MakeCompound(scene);
brepList.forEach((curShape) => {
                  console.log(curShape)
                  sceneBuilder.Add(scene, curShape);
});
this.updateShape(scene, 0.01 )

I got Uncaught RuntimeError: function signature mismatch at BRep_Builder.Add.BRep_Builder.Add (http://127.0.0.1:5500/node_modules/opencascade.js/dist/opencascade.wasm.js:11:914335)

Any suggestion? Did I misuse the functions? I print the curShape and it looks fine... @zalo

zalo commented 3 years ago

It's a shame that the Web Assembly errors are so obtuse :(

Instead of calling this.updateShape() yourself, consider simply adding the shapes you want to draw to the global sceneShapes array. The backend will create a compound shape with a BRep_Builder using sceneShapes automatically.

So something like: ForEachShell(shape, (index, curShape) => sceneShapes.push(curShape));

Also consider checking whether your curShape.IsNull(). I haven't done any testing with TopoDS_Shells yet, so you're the first to test whether they work!

Here's a slightly complex example demonstrating the use of base OpenCascade functions.

If you comment out pipe.MakeSolid(); it will add a Shell to sceneShapes, and thus to the BRep builder that uses it; so the issue isn't with adding shells... it may be something wrong with what the shell iterator is returning...

ancorasir commented 3 years ago

I tested the following code

let holeRadius = Slider("Radius", 30 , 20 , 40);

let sphere     = Sphere(50);
let cylinderZ  =                     Cylinder(holeRadius, 200, true);
let cylinderY  = Rotate([0,1,0], 90, Cylinder(holeRadius, 200, true));
let cylinderX  = Rotate([1,0,0], 90, Cylinder(holeRadius, 200, true));

Translate([0, 0, 50], Difference(sphere, [cylinderX, cylinderY, cylinderZ]));

// Don't forget to push imported or oc-defined shapes into sceneShapes to add them to the workspace!
console.log("Check brep=====================");
const brepList1 = [];
const brepList2 = [];

brepList1.push(sceneShapes[0]);

let anExplorer = new oc.TopExp_Explorer(sceneShapes[0], oc.TopAbs_SHELL);
for (anExplorer.Init(sceneShapes[0], oc.TopAbs_SHELL); anExplorer.More(); anExplorer.Next()) {
    myShell = oc.TopoDS.prototype.Shell(anExplorer.Current());
    console.log("Shell inside for loop is not ", myShell.IsNull())
    brepList2.push(myShell);
};

console.log("brep1 is not ",brepList1[0].IsNull());
console.log("brep2 is not ",brepList2[0].IsNull());

The outpu is ` Check brep=====================

index.js:251 Shell inside for loop is not false

index.js:251 brep1 is not false

index.js:251 brep2 is not true `

Seems I cannot get a list of shells by pushing the shells inside the for loop. ForEachShell(shape, (index, curShape) => sceneShapes.push(curShape)); lead to same mismatch errors because null is added to the sceneShapes. I guess the problem is with the object return by occ, which seems to be pointers. Somehow they got broken out side the for loop. @zalo

zalo commented 3 years ago

Hrm, I now believe that Solids do not contain Shells, just as Faces do not contain the Wires that could have gone into making them. The two generally iterable types appear to just be Faces and Edges.

Therefore, I believe the inability to iterate across Shells in the Sphere primitive is generally correct behavior in the context of OpenCascade, and I'll be closing this issue (unless you can find an example of someone attempting this in the C++ version and succeeding...)

ancorasir commented 3 years ago

@zalo Actually I succeeded in getting the shell of the sphere using the following code

let holeRadius = Slider("Radius", 30 , 20 , 40);
let sphere     = Sphere(50);
function getShell(shape){
    let anExplorer = new oc.TopExp_Explorer(shape, oc.TopAbs_SHELL);
    for (anExplorer.Init(shape, oc.TopAbs_SHELL); anExplorer.More(); anExplorer.Next()) {
        return oc.TopoDS.prototype.Shell(anExplorer.Current());
    };
};
shell = getShell(sphere);
shell1 = Translate([0,0,50], shell);
sceneShapes.push(shell1);

I can see two spheres in the scene. The strange part is I can get the shell using "return shell" but not ".push(shell)" out side the for loop...And in my first example, the console log inside the for loop has shown that the shell is not null but it can not be obtained outside the for loop. Screenshot from 2020-08-07 14-26-40

zalo commented 3 years ago

Interesting; I notice you’re also casting it to a shell object... I’ll need to investigate more carefully...

ancorasir commented 3 years ago

This problem also occurs to face as well.

function getFace(shape){
    let anExplorer = new oc.TopExp_Explorer(shape, oc.TopAbs_FACE);
    var list = [];
    for (anExplorer.Init(shape, oc.TopAbs_FACE); anExplorer.More(); anExplorer.Next()) {
        ff = oc.TopoDS.prototype.Face(anExplorer.Current());
        list.push(ff);
    };
    return [ff, list];
};

The above code will return null face and list of null face. But if put the return inside the for loop, I will get face and list of face correctly. I guess the problem is about how js get access to the webassembly memory. Somehow, outside for/if, the pointer to the memory get ruined. I'm not familiar with this part... @zalo

zalo commented 3 years ago

Unfortunately, neither am I; perhaps @donalffons (creator of the opencascade.js emscripten cross-compilation) knows more about how the memory/references for shapes are allocated and handled? Perhaps TopExp just needs an IDL modification? Or the references need to be grabbed via special Handle wrapper types?

My current workaround suggestion is to do any work on the faces/shells from within the for-loop; I believe it’s presently how the ForEachEdge is able to handle adding fillets.

Though, I think I’m noticing a sister to this bug in the STEP file reader, where the shape from the last file that is read in overwrites the shapes from earlier files (preventing more than one step from being loaded...)

donalffons commented 3 years ago

This seems to be an issue with how

Here is a slightly modified code example from you @ancorasir (CascadeStudio Example):

let holeRadius = Slider("Radius", 30 , 20 , 40);

let sphere     = Sphere(50);
let cylinderZ  =                     Cylinder(holeRadius, 200, true);
let cylinderY  = Rotate([0,1,0], 90, Cylinder(holeRadius, 200, true));
let cylinderX  = Rotate([1,0,0], 90, Cylinder(holeRadius, 200, true));

Translate([0, 0, 50], Difference(sphere, [cylinderX, cylinderY, cylinderZ]));

let anExplorer = new oc.TopExp_Explorer(sceneShapes[0], oc.TopAbs_SHELL);

anExplorer.Init(sceneShapes[0], oc.TopAbs_SHELL);
const myShell = oc.TopoDS.prototype.Shell(anExplorer.Current()); // I think, casting to a Shell makes no difference
console.log("myShell before 'Next' ", myShell.ptr, myShell.IsNull());
anExplorer.Next();
console.log("myShell after 'Next' ", myShell.ptr, myShell.IsNull())

Output:

myShell before 'Next'  23878428 false
myShell after 'Next'  23878428 true

So, it seems like myShell is some kind of reference (or pointer) whose referenced object changes when you call anExplorer.Next(). The fact that the value of ptr doesn't change seems to mean that the each shell that is made available by TopExp_Explorer is created in the same object / point in memory.

I think what you want to do, in order for the .push(myShell) function to work, is to create a (shallow) copy of myShell before calling anExplorer.Next(). I have used this (hacky) approach in the examples repository. It's not pretty but it works:

let holeRadius = Slider("Radius", 30 , 20 , 40);
let sphere     = Sphere(50);
function getShell(shape){
    let anExplorer = new oc.TopExp_Explorer(shape, oc.TopAbs_SHELL);
    for (anExplorer.Init(shape, oc.TopAbs_SHELL); anExplorer.More(); anExplorer.Next()) {
        const currShell = anExplorer.Current();
        const newShell = new oc.TopExp_Explorer(currShell, oc.TopAbs_SHELL).Current();
        const shell1 = Translate([0,0,50], newShell);
        sceneShapes.push(shell1);
    };
};
getShell(sphere);
zalo commented 3 years ago

That’s a useful explanation and a very clever workaround (akin to wishing for more wishes!)

I’ll poke around OpenCascade’s API to see if there are any more sinless methods of getting to the “bottom pointer”, or creating separate references, and see if I can make a wrapper for that.

zalo commented 3 years ago

On a hunch, I exposed the so-called Move constructors in TopoDS_Shape (and all its faux "subclasses"), which (it turns out) allow you to steal the shape from the iterator (without copying it(?)) in a similar manner to the double TopExp example.

You can see how that works here.

Let me know if this properly addresses your issue 👍

zalo commented 3 years ago

I've used the move constructor to fix a problem with multiple .step import (which now works!) and to fix this problem, so I think it's safe to close this issue.