Conversation
Signed-off-by: Michael Pollind <mpollind@gmail.com>
Signed-off-by: Michael Pollind <mpollind@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR reworks the application context logic by consolidating the SDL application framework into a unified sdl_app.zig module and refactoring how application context is managed. The changes eliminate separate project directories in favor of a single examples/ directory structure, and update the context handling to use a wrapper type AppContext(Context) that includes IO and allocator management.
Changes:
- Renamed
sdl_application.zigtosdl_app.zigand changed the context pattern from heap-allocated instances to a staticAppContextwrapper containing IO, allocator, and frame arena - Updated all example files to use the new
sdl_appmodule andAppContext(Context)pattern with adjusted handler signatures - Removed the separate
examples/02-mesh/project directory and its build files, consolidating the example intoexamples/02Mesh.zig
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/sdl_app.zig | Implements new AppContext wrapper type and updated handler signatures; removes optional context pattern |
| examples/02Mesh.zig | Updates to use new context pattern with sdl_app.AppContext(Context) and removes manual allocator management |
| examples/02-mesh/src/root.zig | Removed entire file |
| examples/02-mesh/src/main.zig | Removed entire file |
| examples/02-mesh/build.zig.zon | Removed entire file |
| examples/02-mesh/build.zig | Removed entire file |
| examples/01Shader.zig | Updates to use new context pattern and replaces local allocator with app_context fields |
| examples/00Clear.zig | Renames AppContext to Context and updates main signature to accept std.process.Init |
| build.zig.zon | Updates vulkan-zig dependency to newer commit |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cntx: *Context, | ||
| io: std.Io, | ||
| gpa: std.mem.Allocator, | ||
| frame_arean: std.heap.ArenaAllocator, |
There was a problem hiding this comment.
Corrected spelling of 'frame_arean' to 'frame_arena'.
| allocator: std.mem.Allocator = undefined, | ||
| frame_heap: std.heap.ArenaAllocator = undefined, | ||
|
|
There was a problem hiding this comment.
The fields 'allocator' and 'frame_heap' are now redundant since AppContext provides 'gpa' and 'frame_arean'. These should either be removed or their usage should be clarified.
| allocator: std.mem.Allocator = undefined, | |
| frame_heap: std.heap.ArenaAllocator = undefined, |
| var cntx: *Context = &app_context.inner; | ||
|
|
||
| app.renderer = try rhi.Renderer.init(allocator, .{ | ||
| cntx.renderer = try rhi.Renderer.init(cntx.gpa, .{ |
There was a problem hiding this comment.
Line 250 references cntx.gpa but the Context struct at lines 10-11 does not have a gpa field. The code should use app_context.gpa instead.
| fn iterate_handler(cntx: *sdl_app.AppContext(Context)) anyerror!sdl_app.sdl.SDL_AppResult { | ||
| while (cntx.timekeeper.consume()) {} |
There was a problem hiding this comment.
Function signature expects *sdl_app.AppContext(Context) but accesses fields like timekeeper directly on cntx, which should be on cntx.inner based on the AppContext structure. This will cause a compilation error.
| .result = sdl_application.sdl.SDL_APP_CONTINUE, | ||
| }; | ||
| cntx.window = window.?; | ||
| cntx.allocator = allocator; |
There was a problem hiding this comment.
The variable allocator is not defined in this scope. Based on the context rework, this should likely use app_context.gpa.
| pub fn main(init: std.process.Init) !void { | ||
| _ = sdl_application.SdlApplicaton(Context, .{ |
There was a problem hiding this comment.
File still references sdl_application instead of the renamed sdl_app module, creating an inconsistency with the refactoring in other example files.
1 participant
No description provided.