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

JS Interface Modification #197

Closed
pca006132 opened this issue Sep 8, 2022 · 19 comments · Fixed by #221
Closed

JS Interface Modification #197

pca006132 opened this issue Sep 8, 2022 · 19 comments · Fixed by #221

Comments

@pca006132
Copy link
Collaborator

It seems that the code that embind generates is not very ergonomic:

  • std::vector<T> is incompatible with javascript array, so we need a new type for each std::vector<T> and users have to use that.
  • Seems to require overloading to generate functions with optional parameters, which is used extensively in our APIs.
  • Unable to convert array to glm::vector2/glm::vector3.
  • Seems hard to get function pointer working (for Warp).

I propose the following changes to our JS interface:

  • Change the naming convention to lower camel case.
  • Make the embind generated bindings internal, and provide a public API implemented with js that performs automatic type conversion mentioned above. (do have some performance penalty)
  • Instead of passing a callback to warp, the API for js will be std::vector<glm::vec3> -> std::vector<glm::vec3>, i.e. give the user the entire buffer for modification, which will likely be faster and don't have to deal with callbacks.
@elalish
Copy link
Owner

elalish commented Sep 8, 2022

Agreed, that sounds like a better approach. I look forward to reviewing the changes. As always, we'll know we're on the right track if we can make clean usage examples. 👍

@pca006132
Copy link
Collaborator Author

There is another thing that I'm not sure whether to put it in the js binding or just in the js editor. Our code requires precise reference counting to free resources, and because our API are functional, users have to insert delete calls everywhere to avoid memory leak. For example:

b = a.translate(1, 1, 1)
b.delete()

This will leak resources as a is not being deleted.

I'm thinking about using something like an arena to manage this for the users, at least in our js editor. The idea is that each API call will register the object into the arena and user don't have to delete it manually. When the script finishes, we will delete every object in the arena. This approach will cause higher memory usage than inserting delete calls, but is much simpler than requiring users to manage the resources by themselves. The problem is I'm not sure whether this should be put into the binding or not.

An alternative way would be to write our own language, which can solve this optimally and allows operator overloading. I'm happy to do that but not sure if users will want another language :P

@elalish
Copy link
Owner

elalish commented Sep 11, 2022

Yeah, as a JS developer I can say we're very much not in the habit of thinking about memory management. Still, there is precedent with the required Three.js dispose() calls for GPU memory management.

Do you think there's a way to make some kind of scope-wrapper that would do what you're describing but in the context of a given user-function? So if I make a JS function that makes a bunch of manifolds and combines them into a single output manifold, the function-wrapper automatically causes those temporary resources to be freed when the function returns, like would happen in C++?

Thanks for the offer, but let's try to avoid making yet another language :P However, what do you think about writing the editor internals in TS? Sounds like a big enough project that it might be nice to have a type-checker.

@pca006132
Copy link
Collaborator Author

Scope-wrapper can be implemented, but it will be buggy when user use yield or async function callback. Should be fine if the users are aware of this.

Regarding the editor internals, I'm not sure what would be the scope of the project. My original idea was just to have some text editor and then run the script, and autocomplete should be provided by a language server or some other libraries. Implementing a custom autocomplete+linting is hard, especially with a flexible language like javascript.

@elalish
Copy link
Owner

elalish commented Sep 11, 2022

Fair enough, if we can keep the editor small and simple, so much the better. And yes, let's just say no async allowed. Honestly there's not much point anyway - we're not doing UI here, and JS is fundamentally single-threaded anyway.

@elalish
Copy link
Owner

elalish commented Sep 23, 2022

@pca006132 Check this out, it turns out you can link into the JS Garbage Collector. Despite all the dire warnings, I think this is ideal for our use case. Basically we can just cause the WASM objects to delete whenever the JS GC would normally delete them. Then we don't even have to think about it, nor invent a wrapper API. What do you think?

@elalish
Copy link
Owner

elalish commented Sep 23, 2022

Hmm, on the other hand, since JS is single-threaded, I'm not sure the GC will have a chance to run during execution, so the idea of garbage collecting inside of functions may not happen. Perhaps something explicit is better.

@pca006132
Copy link
Collaborator Author

