freetype: Avoid dereferencing null bitmap buffer. #336

Merged

jamii merged 1 commit from main into main

2022-06-10 19:55:19 +00:00

jamii commented

2022-06-08 20:05:11 +00:00

(Migrated from github.com)

For some characters (eg \r) the returned glyph will have .{.rows = 0, .width=0, .pitch = 0, .buffer = null}. In zig null[0..0] causes a panic rather than returning an empty slice.

By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.

For some characters (eg \r) the returned glyph will have .{.rows = 0, .width=0, .pitch = 0, .buffer = null}. In zig null[0..0] causes a panic rather than returning an empty slice. - [x] By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.

alichraghi commented

2022-06-08 21:19:07 +00:00

(Migrated from github.com)

thanks @jamii
i think it would be better to return an optional (?[]const u8) instead of empty buffer
this also requires a minor change in single-glyph example

thanks @jamii i think it would be better to return an optional (`?[]const u8`) instead of empty buffer this also requires a minor change in *single-glyph* example

👍 2

emidoots commented

2022-06-10 14:08:23 +00:00

(Migrated from github.com)

Hey @jamii just wanted to confirm you're planning to update this PR?

👍 1

jamii commented

2022-06-10 15:31:31 +00:00

(Migrated from github.com)

Sure. ?[]const u8 requires an extra branch in any rendering code though, compared to an empty buffer.

                const buffer = bitmap.buffer();
                const bitmap_copy = allocator.alloc(u.Color, bitmap_width * bitmap_height) catch u.oom();
                var x: usize = 0;
                while (x < bitmap_width) : (x += 1) {
                    var y: usize = 0;
                    while (y < bitmap_height) : (y += 1) {
                        bitmap_copy[(y * bitmap_width) + x] = .{
                            .r = 255,
                            .g = 255,
                            .b = 255,
                            .a = buffer[(y * @intCast(usize, bitmap_pitch)) + x],
                        };
                    }
                }

                const bitmap_copy = allocator.alloc(u.Color, bitmap_width * bitmap_height) catch u.oom();
                if (bitmap.buffer()) |buffer| {
                    var x: usize = 0;
                    while (x < bitmap_width) : (x += 1) {
                        var y: usize = 0;
                        while (y < bitmap_height) : (y += 1) {
                            bitmap_copy[(y * bitmap_width) + x] = .{
                                .r = 255,
                                .g = 255,
                                .b = 255,
                                .a = buffer[(y * @intCast(usize, bitmap_pitch)) + x],
                            };
                        }
                    }
                }

Sure. `?[]const u8` requires an extra branch in any rendering code though, compared to an empty buffer. ``` zig const buffer = bitmap.buffer(); const bitmap_copy = allocator.alloc(u.Color, bitmap_width * bitmap_height) catch u.oom(); var x: usize = 0; while (x < bitmap_width) : (x += 1) { var y: usize = 0; while (y < bitmap_height) : (y += 1) { bitmap_copy[(y * bitmap_width) + x] = .{ .r = 255, .g = 255, .b = 255, .a = buffer[(y * @intCast(usize, bitmap_pitch)) + x], }; } } ``` vs ``` zig const bitmap_copy = allocator.alloc(u.Color, bitmap_width * bitmap_height) catch u.oom(); if (bitmap.buffer()) |buffer| { var x: usize = 0; while (x < bitmap_width) : (x += 1) { var y: usize = 0; while (y < bitmap_height) : (y += 1) { bitmap_copy[(y * bitmap_width) + x] = .{ .r = 255, .g = 255, .b = 255, .a = buffer[(y * @intCast(usize, bitmap_pitch)) + x], }; } } } ```

alichraghi commented

2022-06-10 15:44:55 +00:00

(Migrated from github.com)

@jamii this is a binding so it must be user choice to unwrap optional values (no self though code as much as possible)

👍 1

alichraghi (Migrated from github.com) approved these changes

2022-06-10 17:12:11 +00:00

emidoots (Migrated from github.com) approved these changes

2022-06-10 19:54:00 +00:00

emidoots (Migrated from github.com) left a comment

Thanks a ton!