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

Access to Mesh fields. #17

Open
gravicappa opened this issue Oct 23, 2021 · 8 comments
Open

Access to Mesh fields. #17

gravicappa opened this issue Oct 23, 2021 · 8 comments

Comments

@gravicappa
Copy link

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.

@tjammer
Copy link
Owner

tjammer commented Oct 23, 2021

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
Copy link
Owner

tjammer commented Oct 23, 2021

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

@tjammer tjammer closed this as completed Oct 24, 2021
@tjammer
Copy link
Owner

tjammer commented Oct 24, 2021

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

@gravicappa
Copy link
Author

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.

@tjammer
Copy link
Owner

tjammer commented Nov 7, 2021

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.

@gravicappa
Copy link
Author

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
Copy link
Author

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.

@tjammer tjammer reopened this Jan 17, 2022
@tjammer
Copy link
Owner

tjammer commented Jan 17, 2022

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

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

No branches or pull requests

2 participants