I think it is fine to use the finalizer. We only need to call the cleanup to free memory, which is not critical when there is still plenty of memory. Perhaps I can try to implement that and do some tests to see if normal browsers will free up the memory or not. E.g. run menger sponge several times.

@elalish
Copy link
Owner

elalish commented Sep 23, 2022

That would be awesome! And I'll build some more complex examples we can parameterize for number of facets and see if that exposes anything.

@pca006132
Copy link
Collaborator Author

JavaScript only gained support for finalizers in ECMAScript 2021, or ECMA-262 Edition 12. The new API is called FinalizationRegistry and it still does not offer any guarantees that the provided finalization callback will be called. Embind uses this for cleanup if available, but only for smart pointers, and only as a last resort.

I tested it with Firefox 103.0.2 and chromium 104.0.5112.79, it doesn't seem to work. I guess I will just write the code that does the cleanup at the end. And I just found a bug in the rotate binding.

@elalish
Copy link
Owner

elalish commented Sep 24, 2022

Okay, thanks for checking. That sounds good.

@pca006132
Copy link
Collaborator Author

Found that we have several important APIs that are not yet exposed to js: sdf, warp, smooth. Things like tetrahedron, compose/decompose, mesh stats, mesh relation, circular segment related setters are also not yet exposed but should be simple to do so..

Here are some of the examples ported to js:

  • stretchy bracelet:
function base(
    width: number, radius: number, decorRadius: number, twistRadius: number,
    nDecor: number, innerRadius: number, outerRadius: number, cut: number,
    nCut: number, nDivision: number): Manifold {
  function rotate(v: Vec2, theta: number): Vec2 {
    return [
      v[0] * Math.cos(theta) - v[1] * Math.sin(theta),
      v[0] * Math.sin(theta) + v[1] * Math.cos(theta)
    ];
  }

  let b = cylinder(width, radius + twistRadius / 2);
  let circle: Polygons = [[]];
  let dPhiDeg = 180 / nDivision;
  for (let i = 0; i < 2 * nDivision; ++i) {
    circle[0].push([
      decorRadius * Math.cos(dPhiDeg * i * Math.PI / 180) + twistRadius,
      decorRadius * Math.sin(dPhiDeg * i * Math.PI / 180)
    ]);
  }
  let decor =
      extrude(circle, width, nDivision, 180).scale([1, 0.5, 1]).translate([
        0, radius, 0
      ]);
  for (let i = 0; i < nDecor; i++)
    b = b.add(decor.rotate([0, 0, (360.0 / nDecor) * i]));
  let stretch: Polygons = [[]];
  const dPhiRad = 2 * Math.PI / nCut;

  const p0: Vec2 = [outerRadius, 0];
  const p1: Vec2 = [innerRadius, -cut];
  const p2: Vec2 = [innerRadius, cut];
  for (let i = 0; i < nCut; ++i) {
    stretch[0].push(rotate(p0, dPhiRad * i));
    stretch[0].push(rotate(p1, dPhiRad * i));
    stretch[0].push(rotate(p2, dPhiRad * i));
    stretch[0].push(rotate(p0, dPhiRad * i));
  }
  b = intersection(extrude(stretch, width), b);
  return b;
}

function stretchyBracelet(
    radius: number = 30, height: number = 8, width: number = 15,
    thickness: number = 0.4, nDecor: number = 20, nCut: number = 27,
    nDivision: number = 30): Manifold {
  const twistRadius = Math.PI * radius / nDecor;
  const decorRadius = twistRadius * 1.5;
  const outerRadius = radius + (decorRadius + twistRadius) * 0.5;
  const innerRadius = outerRadius - height;
  const cut = 0.5 * (Math.PI * 2 * innerRadius / nCut - thickness);
  const adjThickness = 0.5 * thickness * height / cut;

  return difference(
      base(
          width, radius, decorRadius, twistRadius, nDecor,
          innerRadius + thickness, outerRadius + adjThickness,
          cut - adjThickness, nCut, nDivision),
      base(
          width, radius - thickness, decorRadius, twistRadius, nDecor,
          innerRadius, outerRadius + 3 * adjThickness, cut, nCut, nDivision));
}
  • Rounded frame:
