Merge pull request #54 from Interrupt/fixes/quakemdl

Getting the QuakeMdl and QuakeMap examples to run cleanly

Author: Interrupt

src/examples/quakemap.zig

@@ -8,6 +8,7 @@ const graphics = delve.platform.graphics;
 const math = delve.math;
 
 var camera: delve.graphics.camera.Camera = undefined;
+var shader: delve.platform.graphics.Shader = undefined;
 var fallback_material: graphics.Material = undefined;
 var fallback_quake_material: delve.utils.quakemap.QuakeMaterial = undefined;
 var materials: std.StringHashMap(delve.utils.quakemap.QuakeMaterial) = undefined;
@@ -33,6 +34,7 @@ pub fn main() !void {
         .init_fn = on_init,
         .tick_fn = on_tick,
         .draw_fn = on_draw,
+        .cleanup_fn = on_cleanup,
     };
 
     // Pick the allocator to use depending on platform
@@ -42,7 +44,8 @@ pub fn main() !void {
         // See https://github.com/ziglang/zig/issues/19072
         try delve.init(std.heap.c_allocator);
     } else {
-        try delve.init(gpa.allocator());
+        // Using the default allocator will let us detect memory leaks
+        try delve.init(delve.mem.createDefaultAllocator());
     }
 
     try delve.modules.registerModule(example);
