From 8bd16c32f3bbe724321127fcaaf798a1928cf0fe Mon Sep 17 00:00:00 2001 From: Jeroen van Rijn Date: Sat, 30 Apr 2022 21:00:32 +0200 Subject: [PATCH] [pbm] Fixes. --- core/image/netpbm/helpers.odin | 5 +- core/image/netpbm/netpbm.odin | 45 ++++++++-------- tests/core/image/test_core_image.odin | 76 ++++++++++++++------------- 3 files changed, 64 insertions(+), 62 deletions(-) diff --git a/core/image/netpbm/helpers.odin b/core/image/netpbm/helpers.odin index 8c5cdd622..2fbd32ecc 100644 --- a/core/image/netpbm/helpers.odin +++ b/core/image/netpbm/helpers.odin @@ -6,15 +6,16 @@ import "core:image" destroy :: proc(img: ^image.Image) -> bool { if img == nil do return false + defer free(img) + bytes.buffer_destroy(&img.pixels) + //! TEMP CAST info, ok := img.metadata.(^image.Netpbm_Info) if !ok do return false - bytes.buffer_destroy(&img.pixels) header_destroy(&info.header) free(info) img.metadata = nil - free(img) return true } diff --git a/core/image/netpbm/netpbm.odin b/core/image/netpbm/netpbm.odin index cccf7e865..768c06110 100644 --- a/core/image/netpbm/netpbm.odin +++ b/core/image/netpbm/netpbm.odin @@ -46,13 +46,13 @@ load_from_file :: proc(filename: string, allocator := context.allocator) -> (img load_from_buffer :: proc(data: []byte, allocator := context.allocator) -> (img: ^Image, err: Error) { context.allocator = allocator + img = new(Image) + header: Header; defer header_destroy(&header) header_size: int header, header_size = parse_header(data) or_return img_data := data[header_size:] - - img = new(Image) decode_image(img, header, img_data) or_return info := new(Info) @@ -62,8 +62,7 @@ load_from_buffer :: proc(data: []byte, allocator := context.allocator) -> (img: } img.metadata = info - err = Format_Error.None - return + return img, nil } save :: proc { @@ -140,8 +139,14 @@ save_to_buffer :: proc(img: ^Image, custom_info: Info = {}, allocator := context fmt.sbprintf(&data, "%i\n", header.maxval) } } else if header.format in PAM { - fmt.sbprintf(&data, "WIDTH %i\nHEIGHT %i\nMAXVAL %i\nDEPTH %i\nTUPLTYPE %s\nENDHDR\n", - img.width, img.height, header.maxval, img.channels, header.tupltype) + if len(header.tupltype) > 0 { + fmt.sbprintf(&data, "WIDTH %i\nHEIGHT %i\nMAXVAL %i\nDEPTH %i\nTUPLTYPE %s\nENDHDR\n", + img.width, img.height, header.maxval, img.channels, header.tupltype) + } else { + fmt.sbprintf(&data, "WIDTH %i\nHEIGHT %i\nMAXVAL %i\nDEPTH %i\nENDHDR\n", + img.width, img.height, header.maxval, img.channels) + } + } else if header.format in PFM { scale := -header.scale if header.little_endian else header.scale fmt.sbprintf(&data, "%i %i\n%f\n", img.width, img.height, scale) @@ -369,10 +374,12 @@ _parse_header_pnm :: proc(data: []byte) -> (header: Header, length: int, err: Er if header.width < 1 \ || header.height < 1 \ || header.maxval < 1 || header.maxval > int(max(u16)) { - err = Format_Error.Invalid_Header_Value + fmt.printf("[pnm] Header: {{width = %v, height = %v, maxval: %v}}\n", header.width, header.height, header.maxval) + err = .Invalid_Header_Value return } + length -= 1 err = Format_Error.None return } @@ -427,7 +434,7 @@ _parse_header_pam :: proc(data: []byte, allocator := context.allocator) -> (head case "TUPLTYPE": if len(value) == 0 { - err = Format_Error.Invalid_Header_Value + err = .Invalid_Header_Value return } @@ -462,6 +469,7 @@ _parse_header_pam :: proc(data: []byte, allocator := context.allocator) -> (head || header.height < 1 \ || header.maxval < 1 \ || header.maxval > int(max(u16)) { + fmt.printf("[pam] Header: {{width = %v, height = %v, maxval: %v}}\n", header.width, header.height, header.maxval) err = Format_Error.Invalid_Header_Value return } @@ -540,7 +548,8 @@ _parse_header_pfm :: proc(data: []byte) -> (header: Header, length: int, err: Er if header.width < 1 \ || header.height < 1 \ || header.scale == 0.0 { - err = Format_Error.Invalid_Header_Value + fmt.printf("[pfm] Header: {{width = %v, height = %v, scale: %v}}\n", header.width, header.height, header.scale) + err = .Invalid_Header_Value return } @@ -559,23 +568,13 @@ decode_image :: proc(img: ^Image, header: Header, data: []byte, allocator := con buffer_size := image.compute_buffer_size(img.width, img.height, img.channels, img.depth) - when false { // we can check data size for binary formats if header.format in BINARY { - if header.format == .P4 { - p4_size := (img.width / 8 + 1) * img.height - if len(data) < p4_size { - err = Format_Error.Buffer_Too_Small - return - } - } else { - if len(data) < buffer_size { - err = Format_Error.Buffer_Too_Small - return - } + if len(data) < buffer_size { + fmt.printf("len(data): %v, buffer size: %v\n", len(data), buffer_size) + return .Buffer_Too_Small } } - } // for ASCII and P4, we use length for the termination condition, so start at 0 // BINARY will be a simple memcopy so the buffer length should also be initialised @@ -605,7 +604,7 @@ decode_image :: proc(img: ^Image, header: Header, data: []byte, allocator := con // Simple binary case .P5, .P6, .P7, .Pf, .PF: - mem.copy(raw_data(img.pixels.buf), raw_data(data), buffer_size) + copy(img.pixels.buf[:], data[:]) // convert to native endianness if header.format in PFM { diff --git a/tests/core/image/test_core_image.odin b/tests/core/image/test_core_image.odin index 93baa76ec..f1f1e1244 100644 --- a/tests/core/image/test_core_image.odin +++ b/tests/core/image/test_core_image.odin @@ -1530,54 +1530,56 @@ run_png_suite :: proc(t: ^testing.T, suite: []PNG_Test) -> (subtotal: int) { } } - // Roundtrip through PBM to test the PBM encoders and decoders - prefer binary - pbm_buf, pbm_save_err := pbm.save_to_buffer(img) - defer delete(pbm_buf) + { + // Roundtrip through PBM to test the PBM encoders and decoders - prefer binary + pbm_buf, pbm_save_err := pbm.save_to_buffer(img) + defer delete(pbm_buf) - filename := fmt.tprintf("%v-%v.ppm", file.file, count) - pbm.save_to_file(filename, img) + error = fmt.tprintf("%v test %v PBM save failed with %v.", file.file, count, pbm_save_err) + expect(t, pbm_save_err == nil, error) - error = fmt.tprintf("%v test %v PBM save failed with %v.", file.file, count, pbm_save_err) - expect(t, pbm_save_err == nil, error) + if pbm_save_err == nil { + // Try to load it again. + pbm_img, pbm_load_err := pbm.load(pbm_buf) + defer pbm.destroy(pbm_img) - if pbm_save_err == nil { - // Try to load it again. - pbm_img, pbm_load_err := pbm.load(pbm_buf) - defer pbm.destroy(pbm_img) + error = fmt.tprintf("%v test %v PBM load failed with %v.", file.file, count, pbm_load_err) + expect(t, pbm_load_err == nil, error) - if pbm_load_err == nil { - fmt.printf("%v test %v PBM load worked with %v.\n", file.file, count, pbm_load_err) + if pbm_load_err == nil { + pbm_hash := hash.crc32(pbm_img.pixels.buf[:]) - pbm_hash := hash.crc32(pbm_img.pixels.buf[:]) - if pbm_hash == png_hash { - fmt.printf("\t%v test %v PBM load hash %08x matched PNG's\n", file.file, count, png_hash) - } else { - if img.width != pbm_img.width || img.height != pbm_img.height || img.channels != pbm_img.channels || img.depth != pbm_img.depth { - fmt.printf("\tHash failed. IMG: %v, %v, %v, %v PBM: %v, %v, %v, %v\n", img.width, img.height, img.channels, img.depth, pbm_img.width, pbm_img.height, pbm_img.channels, pbm_img.depth) - } else if len(img.pixels.buf) != len(pbm_img.pixels.buf) { - fmt.printf("\tLengths differ. IMG: %v PBM: %v\n", len(img.pixels.buf), len(pbm_img.pixels.buf)) - } else if file.file[:3] == "bas" { - for v, i in img.pixels.buf { - if v != pbm_img.pixels.buf[i] { - fmt.printf("\tChannels: %v, Depth: %v, Pixel %v differs. PNG: %v, PBM: %v\n", img.channels, img.depth, i, img.pixels.buf[i:][:4], pbm_img.pixels.buf[i:][:4]) - break - } - } - } - // error = fmt.tprintf("%v test %v PBM load hash is %08x, expected it match PNG's %08x with %v.", file.file, count, pbm_hash, png_hash, test.options) - // expect(t, pbm_hash == png_hash, error) + error = fmt.tprintf("%v test %v PBM load hash is %08x, expected it match PNG's %08x with %v.", file.file, count, pbm_hash, png_hash, test.options) + expect(t, pbm_hash == png_hash, error) } - } else { - // error = fmt.tprintf("%v test %v PBM load failed with %v.", file.file, count, pbm_load_err) - // expect(t, pbm_load_err == nil, error) } } - // Roundtrip through PBM to test the PBM encoders and decoders - prefer ASCII - // pbm_info, pbm_format_selected = pbm.autoselect_pbm_format_from_image(img, false) - // fmt.printf("Autoselect PBM: %v (%v)\n", pbm_info, pbm_format_selected) + { + // Roundtrip through PBM to test the PBM encoders and decoders - prefer ASCII + pbm_info, pbm_format_selected := pbm.autoselect_pbm_format_from_image(img, false) + pbm_buf, pbm_save_err := pbm.save_to_buffer(img, pbm_info) + defer delete(pbm_buf) + error = fmt.tprintf("%v test %v PBM save failed with %v.", file.file, count, pbm_save_err) + expect(t, pbm_save_err == nil, error) + if pbm_save_err == nil { + // Try to load it again. + pbm_img, pbm_load_err := pbm.load(pbm_buf) + defer pbm.destroy(pbm_img) + + error = fmt.tprintf("%v test %v PBM load failed with %v.", file.file, count, pbm_load_err) + expect(t, pbm_load_err == nil, error) + + if pbm_load_err == nil { + pbm_hash := hash.crc32(pbm_img.pixels.buf[:]) + + error = fmt.tprintf("%v test %v PBM load hash is %08x, expected it match PNG's %08x with %v.", file.file, count, pbm_hash, png_hash, test.options) + expect(t, pbm_hash == png_hash, error) + } + } + } } if .return_metadata in test.options {