function roundedFrame(edgeLength: number, radius: number, circularSegments: number = 0): Manifold {
  let edge = cylinder(edgeLength, radius, -1, circularSegments);
  let corner = sphere(radius, circularSegments);

  let edge1 = union(corner, edge);
  edge1 = edge1.rotate([-90, 0, 0]).translate([-edgeLength/2, -edgeLength/2, 0]);

  let edge2 = edge1.rotate([0, 0, 180]);
  edge2 = union(edge2, edge1);
  edge2 = union(edge2, edge.translate([-edgeLength/2, -edgeLength/2, 0]));

  let edge4 = edge2.rotate([0, 0, 90]);
  edge4 = union(edge4, edge2);

  let frame = edge4.translate([0, 0, -edgeLength/2]);
  frame = union(frame, frame.rotate([180, 0, 0]));

  return frame;
}
  • Menger sponge:
function vec2add(a: Vec2, b: Vec2): Vec2 {
  return [a[0] + b[0], a[1] + b[1]];
}

// union a set of objects together
// we did not expose the compose interface so we have to do this manually
function compose(objs: Manifold[]): Manifold {
  let result = objs.pop();
  for (let obj of objs) {
    result = union(result, obj);
  }
  return result;
}

function fractal(
    holes: Manifold[], hole: Manifold, w: number, position: Vec2, depth: number,
    maxDepth: number) {
  w /= 3;
  holes.push(
      hole.scale([w, w, 1.0]).translate([position[0], position[1], 0.0]));
  if (depth == maxDepth) return;
  let offsets: Vec2[] = [
    [-w, -w], [-w, 0.0], [-w, w], [0.0, w], [w, w], [w, 0.0], [w, -w], [0.0, -w]
  ];
  for (let offset of offsets)
    fractal(holes, hole, w, vec2add(position, offset), depth + 1, maxDepth);
}

function mengerSponge(n: number) {
  let result = cube([1, 1, 1], true);
  let holes: Manifold[] = [];
  fractal(holes, result, 1.0, [0.0, 0.0], 1, n);

  let hole = compose(holes);

  result = difference(result, hole);
  hole = hole.rotate([90, 0, 0]);
  result = difference(result, hole);
  hole = hole.rotate([0, 0, 90]);
  result = difference(result, hole);

  return result;
}

@elalish
Copy link
Owner

elalish commented Sep 26, 2022

Regarding Warp, is this what we need? If we can figure out how to make that work, I think it'll also be ideal for the SDF.

@pca006132
Copy link
Collaborator Author

Yes

@elalish
Copy link
Owner

elalish commented Sep 26, 2022

I must admit, the docs are a little sparse. Perhaps this is what you were referring to above about it being hard? The function pointer is pretty restricted: only simple integer/float types allowed. To modify a vec3, it seems like we'd just want a JS void function that takes a pointer, but I'm unclear how to dereference it. It seems like there is a WASM HEAP accessible from both the C++ and JS sides, but I can't tell if that's the entire C++ memory layout of the library, or some special area. Hoping your compiler background might help make some sense of this?

@pca006132
Copy link
Collaborator Author

I think it should be fine just to call the getValue and setValue to dereference the pointer (32-bit integers). If you read the MDN page about wasm, they mention that the wasm heap is just a large array that is accessible from js, but the js things are not accessible from wasm unless you explicitly put them there.
The hard thing I mentioned previously is just about how to get function pointers that accepts/return some struct, I did not read it carefully and don't know that it is impossible (we can only do it through pointers).

So I think we need some js side code to help the users. The user API will still be a function accepting a vec3 and returning the changed vec3, but our js code will take care of dereferencing the pointer and adding itself to the wasm table. Maybe you can try to implement it first, and I can help later this week.

@elalish
Copy link
Owner

elalish commented Sep 26, 2022

Okay, that's very helpful, thanks! Agreed regarding the wrapper code; I'll give it a shot tomorrow and see if I can make any progress.

@pca006132
Copy link
Collaborator Author

I will work on the missing APIs for both js and python tmr, so you can focus on dealing with the pypi/npm packaging for the release.

@elalish
Copy link
Owner

elalish commented Sep 30, 2022

Sounds good, thanks!

@elalish elalish mentioned this issue Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants