raylib-ocaml icon indicating copy to clipboard operation
raylib-ocaml copied to clipboard

Access to Mesh fields.

Open gravicappa opened this issue 3 years ago • 8 comments

Is it possible to add bindings to access Mesh fields which is useful for mesh generation like here? I've seen such bindings in old version of raylib-ocaml (0.2.0 I think), but it seems to be gone now for some reason.

gravicappa avatar Oct 23 '21 05:10 gravicappa

Yes, definitely! Older versions exposed the ctypes-types directly, I changed that at some point to make the API more readable.

I should find some time this weekend to add them

tjammer avatar Oct 23 '21 09:10 tjammer

I added the getters and setters to the module and added the mesh_generation example, see here.

tjammer avatar Oct 23 '21 20:10 tjammer

I'll do a new release to opam soon so other people can use it. Until then, you can build from source.

If you run into any troubles, feel free to comment here again, then I will repon the ticket

tjammer avatar Oct 24 '21 16:10 tjammer

Hello again.

I am not ctypes expert so I have a question. When in this code CArray instance created by of_list function will be collected? Should I wrap mesh object to keep all respective arrays reachable to OCaml GC to avoid memory corruption? I've encountered suspicious issues that may be related to CArray lifetimes.

Thanks.

gravicappa avatar Nov 06 '21 17:11 gravicappa

What are the issues you've encountered?

I notice that when I force gc in the example with GC.full_major (), I get errors when unloading the models with Array.iter unload_model models at the end.

free(): double free detected in tcache 2
double free or corruption (out)

I suspect that raylib tries to free the memory even though that's handled by the OCaml GC in this case? I didn't have any problems at runtime though and get no errors if I just don't call unload_model.

tjammer avatar Nov 07 '21 13:11 tjammer

I was generating a number of meshes (with ~1k vertices) and noticed visual glitches when drawing them.

Some refined pseudocode:

let finalise _ = prerr_endline "carray is freed" in

let carray_of_queue queue =
  Ctypes.Carray.make ~finalise … queue.length in

let mesh = Raylib.Mesh.create () in
Raylib.Mesh.set_vertices mesh (carray_of_queue vertices)
Raylib.Mesh.set_texcoords mesh (carray_of_queue tex_coords);
Raylib.Mesh.set_normals mesh (carray_of_queue normals);
Raylib.Mesh.set_indices mesh (carray_of_queue indices);
Raylib.upload_mesh (Raylib.addr mesh) false;

Then I observed carray is freed in logs before the mesh is uploaded or drawn. As far as I understood Ctypes.CArray.t instance returned by carray_of_queue is not accessible right after the function call and is being freed because Mesh.t instance is opaque to OCaml GC. That's probably naive conclusion but the issue was fixed after I returned wrapped object like { mesh; verts_carray; normals_carray; … } to keep Ctypes.CArray.t instances accessible for GC.

gravicappa avatar Nov 07 '21 17:11 gravicappa

Hello.

As currently implemented mesh manipulations are fragile:

  1. Without storing the carray instances separately for preventing being collected by GC leads to memory corruption issues.
  2. Calling unload_mesh causes double free issue and occasional crashes.

I've managed to come up with the following workarounds:

let create () =
  let mesh = Raylib.Mesh.create () in
  ...
  Raylib.Mesh.set_vertices mesh vertices;

  (* Fixes #1.
     The implementation is stolen from discussion of the aforementioned ctypes
     issue *)
  add_gc_link ~from: mesh ~to_: vertices;
  mesh

let unload mesh =
  let empty t =
    let ptr = Ctypes.from_voidp t Ctypes.null in
    Ctypes.CArray.from_ptr ptr 0 in

  (* Fixes #2. *)
  Raylib.Mesh.set_vertices mesh (empty Ctypes.float);
  ...
  Raylib.unload_mesh

But I presume since Raylib itself is positioned as a tool for those who tries to avoid meddling with low-level stuff this workarounds are not obvious. Probably it should be done in the library itself. Or at least the issues should be noted in documentation.

gravicappa avatar Jan 16 '22 18:01 gravicappa

Thanks for keeping up on this.

I agree that those interactions are not obvious and are best handled in the bindings (somehow). For the first issue, we can probably wrap the (two?) CArray create functions and add the workaround.

For the second issue I'm not so sure though

tjammer avatar Jan 17 '22 16:01 tjammer