feat: implemented create and update texture #2
Loading…
Reference in a new issue
No description provided.
Delete branch "coding-agent/zenolith-sdl2:feature/create-texture"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Hi,
Made a few changes in zenolith-sdl2 platform and texture to be able to create texture and update it. Please review it and let me know if any changes are required.
A decent first try knowing you, although there's a lot of improvements to be done.
Also change
Sdl2Platform.createFont
such that it usescreateFont
andSdl2Font.addAtlasSprite
such that it usesupdateTexture
. Thanks!@ -366,3 +366,3 @@
const atlas = c.SDL_CreateTexture(
self.renderer,
c.SDL_PIXELFORMAT_RGBA8888,
c.c.SDLPIXELFORMAT_RGBA8888,
WTF (this doesn't even compile)
@ -401,2 +401,4 @@
});
}
const PixelAccess = enum {
Turn this into an
enum(c_int)
and remove anyswitch
es converting it to the respective SDL2 value.Also move it into
Sdl2Texture
and mark itpub
@ -403,0 +407,4 @@
target,
};
pub fn createTexture(
Create a struct
CreateTextureOptions
, add fields for all possible options including sensible defaults where possible.createTexture
should only take this struct as an argument (except for theself
parameter).@ -403,0 +409,4 @@
pub fn createTexture(
self: *Sdl2Platform,
width: c_int,
Take a
zenolith.layout.Size
instead of 2 separate size params inCreateTextureOptions
.@ -403,0 +411,4 @@
self: *Sdl2Platform,
width: c_int,
height: c_int,
pixel_format: util.PixelFormat,
This should default to
.static
in the options struct.@ -403,0 +430,4 @@
) orelse return error.CreateTexture;
errdefer c.SDL_DestroyTexture(texture);
if (c.SDL_SetTextureBlendMode(texture, c.SDL_BLENDMODE_BLEND) != 0) return error.CreateTexture;
This is use case-specific, take an optional parameter in the options struct and set this accordingly.
@ -22,2 +22,4 @@
return .{ .width = @intCast(w), .height = @intCast(h) };
}
pub fn updateTexture(self: *Sdl2Texture, comptime T: type, pixels: []const T, pitch: c_int) !void {
It makes no sense for this to be generic, the original just takes a
void*
due to crap API design. Take a[]const u8
here. Also useu31
forpitch
, negative pitch doesn't make sense.@ -23,1 +23,4 @@
}
pub fn updateTexture(self: *Sdl2Texture, comptime T: type, pixels: []const T, pitch: c_int) !void {
if (c.SDL_UpdateTexture(self.tex, null, pixels.ptr, pitch) != 0) return error.UpdateTexture;
Don't hard-code null here, take a
?zenolith.layout.Rectangle
and use it here if provided.@ -413,2 +413,4 @@
};
}
pub const PixelFormat = enum {
Remove the
unknown
variant, turn this into anenum(c_int)
and move it intoSdl2Texture
.@ -415,0 +458,4 @@
NV21,
};
pub fn toSdlPixelFormat(pixel_format: PixelFormat) c_int {
This function is redundant.
Thanks for the review!
I'm thinking on the logic to implement this but I am not too sure how. Because to use this function I should have the platform already and call it as a method. I think a way around it would be to have the function receiving the renderer as an argument instead of self. Please let me know your thought on it.
You should simply replace the
*c.SDL_Texture
inSdl2Font
with aSdl2Texture
which you can then invokesetPixels
on.Weitermachen 👍
@ -402,1 +402,4 @@
}
const CreateTextureOptions = struct {
width: c_int,
height: c_int,
Take a
zenolith.layout.Size
instead of width and height.@ -403,0 +405,4 @@
height: c_int,
pixel_format: Sdl2Texture.PixelFormat,
pixel_access: Sdl2Texture.PixelAccess = .static,
blend_mode: Sdl2Texture.TextureBlendMode = .none,
On second thought, let's remove this from createTexture and instead add a
setBlendMode
function toSdl2Texture
.@ -403,0 +418,4 @@
) orelse return error.CreateTexture;
errdefer c.SDL_DestroyTexture(texture);
if (c.SDL_SetTextureBlendMode(texture, options.blend_mode) != 0) return error.CreateTexture;
Remove this.
@ -22,2 +80,4 @@
return .{ .width = @intCast(w), .height = @intCast(h) };
}
pub fn updateTexture(self: *Sdl2Texture, pixels: []const u8, pitch: u31, rect: ?zenolith.layout.Rectangle) !void {
Rename to
setPixels
.@ -23,1 +81,4 @@
}
pub fn updateTexture(self: *Sdl2Texture, pixels: []const u8, pitch: u31, rect: ?zenolith.layout.Rectangle) !void {
if (c.SDL_UpdateTexture(self.tex, rect, pixels.ptr, pitch) != 0) return error.UpdateTexture;
Add an
std.debug.assert(pixels.len % pitch == 0);
before this as a sanity check.Also, you're missing an
@intFromEnum
here, this will almost certainly result in a compile error once this is referenced, which will happen once you make the font stuff use this :D@ -413,2 +413,4 @@
};
}
pub const PixelFormat = enum(c_int) {
Why is this here? We already have this in
Sdl2Texture
.Vorfreude! (probably not using the word correctly but I think you get it)
@ -31,3 +33,3 @@
pub fn deinit(self: *Sdl2Font) void {
c.SDL_DestroyTexture(self.atlas);
c.SDL_DestroyTexture(self.atlas.tex);
Add a
deinit
function toSdl2Texture
and call that instead.@ -403,0 +406,4 @@
pub fn createTexture(self: Sdl2Platform, options: CreateTextureOptions) !Texture {
const texture = c.SDL_CreateTexture(
self.renderer,
@intCast(@intFromEnum(options.pixel_format)),
intCast
shouldn't be required here; if these params aren'tc_int
s, change the repr of the enum instead.@ -403,0 +411,4 @@
options.size.width,
options.size.height,
) orelse return error.CreateTexture;
errdefer c.SDL_DestroyTexture(texture);
This is redundant as no errors can be returned after this line.
@ -11,2 +12,4 @@
const Sdl2Texture = @This();
/// removed 32 formats as some of them are the same values
/// as 8888 formats
Replace the comment with this:
(also understand what this means :D)
I will read more about it...
@ -22,2 +79,4 @@
return .{ .width = @intCast(w), .height = @intCast(h) };
}
pub fn setPixels(self: *Sdl2Texture, pixels: []const u8, pitch: u31, rect: ?zenolith.layout.Rectangle) !void {
No need to use a pointer for
self
.@ -24,0 +83,4 @@
std.debug.assert(pixels.len % pitch == 0);
const rec = if (rect) |r| util.toSdlRect(r) else null;
if (c.SDL_UpdateTexture(self.tex, @ptrCast(&rec), pixels.ptr, pitch) != 0) return error.UpdateTexture;
This is unsound, as this will incorrectly cast a pointer to an optional to an optional pointer in some cases. Do not throw around
ptrCast
like this. The&
should go inside theif
.@ -24,0 +86,4 @@
if (c.SDL_UpdateTexture(self.tex, @ptrCast(&rec), pixels.ptr, pitch) != 0) return error.UpdateTexture;
}
pub fn setBlendMode(self: *Sdl2Texture, blend_mode: TextureBlendMode) !void {
No need to use a pointer for
self
.One last thing!
@ -3,6 +3,7 @@ const std = @import("std");
const zenolith = @import("zenolith");
const c = @import("ffi.zig").c;
const util = @import("util.zig");
Redundant import
This import is actually being used here
My bad!