@@ -146,9 +149,11 @@ pub fn on_init() !void {
     var err: delve.utils.quakemap.ErrorInfo = undefined;
     quake_map = try delve.utils.quakemap.QuakeMap.read(allocator, test_map_file, map_transform, &err);
 
+    shader = graphics.Shader.initDefault(.{ .vertex_attributes = delve.graphics.mesh.getShaderAttributes() });
+
     // Create a material out of the texture
     fallback_material = try graphics.Material.init(.{
-        .shader = graphics.Shader.initDefault(.{ .vertex_attributes = delve.graphics.mesh.getShaderAttributes() }),
+        .shader = shader,
         .texture_0 = fallback_tex,
         .samplers = &[_]graphics.FilterMode{.NEAREST},
     });
@@ -163,7 +168,6 @@ pub fn on_init() !void {
     player_pos = camera.position;
 
     materials = std.StringHashMap(delve.utils.quakemap.QuakeMaterial).init(allocator);
-    const shader = graphics.Shader.initDefault(.{ .vertex_attributes = delve.graphics.mesh.getShaderAttributes() });
 
     for (quake_map.worldspawn.solids.items) |solid| {
         for (solid.faces.items) |face| {
@@ -324,3 +328,12 @@ pub fn setGravityCmd(new_gravity: f32) void {
     gravity = new_gravity;
     delve.debug.log("Changed gravity to {d}", .{gravity});
 }
+
+pub fn on_cleanup() !void {
+    var it = materials.valueIterator();
+    while (it.next()) |mat_ptr| {
+        mat_ptr.material.deinit();
+    }
+    materials.deinit();
+    shader.destroy();
+}

src/examples/quakemdl.zig

@@ -21,7 +21,8 @@ pub fn main() !void {
         // See https://github.com/ziglang/zig/issues/19072
         try delve.init(std.heap.c_allocator);
     } else {
-        try delve.init(gpa.allocator());
+        // Using the default allocator will let us detect memory leaks
+        try delve.init(delve.mem.createDefaultAllocator());
     }
 
     const example = delve.modules.Module{

src/framework/utils/quakemdl.zig

@@ -11,18 +11,33 @@ const graphics = @import("../platform/graphics.zig");
 
 const assert = std.debug.assert;
 const Allocator = std.mem.Allocator;
+const ArenaAllocator = std.heap.ArenaAllocator;
 const File = std.fs.File;
 
 pub const MDL = struct {
     frames: []MDLFrameType,
     skins: []MDLSkinType,
-    material: *graphics.Material,
-    allocator: Allocator,
+    material: graphics.Material,
+    arena_allocator: ArenaAllocator,
 
     pub fn deinit(self: *MDL) void {
-        self.allocator.free(self.frames);
-        self.allocator.free(self.skins);
-        self.allocator.destroy(self.material);
+        for (self.skins) |skin| {
+            switch (skin) {
+                .single => |*frame| {
+                    // self.allocator.free(frame.pixels);
+                    @constCast(&frame.texture).destroy();
+                },
+                .group => |group| {
+                    for (group.textures) |tex| {
+                        @constCast(&tex).destroy();
+                    }
+                    // self.allocator.free(group.frames);
+                },
+            }
+        }
+
+        self.arena_allocator.deinit();
+        self.material.deinit();
     }
 };
 
@@ -97,7 +112,7 @@ const MDLMeshBuildConfig_ = struct {
     skin_width: f32,
     skin_height: f32,
     transform: math.Mat4,
-    material: *graphics.Material,
+    material: graphics.Material,
 };
 
 const MDLSkin_ = struct {
@@ -130,6 +145,7 @@ const MDLSkin_ = struct {
     pub fn toTexture(self: *const MDLSkin_, allocator: Allocator) !graphics.Texture {
         const size: u32 = self.width * self.height;
         const bytes = try allocator.alloc(u8, size * 4);
+        defer allocator.free(bytes);
 
         for (0.., self.pixels) |j, index| {
             const i = @as(u32, index);
@@ -150,6 +166,7 @@ const MDLSkinGroup_ = struct {
     count: u32,
     intervals: []f32,
     pixels: []u8,
+    allocator: Allocator,
 
     pub fn read(allocator: Allocator, file: File, width: u32, height: u32) !MDLSkinGroup_ {
         // Skin type
@@ -165,7 +182,7 @@ const MDLSkinGroup_ = struct {
         // Skin intervals
         const intervals_buff = try allocator.alloc(u8, count * @sizeOf(f32));
         _ = try file.read(intervals_buff);
-        const intervals: []f32 = try bytesToStructArray(f32, intervals_buff);
+        const intervals: []f32 = try bytesToStructArray(f32, allocator, intervals_buff);
 
         // Skin pixels
         const size: u32 = width * height * count;
@@ -179,12 +196,19 @@ const MDLSkinGroup_ = struct {
             .count = count,
             .intervals = intervals,
             .pixels = pixels,
+            .allocator = allocator,
         };
     }
 
+    pub fn deinit(self: *MDLSkinGroup_) void {
+        self.allocator.free(self.intervals);
+        self.allocator.free(self.pixels);
+    }
+
     pub fn toTexture(self: *const MDLSkinGroup_, allocator: Allocator, index: u32) !graphics.Texture {
         const size: u32 = self.width * self.height;
         const bytes = try allocator.alloc(u8, size * 4);
+        defer allocator.free(bytes);
 
         const start = index * size;
         const end = start + size;
@@ -225,7 +249,7 @@ const MDLFrame_ = struct {
         const vertbuff = try allocator.alloc(u8, @sizeOf(TriVertex_) * vertex_count);
         defer allocator.free(vertbuff);
         _ = try file.read(vertbuff);
-        const trivertexes = try bytesToStructArray(TriVertex_, vertbuff);
+        const trivertexes = try bytesToStructArray(TriVertex_, allocator, vertbuff);
 
         return .{
             .min = min,
@@ -258,7 +282,7 @@ const MDLFrameGroup_ = struct {
         // Frame intervals
         const intervals_buff = try allocator.alloc(u8, count * @sizeOf(f32));
         _ = try file.read(intervals_buff);
-        const intervals: []f32 = try bytesToStructArray(f32, intervals_buff);
+        const intervals: []f32 = try bytesToStructArray(f32, allocator, intervals_buff);
 
         // Frames
         const frames: []MDLFrame_ = try allocator.alloc(MDLFrame_, count);
@@ -307,8 +331,7 @@ const TriVertex_ = struct {
     }
 };
 
-fn bytesToStructArray(comptime T: type, bytes: []u8) std.mem.Allocator.Error![]T {
-    const allocator = mem.getAllocator();
+fn bytesToStructArray(comptime T: type, allocator: Allocator, bytes: []u8) std.mem.Allocator.Error![]T {
     const size: u32 = @sizeOf(T);
     const length: u32 = @as(u32, @intCast(bytes.len)) / size;
     const result: []T = try allocator.alloc(T, length);
@@ -392,7 +415,10 @@ fn makeMesh(allocator: Allocator, frame: MDLFrame_, config: MDLMeshBuildConfig_)
     return builder.buildMesh(config.material);
 }
 
-pub fn open(allocator: Allocator, path: []const u8) !MDL {
+pub fn open(in_allocator: Allocator, path: []const u8) !MDL {
+    var arena = ArenaAllocator.init(in_allocator);
+    var allocator = arena.allocator();
+
     var file = try std.fs.cwd().openFile(
         path,
         std.fs.File.OpenFlags{
@@ -416,9 +442,10 @@ pub fn open(allocator: Allocator, path: []const u8) !MDL {
         const skin_type: SkinType = @enumFromInt(@as(u32, @bitCast(work)));
 
         if (skin_type == SkinType.SINGLE) {
-            const skin = try MDLSkin_.read(allocator, file, header.skin_width, header.skin_height);
+            var skin = try MDLSkin_.read(allocator, file, header.skin_width, header.skin_height);
             const texture = try skin.toTexture(allocator);
             skins[i] = .{ .single = .{ .texture = texture } };
+            defer allocator.free(skin.pixels);
         } else if (skin_type == SkinType.GROUP) {
             const group = try MDLSkinGroup_.read(allocator, file, header.skin_width, header.skin_height);
 
@@ -437,8 +464,9 @@ pub fn open(allocator: Allocator, path: []const u8) !MDL {
     }
 
     // Material
-    const default_material = graphics.Material.init(.{
+    const default_material = try graphics.Material.init(.{
         .shader = graphics.Shader.initFromBuiltin(.{ .vertex_attributes = mesh.getShaderAttributes() }, default_mesh),
+        .own_shader = true,
         .texture_0 = skins[0].single.texture,
         .samplers = &[_]graphics.FilterMode{.NEAREST},
     });
@@ -447,20 +475,16 @@ pub fn open(allocator: Allocator, path: []const u8) !MDL {
     const stvert_buff: []u8 = try allocator.alloc(u8, @sizeOf(STVertex_) * header.vertex_count);
     defer allocator.free(stvert_buff);
     _ = try file.read(stvert_buff);
-    const stvertices = try bytesToStructArray(STVertex_, stvert_buff);
+    const stvertices = try bytesToStructArray(STVertex_, allocator, stvert_buff);
     defer allocator.free(stvertices);
 
     // Triangles
     const triangle_buff: []u8 = try allocator.alloc(u8, @sizeOf(Triangle_) * header.triangle_count);
     defer allocator.free(triangle_buff);
     _ = try file.read(triangle_buff);
-    const triangles = try bytesToStructArray(Triangle_, triangle_buff);
+    const triangles = try bytesToStructArray(Triangle_, allocator, triangle_buff);
     defer allocator.free(triangles);
 
-    var material = try allocator.create(graphics.Material);
-    material.* = default_material;
-    material.textures[0] = skins[0].single.texture;
-
     // Transform
     var m = math.Mat4.identity;
     m = m.mul(math.Mat4.translate(math.vec3(header.origin[0], header.origin[2], header.origin[1])));
@@ -478,7 +502,7 @@ pub fn open(allocator: Allocator, path: []const u8) !MDL {
         .stvertexes = stvertices,
         .triangles = triangles,
         .transform = m,
-        .material = material,
+        .material = default_material,
     };
 
     // Frames
@@ -521,8 +545,8 @@ pub fn open(allocator: Allocator, path: []const u8) !MDL {
     const mdl: MDL = .{
         .frames = frames,
         .skins = skins,
-        .material = material,
-        .allocator = allocator,
+        .material = default_material,
+        .arena_allocator = arena,
     };
 
     return mdl;