donalffons / opencascade.js

Port of the OpenCascade CAD library to JavaScript and WebAssembly via Emscripten.
https://ocjs.org/
GNU Lesser General Public License v2.1
625 stars 91 forks source link

embind pointer-to-reference conversion question #11

Closed pentacular closed 4 years ago

pentacular commented 4 years ago

This is working until it gets to makeFace.Add(wire).

I can't figure out if I'm doing this wrong, or if there is a problem in the bindings.

makeFace.Add's binding entry is

.function("Add", static_cast<void (BRepBuilderAPI_MakeFace::*) (const TopoDS_Wire &) >(&BRepBuilderAPI_MakeFace::Add), allow_raw_pointers())

So it's weird that it's complaining that it wants a conversion to TopoDS_Wire.

Any idea?

This is using the embind branch

test('Offset', async t => {
  const oc = await getOc();
  const path = [[0, 0, 0], [1, 0, 0], [1, 1, 0]].map(([x, y, z]) => new oc.gp_Pnt_3(x, y, z));
  const makePolygon = new oc.BRepBuilderAPI_MakePolygon_1();
  for (let nth = 0; nth < path.length; nth++) {
    makePolygon.Add_1(path[nth]);
  }
  const wire = makePolygon.Wire();
  const makeFace = new oc.BRepBuilderAPI_MakeFace_1();
  makeFace.Add(wire);
/*
  @Object {
    message: 'Cannot convert argument of type TopoDS_Wire const* to parameter type TopoDS_Wire',
    name: 'BindingError',
    stack: `BindingError: Cannot convert argument of type TopoDS_Wire const* to parameter type TopoDS_Wire␊
        at Object.eval (webpack-internal:///../../../opencascade.js/dist/opencascade.wasm.js:12:79850)␊
        at new eval (webpack-internal:///../../../opencascade.js/dist/opencascade.wasm.js:12:79667)␊
        at throwBindingError (webpack-internal:///../../../opencascade.js/dist/opencascade.wasm.js:12:80291)␊
        at RegisteredPointer.genericPointerToWireType [as toWireType] (webpack-internal:///../../../opencascade.js/dist/opencascade.wasm.js:12:88943)␊
        at Object.eval [as Add] (webpack-internal:///../../../opencascade.js/dist/opencascade.wasm.js:12:100147)␊

*/
});
donalffons commented 4 years ago

Hi, yeah I have similar issues with the examples repository (const* to non-const conversion). This problem has been introduced with one of the latest commits. I'm in the process of fixing it. Once I have the issue resolved locally, I will push updated builds to the embind branch.

Looks like we'll need some kind of test suite eventually :slightly_smiling_face:. Thanks for reporting!

donalffons commented 4 years ago

I made some fixes for this issue in this commit. Could you try the latest embind branch and let me know if this fixes your problem?

pentacular commented 4 years ago

Thanks for checking it out.

I'll give it a try, but it may be a day or two before I can report.

pentacular commented 4 years ago

I have to say, I really, really do not like the 'patience is a virtue' replacing useful log output.

donalffons commented 4 years ago

But I put a lot of thought into that sentence! :wink: But seriously, I read somehwere (can't find it right now) that someone else experienced the issue of docker containers becoming unresponsive zombies, when they output large amounts of text to stdout over long times. For testing purposes, I am currently suppressing all outputs and am replacing it with this infrequent repeating line just to know if the container is already hanging or still working.

I agree that zero debugging output from the builds isn't exactly helpful. I will post an update in that pull request when (if?) I find a solution to this problem.

EDIT: Maybe you can stick to the commit I mentioned earlier for the moment. I wouldn't merge this to master with no debug output, of course.

pentacular commented 4 years ago

Sounds good and makes more sense.

It was easy enough to reenable locally :)

pentacular commented 4 years ago

Ok, message: 'Cannot convert argument of type TopoDS_Wire const* to parameter type TopoDS_Wire', seems fixed.

But now I'm getting message: 'oc.BRep_Tool.prototype.Pnt is not a function', for const gp_Pnt = oc.BRep_Tool.prototype.Pnt(e.CurrentVertex());

This works (used to work?) on the master branch.

oc.BRep_Tool.Pnt fails also new oc.BRep_Tool().Pnt(e.CurrentVertex()) is also having problems.

Did static methods get broken?

The binding entry I see for this is: .class_function("Pnt", static_cast<gp_Pnt (*) (const TopoDS_Vertex &) >(&BRep_Tool::Pnt), allow_raw_pointers())

donalffons commented 4 years ago

But now I'm getting message: oc.BRep_Tool.prototype.Pnt is not a function, for const gp_Pnt = oc.BRep_Tool.prototype.Pnt(e.CurrentVertex());

The interface changed slightly for static methods since we are moving from WebIDL to Embind. You should be able to simply use oc.BRep_Tool.Pnt without using prototype - but I cannot test this right now. Have a look here for an example on how to use static methods with Embind: https://github.com/donalffons/opencascade.js-examples/blob/embind/src/demos/bottle%20-%20basic/library.js#L136

Does this fix your issues?

donalffons commented 4 years ago

... I have been working on a testing system and added a test based on your code to the test "suite" (it has just two tests as of now). Here is what seems to work:

test('Create Polygonal Face', async () => {
  const oc = await((await new opencascade()).ready);
  const path = [[0, 0, 0], [1, 0, 0], [1, 1, 0]].map(([x, y, z]) => new oc.gp_Pnt_3(x, y, z));
  const makePolygon = new oc.BRepBuilderAPI_MakePolygon_1();
  for (let nth = 0; nth < path.length; nth++) {
    makePolygon.Add_1(path[nth]);
  }
  const wire = makePolygon.Wire();
  new oc.BRepBuilderAPI_MakeFace_15(wire, false);
});

i.e. instead of creating an empty BRepBuilderAPI_MakeFace and then adding wires to it, I'm using the specialized constructor for constructing a face from a wire provided by BRepBuilderAPI_MakeFace.

If you want to display that polygon in a 3d viewport, it seems like you have to create a TopoDS_Compound which holds your face (at least the current tessellation helpers in opencascade.js-examples seem to require that). For testing, I modified the file src/bottle - basic/index.js in opencascade.js-examples and made the following changes:

/*45*/  // ...
/*46*/  // let width = 50, height = 70, thickness = 30;
/*47*/  // let bottle = makeBottle(openCascade, width, height, thickness); // remove code to create bottle
/*48*/  const builder = new openCascade.BRep_Builder();
/*49*/  const aComp = new openCascade.TopoDS_Compound();
/*50*/  builder.MakeCompound(aComp);
/*51*/  const path = [[0, 0, 0], [100, 0, 0], [100, 100, 0]].map(([x, y, z]) => new openCascade.gp_Pnt_3(x, y, z));
/*52*/  const makePolygon = new openCascade.BRepBuilderAPI_MakePolygon_3(path[0], path[1], path[2], true); // use this constructor, otherwise the polygon is not closed and triangulation will fail
/*53*/  const wire = makePolygon.Wire();
/*54*/  const f = new openCascade.BRepBuilderAPI_MakeFace_15(wire, false);
/*55*/  builder.Add(aComp, f.Shape());
/*56*/  await addShape(openCascade, aComp, scene); // plug in your shape here
/*57*/
/*58*/  window.changeSliderWidth = value => {
/*59*/  // ...

(Can't wait for CascadeStudio to be using the new bindings, so that it will be easier to share these types of snippets).

What a beautiy! image

donalffons commented 4 years ago

Live demo: https://donalffons.github.io/opencascade.js-examples/demos/polygon/ Code is in the examples repository.