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

Add MeshUnmanaged (#299) #304

Merged

Conversation

furudbat
Copy link
Contributor

@furudbat furudbat commented Mar 8, 2024

No description provided.

@furudbat
Copy link
Contributor Author

furudbat commented Mar 8, 2024

I'm still not 100% happy with the solution, you can still mess up the construction via implicit cast.

raylib::Mesh mesh = raylib::Mesh::Cubicmap(imMap, Vector3{ 1.0f, 1.0f, 1.0f });     // Use MeshUnmanaged, Mesh will be handled by Model
    raylib::Model model(mesh);
INFO: TEXTURE: [ID 4] Unloaded texture data from VRAM (GPU)
INFO: VAO: [ID 2] Unloaded vertex array data from VRAM (GPU)
INFO: MODEL: Unloaded model (and meshes) from RAM and VRAM
INFO: VAO: [ID 2] Unloaded vertex array data from VRAM (GPU)
double free or corruption (!prev)

Process finished with exit code 134 (interrupted by signal 6:SIGABRT)

@furudbat
Copy link
Contributor Author

furudbat commented Mar 8, 2024

Maybe making the Load() method using r-value ref, can prevent some mishap.

void Load(const ::Mesh& mesh) = delete;
void Load(::Mesh&& mesh) { ... }
void Load(const raylib::Mesh& mesh) = delete;
void Load(raylib::Mesh&& mesh) { ... }

@@ -25,7 +25,7 @@ int main(void)

raylib::Image imMap("resources/cubicmap.png"); // Load cubicmap image (RAM)
raylib::Texture cubicmap(imMap); // Convert image to texture to display (VRAM)
Mesh mesh = raylib::Mesh::Cubicmap(imMap, Vector3{ 1.0f, 1.0f, 1.0f });
raylib::MeshUnmanaged mesh = raylib::MeshUnmanaged::Cubicmap(imMap, Vector3{ 1.0f, 1.0f, 1.0f }); // Use MeshUnmanaged, Mesh will be handled by Model
raylib::Model model(mesh);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this on! Definitely does make sense since Model ends up taking ownership of the Model.

In addition to having MeshUnmanaged, I think having an overloaded method for raylib::Model(raylib::Mesh) that creates a copy of the Mesh as MeshUnmanaged, and uses that. This way the example code wouldn't need to change, but memory would still be 👌

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge this forwards, and then we can take it on as a follow up PR.

@RobLoach RobLoach merged commit 9176e0a into RobLoach:master Mar 9, 2024
8 checks passed
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 this pull request may close these issues.

2 participants