From 6edeb45e7ed19708e5d261bdeca617c7c9483267 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Mon, 2 Sep 2024 00:37:10 -0500 Subject: [PATCH] kitty graphics: address review comments - move wuffs code from src/ to pkg/ - eliminate stray debug logs - make logs a warning instead of an error - remove initialization of some structs to zero --- build.zig | 21 ++++------------- build.zig.zon | 5 +--- pkg/wuffs/build.zig | 30 ++++++++++++++++++++++++ pkg/wuffs/build.zig.zon | 17 ++++++++++++++ {src/wuffs => pkg/wuffs/src}/c.zig | 0 {src/wuffs => pkg/wuffs/src}/defs.zig | 0 {src/wuffs => pkg/wuffs/src}/error.zig | 0 {src/wuffs => pkg/wuffs/src}/main.zig | 0 {src/wuffs => pkg/wuffs/src}/png.zig | 27 ++++++++++----------- {src/wuffs => pkg/wuffs/src}/swizzle.zig | 3 ++- src/renderer/opengl/image.zig | 2 +- src/terminal/kitty/graphics_image.zig | 2 +- 12 files changed, 70 insertions(+), 37 deletions(-) create mode 100644 pkg/wuffs/build.zig create mode 100644 pkg/wuffs/build.zig.zon rename {src/wuffs => pkg/wuffs/src}/c.zig (100%) rename {src/wuffs => pkg/wuffs/src}/defs.zig (100%) rename {src/wuffs => pkg/wuffs/src}/error.zig (100%) rename {src/wuffs => pkg/wuffs/src}/main.zig (100%) rename {src/wuffs => pkg/wuffs/src}/png.zig (85%) rename {src/wuffs => pkg/wuffs/src}/swizzle.zig (98%) diff --git a/build.zig b/build.zig index 8bf51def8..5227e9895 100644 --- a/build.zig +++ b/build.zig @@ -1038,7 +1038,10 @@ fn addDeps( .images = false, .text_input = false, }); - const wuffs_dep = b.dependency("wuffs", .{}); + const wuffs_dep = b.dependency("wuffs", .{ + .target = target, + .optimize = optimize, + }); // Wasm we do manually since it is such a different build. if (step.rootModuleTarget().cpu.arch == .wasm32) { @@ -1063,21 +1066,6 @@ fn addDeps( step.addIncludePath(b.path("src/apprt/gtk")); } - step.addIncludePath(wuffs_dep.path("release/c")); - step.addCSourceFile( - .{ - .file = wuffs_dep.path("release/c/wuffs-v0.4.c"), - .flags = f: { - const flags = @import("src/wuffs/defs.zig").build; - var a: [flags.len][]const u8 = undefined; - inline for (flags, 0..) |flag, i| { - a[i] = "-D" ++ flag ++ "=1"; - } - break :f &a; - }, - }, - ); - // C++ files step.linkLibCpp(); step.addIncludePath(b.path("src")); @@ -1139,6 +1127,7 @@ fn addDeps( step.root_module.addImport("sentry", sentry_dep.module("sentry")); step.root_module.addImport("ziglyph", ziglyph_dep.module("ziglyph")); step.root_module.addImport("vaxis", vaxis_dep.module("vaxis")); + step.root_module.addImport("wuffs", wuffs_dep.module("wuffs")); // Mac Stuff if (step.rootModuleTarget().isDarwin()) { diff --git a/build.zig.zon b/build.zig.zon index 90bcc8ecd..bd86b3427 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -40,11 +40,8 @@ .sentry = .{ .path = "./pkg/sentry" }, .simdutf = .{ .path = "./pkg/simdutf" }, .utfcpp = .{ .path = "./pkg/utfcpp" }, + .wuffs = .{ .path = "./pkg/wuffs" }, .zlib = .{ .path = "./pkg/zlib" }, - .wuffs = .{ - .url = "https://github.com/google/wuffs/archive/refs/tags/v0.4.0-alpha.8.tar.gz", - .hash = "12200984439edc817fbcbbaff564020e5104a0d04a2d0f53080700827052de700462", - }, // Shader translation .glslang = .{ .path = "./pkg/glslang" }, diff --git a/pkg/wuffs/build.zig b/pkg/wuffs/build.zig new file mode 100644 index 000000000..967acf75c --- /dev/null +++ b/pkg/wuffs/build.zig @@ -0,0 +1,30 @@ +const std = @import("std"); + +pub fn build(b: *std.Build) void { + const target = b.standardTargetOptions(.{}); + const optimize = b.standardOptimizeOption(.{}); + + const wuffs = b.dependency("wuffs", .{}); + + const module = b.addModule("wuffs", .{ + .root_source_file = b.path("src/main.zig"), + .target = target, + .optimize = optimize, + .link_libc = true, + }); + + module.addIncludePath(wuffs.path("release/c")); + module.addCSourceFile( + .{ + .file = wuffs.path("release/c/wuffs-v0.4.c"), + .flags = f: { + const flags = @import("src/defs.zig").build; + var a: [flags.len][]const u8 = undefined; + inline for (flags, 0..) |flag, i| { + a[i] = "-D" ++ flag ++ "=1"; + } + break :f &a; + }, + }, + ); +} diff --git a/pkg/wuffs/build.zig.zon b/pkg/wuffs/build.zig.zon new file mode 100644 index 000000000..f37f6d8a0 --- /dev/null +++ b/pkg/wuffs/build.zig.zon @@ -0,0 +1,17 @@ +.{ + .name = "wuffs", + + .version = "0.0.0", + .dependencies = .{ + .wuffs = .{ + .url = "https://github.com/google/wuffs/archive/refs/tags/v0.4.0-alpha.8.tar.gz", + .hash = "12200984439edc817fbcbbaff564020e5104a0d04a2d0f53080700827052de700462", + }, + }, + + .paths = .{ + "build.zig", + "build.zig.zon", + "src", + }, +} diff --git a/src/wuffs/c.zig b/pkg/wuffs/src/c.zig similarity index 100% rename from src/wuffs/c.zig rename to pkg/wuffs/src/c.zig diff --git a/src/wuffs/defs.zig b/pkg/wuffs/src/defs.zig similarity index 100% rename from src/wuffs/defs.zig rename to pkg/wuffs/src/defs.zig diff --git a/src/wuffs/error.zig b/pkg/wuffs/src/error.zig similarity index 100% rename from src/wuffs/error.zig rename to pkg/wuffs/src/error.zig diff --git a/src/wuffs/main.zig b/pkg/wuffs/src/main.zig similarity index 100% rename from src/wuffs/main.zig rename to pkg/wuffs/src/main.zig diff --git a/src/wuffs/png.zig b/pkg/wuffs/src/png.zig similarity index 85% rename from src/wuffs/png.zig rename to pkg/wuffs/src/png.zig index e8cf7cc4f..c81118286 100644 --- a/src/wuffs/png.zig +++ b/pkg/wuffs/src/png.zig @@ -10,8 +10,6 @@ pub fn decode(alloc: std.mem.Allocator, data: []const u8) Error!struct { height: u32, data: []const u8, } { - log.info("data is {d} bytes", .{data.len}); - // Work around some weirdness in WUFFS/Zig, there are some structs that // are defined as "extern" by the Zig compiler which means that Zig won't // allocate them on the stack at compile time. WUFFS has functions for @@ -32,12 +30,12 @@ pub fn decode(alloc: std.mem.Allocator, data: []const u8) Error!struct { ); if (!c.wuffs_base__status__is_ok(&status)) { const e = c.wuffs_base__status__message(&status); - log.err("{s}", .{e}); + log.warn("{s}", .{e}); return error.WuffsError; } } - var source_buffer = std.mem.zeroes(c.wuffs_base__io_buffer); + var source_buffer: c.wuffs_base__io_buffer = undefined; source_buffer.data.ptr = @constCast(@ptrCast(data.ptr)); source_buffer.data.len = data.len; source_buffer.meta.wi = data.len; @@ -45,7 +43,7 @@ pub fn decode(alloc: std.mem.Allocator, data: []const u8) Error!struct { source_buffer.meta.pos = 0; source_buffer.meta.closed = true; - var image_config = std.mem.zeroes(c.wuffs_base__image_config); + var image_config: c.wuffs_base__image_config = undefined; { const status = c.wuffs_png__decoder__decode_image_config( decoder, @@ -54,7 +52,7 @@ pub fn decode(alloc: std.mem.Allocator, data: []const u8) Error!struct { ); if (!c.wuffs_base__status__is_ok(&status)) { const e = c.wuffs_base__status__message(&status); - log.err("{s}", .{e}); + log.warn("{s}", .{e}); return error.WuffsError; } } @@ -70,9 +68,10 @@ pub fn decode(alloc: std.mem.Allocator, data: []const u8) Error!struct { height, ); - const color = c.wuffs_base__color_u32_argb_premul; - - const destination = try alloc.alloc(u8, width * height * @sizeOf(color)); + const destination = try alloc.alloc( + u8, + width * height * @sizeOf(c.wuffs_base__color_u32_argb_premul), + ); errdefer alloc.free(destination); // temporary buffer for intermediate processing of image @@ -87,7 +86,7 @@ pub fn decode(alloc: std.mem.Allocator, data: []const u8) Error!struct { work_buffer.len, ); - var pixel_buffer = std.mem.zeroes(c.wuffs_base__pixel_buffer); + var pixel_buffer: c.wuffs_base__pixel_buffer = undefined; { const status = c.wuffs_base__pixel_buffer__set_from_slice( &pixel_buffer, @@ -96,12 +95,12 @@ pub fn decode(alloc: std.mem.Allocator, data: []const u8) Error!struct { ); if (!c.wuffs_base__status__is_ok(&status)) { const e = c.wuffs_base__status__message(&status); - log.err("{s}", .{e}); + log.warn("{s}", .{e}); return error.WuffsError; } } - var frame_config = std.mem.zeroes(c.wuffs_base__frame_config); + var frame_config: c.wuffs_base__frame_config = undefined; { const status = c.wuffs_png__decoder__decode_frame_config( decoder, @@ -110,7 +109,7 @@ pub fn decode(alloc: std.mem.Allocator, data: []const u8) Error!struct { ); if (!c.wuffs_base__status__is_ok(&status)) { const e = c.wuffs_base__status__message(&status); - log.err("{s}", .{e}); + log.warn("{s}", .{e}); return error.WuffsError; } } @@ -126,7 +125,7 @@ pub fn decode(alloc: std.mem.Allocator, data: []const u8) Error!struct { ); if (!c.wuffs_base__status__is_ok(&status)) { const e = c.wuffs_base__status__message(&status); - log.err("{s}", .{e}); + log.warn("{s}", .{e}); return error.WuffsError; } } diff --git a/src/wuffs/swizzle.zig b/pkg/wuffs/src/swizzle.zig similarity index 98% rename from src/wuffs/swizzle.zig rename to pkg/wuffs/src/swizzle.zig index 6f2c4de2d..8a691c19d 100644 --- a/src/wuffs/swizzle.zig +++ b/pkg/wuffs/src/swizzle.zig @@ -62,6 +62,7 @@ fn swizzle( std.debug.assert(c.wuffs_base__pixel_format__bits_per_pixel(&src_fmt) % 8 == 0); const src_size = c.wuffs_base__pixel_format__bits_per_pixel(&src_fmt) / 8; + std.debug.assert(src.len % src_size == 0); const dst = try alloc.alloc(u8, src.len * dst_size / src_size); @@ -85,7 +86,7 @@ fn swizzle( ); if (!c.wuffs_base__status__is_ok(&status)) { const e = c.wuffs_base__status__message(&status); - log.err("{s}", .{e}); + log.warn("{s}", .{e}); return error.WuffsError; } } diff --git a/src/renderer/opengl/image.zig b/src/renderer/opengl/image.zig index 414da81bd..85f59f1f3 100644 --- a/src/renderer/opengl/image.zig +++ b/src/renderer/opengl/image.zig @@ -2,7 +2,7 @@ const std = @import("std"); const Allocator = std.mem.Allocator; const assert = std.debug.assert; const gl = @import("opengl"); -const wuffs = @import("../../wuffs/main.zig"); +const wuffs = @import("wuffs"); /// Represents a single image placement on the grid. A placement is a /// request to render an instance of an image. diff --git a/src/terminal/kitty/graphics_image.zig b/src/terminal/kitty/graphics_image.zig index 3af3874c9..d1a20f160 100644 --- a/src/terminal/kitty/graphics_image.zig +++ b/src/terminal/kitty/graphics_image.zig @@ -10,7 +10,7 @@ const command = @import("graphics_command.zig"); const point = @import("../point.zig"); const PageList = @import("../PageList.zig"); const internal_os = @import("../../os/main.zig"); -const wuffs = @import("../../wuffs/main.zig"); +const wuffs = @import("wuffs"); const log = std.log.scoped(.kitty_gfx);