Browse Source

[pbm] Fixes.

Jeroen van Rijn 3 years ago
parent
commit
8bd16c32f3
3 changed files with 69 additions and 67 deletions
  1. 3 2
      core/image/netpbm/helpers.odin
  2. 22 23
      core/image/netpbm/netpbm.odin
  3. 44 42
      tests/core/image/test_core_image.odin

+ 3 - 2
core/image/netpbm/helpers.odin

@@ -6,15 +6,16 @@ import "core:image"
 destroy :: proc(img: ^image.Image) -> bool {
 destroy :: proc(img: ^image.Image) -> bool {
 	if img == nil do return false
 	if img == nil do return false
 
 
+	defer free(img)
+	bytes.buffer_destroy(&img.pixels)
+
 	//! TEMP CAST
 	//! TEMP CAST
 	info, ok := img.metadata.(^image.Netpbm_Info)
 	info, ok := img.metadata.(^image.Netpbm_Info)
 	if !ok do return false
 	if !ok do return false
 
 
-	bytes.buffer_destroy(&img.pixels)
 	header_destroy(&info.header)
 	header_destroy(&info.header)
 	free(info)
 	free(info)
 	img.metadata = nil
 	img.metadata = nil
-	free(img)
 
 
 	return true
 	return true
 }
 }

+ 22 - 23
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) {
 load_from_buffer :: proc(data: []byte, allocator := context.allocator) -> (img: ^Image, err: Error) {
 	context.allocator = allocator
 	context.allocator = allocator
 
 
+	img = new(Image)
+
 	header: Header; defer header_destroy(&header)
 	header: Header; defer header_destroy(&header)
 	header_size: int
 	header_size: int
 	header, header_size = parse_header(data) or_return
 	header, header_size = parse_header(data) or_return
 
 
 	img_data := data[header_size:]
 	img_data := data[header_size:]
-
-	img = new(Image)
 	decode_image(img, header, img_data) or_return
 	decode_image(img, header, img_data) or_return
 
 
 	info := new(Info)
 	info := new(Info)
@@ -62,8 +62,7 @@ load_from_buffer :: proc(data: []byte, allocator := context.allocator) -> (img:
 	}
 	}
 	img.metadata = info
 	img.metadata = info
 
 
-	err = Format_Error.None
-	return
+	return img, nil
 }
 }
 
 
 save :: proc {
 save :: proc {
@@ -140,8 +139,14 @@ save_to_buffer :: proc(img: ^Image, custom_info: Info = {}, allocator := context
 			fmt.sbprintf(&data, "%i\n", header.maxval)
 			fmt.sbprintf(&data, "%i\n", header.maxval)
 		}
 		}
 	} else if header.format in PAM {
 	} 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 {
 	} else if header.format in PFM {
 		scale := -header.scale if header.little_endian else header.scale
 		scale := -header.scale if header.little_endian else header.scale
 		fmt.sbprintf(&data, "%i %i\n%f\n", img.width, img.height, 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 \
 	if header.width < 1 \
 	|| header.height < 1 \
 	|| header.height < 1 \
 	|| header.maxval < 1 || header.maxval > int(max(u16)) {
 	|| 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
 		return
 	}
 	}
 
 
+	length -= 1
 	err = Format_Error.None
 	err = Format_Error.None
 	return
 	return
 }
 }
@@ -427,7 +434,7 @@ _parse_header_pam :: proc(data: []byte, allocator := context.allocator) -> (head
 
 
 		case "TUPLTYPE":
 		case "TUPLTYPE":
 			if len(value) == 0 {
 			if len(value) == 0 {
-				err = Format_Error.Invalid_Header_Value
+				err = .Invalid_Header_Value
 				return
 				return
 			}
 			}
 
 
@@ -462,6 +469,7 @@ _parse_header_pam :: proc(data: []byte, allocator := context.allocator) -> (head
 	|| header.height < 1 \
 	|| header.height < 1 \
 	|| header.maxval < 1 \
 	|| header.maxval < 1 \
 	|| header.maxval > int(max(u16)) {
 	|| 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
 		err = Format_Error.Invalid_Header_Value
 		return
 		return
 	}
 	}
@@ -540,7 +548,8 @@ _parse_header_pfm :: proc(data: []byte) -> (header: Header, length: int, err: Er
 	if header.width < 1 \
 	if header.width < 1 \
 	|| header.height < 1 \
 	|| header.height < 1 \
 	|| header.scale == 0.0 {
 	|| 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
 		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)
 	buffer_size := image.compute_buffer_size(img.width, img.height, img.channels, img.depth)
 
 
-	when false {
 	// we can check data size for binary formats
 	// we can check data size for binary formats
 	if header.format in BINARY {
 	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
 	// 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
 	// 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
 	// Simple binary
 	case .P5, .P6, .P7, .Pf, .PF:
 	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
 		// convert to native endianness
 		if header.format in PFM {
 		if header.format in PFM {

+ 44 - 42
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)
-
-					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)
-
-					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_load_err == nil {
-							fmt.printf("%v test %v PBM load worked with %v.\n", file.file, count, pbm_load_err)
-
-							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)
+					{
+						// 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)
+
+						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)
 							}
 							}
-						} 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 {
 				if .return_metadata in test.options {