From 1921e6db90eebb12b55f25f607296cc6ad800415 Mon Sep 17 00:00:00 2001 From: John Bowler Date: Mon, 16 May 2011 20:57:54 -0500 Subject: [PATCH] [devel] Reversed earlier change of transformation order; move png_expand_16 back where it was before libpng-1.5.3beta07. The change doesn't work because it requires 16 bit gamma tables when the code only generates 8 bit ones. This fails silently; the libpng code just doesn't do any gamma correction. Moving the tests back leaves the old, inaccurate, 8 bit gamma calculations, but these are clearly better than none! --- ANNOUNCE | 5 +++++ CHANGES | 5 +++++ pngrtran.c | 37 +++++++++---------------------------- pngvalid.c | 51 ++++++++++++++++++++++++++++++++++++++++----------- 4 files changed, 59 insertions(+), 39 deletions(-) diff --git a/ANNOUNCE b/ANNOUNCE index bd298d10b..02ed1ea01 100644 --- a/ANNOUNCE +++ b/ANNOUNCE @@ -128,6 +128,11 @@ Version 1.5.3beta08 [May 16, 2011] Minor cleanup and some extra checking in pngrutil.c and pngrtran.c Version 1.5.3beta09 [May 17, 2011] + Reversed earlier 1.5.3 change of transformation order; move png_expand_16 back. + The change doesn't work because it requires 16 bit gamma tables when the code + only generates 8 bit ones. This fails silently; the libpng code just doesn't + do any gamma correction. Moving the tests back leaves the old, inaccurate, 8 + bit gamma calculations, but these are clearly better than none! Send comments/corrections/commendations to png-mng-implement at lists.sf.net: (subscription required; visit diff --git a/CHANGES b/CHANGES index e6d36fc14..b99b1de0a 100644 --- a/CHANGES +++ b/CHANGES @@ -3389,6 +3389,11 @@ Version 1.5.3beta08 [May 16, 2011] Minor cleanup and some extra checking in pngrutil.c and pngrtran.c Version 1.5.3beta09 [May 17, 2011] + Reversed earlier 1.5.3 change of transformation order; move png_expand_16 back. + The change doesn't work because it requires 16 bit gamma tables when the code + only generates 8 bit ones. This fails silently; the libpng code just doesn't + do any gamma correction. Moving the tests back leaves the old, inaccurate, 8 + bit gamma calculations, but these are clearly better than none! Send comments/corrections/commendations to png-mng-implement at lists.sf.net (subscription required; visit diff --git a/pngrtran.c b/pngrtran.c index 5ef85aa95..03eada22c 100644 --- a/pngrtran.c +++ b/pngrtran.c @@ -1318,7 +1318,7 @@ png_init_read_transformations(png_structp png_ptr) * 8) PNG_ENCODE_ALPHA * 9) PNG_16_TO_8 (strip16) * 10) PNG_QUANTIZE (converts to palette) - * 11) PNG_EXPAND_16 [NOTE: temporarily moved to (3) for accuracy!] + * 11) PNG_EXPAND_16 * 12) PNG_GRAY_TO_RGB iff PNG_BACKGROUND_IS_GRAY * 13) PNG_INVERT_MONO * 14) PNG_SHIFT @@ -1808,14 +1808,6 @@ png_read_transform_info(png_structp png_ptr, png_infop info_ptr) } #endif -#ifdef PNG_READ_EXPAND_16_SUPPORTED - if (png_ptr->transformations & PNG_EXPAND_16 && info_ptr->bit_depth == 8 && - info_ptr->color_type != PNG_COLOR_TYPE_PALETTE) - { - info_ptr->bit_depth = 16; - } -#endif - #if defined(PNG_READ_BACKGROUND_SUPPORTED) ||\ defined(PNG_READ_ALPHA_MODE_SUPPORTED) /* The following is almost certainly wrong unless the background value is in @@ -1870,6 +1862,14 @@ png_read_transform_info(png_structp png_ptr, png_infop info_ptr) } #endif +#ifdef PNG_READ_EXPAND_16_SUPPORTED + if (png_ptr->transformations & PNG_EXPAND_16 && info_ptr->bit_depth == 8 && + info_ptr->color_type != PNG_COLOR_TYPE_PALETTE) + { + info_ptr->bit_depth = 16; + } +#endif + #ifdef PNG_READ_PACK_SUPPORTED if ((png_ptr->transformations & PNG_PACK) && (info_ptr->bit_depth < 8)) info_ptr->bit_depth = 8; @@ -1995,19 +1995,6 @@ png_do_read_transformations(png_structp png_ptr) } #endif - /* TODO: Delay the 'expand 16' step until later for efficiency, so that the - * intermediate steps work with 8 bit data. - */ -#ifdef PNG_READ_EXPAND_16_SUPPORTED - /* Do the expansion now, after all the arithmetic has been done. Notice - * that previous transformations can handle the PNG_EXPAND_16 flag if this - * is efficient (particularly true in the case of gamma correction, where - * better accuracy results faster!) - */ - if (png_ptr->transformations & PNG_EXPAND_16) - png_do_expand_16(&png_ptr->row_info, png_ptr->row_buf + 1); -#endif - #ifdef PNG_READ_STRIP_ALPHA_SUPPORTED if ((png_ptr->transformations & PNG_STRIP_ALPHA) && !(png_ptr->transformations & PNG_COMPOSE) && @@ -2127,11 +2114,6 @@ png_do_read_transformations(png_structp png_ptr) } #endif /* PNG_READ_QUANTIZE_SUPPORTED */ -#if 0 - /* This is where this code *should* be for efficiency, but that requires all - * the inaccurate calculations above to output 16 bit values if expand_16 is - * set! - */ #ifdef PNG_READ_EXPAND_16_SUPPORTED /* Do the expansion now, after all the arithmetic has been done. Notice * that previous transformations can handle the PNG_EXPAND_16 flag if this @@ -2141,7 +2123,6 @@ png_do_read_transformations(png_structp png_ptr) if (png_ptr->transformations & PNG_EXPAND_16) png_do_expand_16(&png_ptr->row_info, png_ptr->row_buf + 1); #endif -#endif /*commented out*/ #ifdef PNG_READ_GRAY_TO_RGB_SUPPORTED /*NOTE: moved here in 1.5.3 (from much later in this list.) */ diff --git a/pngvalid.c b/pngvalid.c index a3ddf18f8..3a548bcb3 100644 --- a/pngvalid.c +++ b/pngvalid.c @@ -1528,6 +1528,7 @@ typedef struct png_modifier double error_gray_16; double error_color_8; double error_color_16; + double error_indexed; /* Flags: */ /* Whether or not to interlace. */ @@ -1593,6 +1594,7 @@ modifier_init(png_modifier *pm) pm->maxout16 = pm->maxpc16 = pm->maxabs16 = pm->maxcalc16 = 0; pm->error_gray_2 = pm->error_gray_4 = pm->error_gray_8 = 0; pm->error_gray_16 = pm->error_color_8 = pm->error_color_16 = 0; + pm->error_indexed = 0; pm->interlace_type = PNG_INTERLACE_NONE; pm->test_standard = 0; pm->test_size = 0; @@ -6814,6 +6816,12 @@ gamma_test(png_modifier *pmIn, PNG_CONST png_byte colour_typeIn, png_error(pp, "bad bit depth (internal: 2)"); } } + + else if (d.this.colour_type == 3) + { + if (d.maxerrout > d.pm->error_indexed) + d.pm->error_indexed = d.maxerrout; + } } Catch(fault) @@ -7184,8 +7192,9 @@ perform_gamma_composition_tests(png_modifier *pm, int do_background, static void init_gamma_errors(png_modifier *pm) { - pm->error_gray_2 = pm->error_gray_4 = pm->error_gray_8 = pm->error_color_8 = - 0; + pm->error_gray_2 = pm->error_gray_4 = pm->error_gray_8 = 0; + pm->error_color_8 = 0; + pm->error_indexed = 0; pm->error_gray_16 = pm->error_color_16 = 0; } @@ -7201,6 +7210,7 @@ summarize_gamma_errors(png_modifier *pm, png_const_charp who, int low_bit_depth) printf(" 4 bit gray: %.5f\n", pm->error_gray_4); printf(" 8 bit gray: %.5f\n", pm->error_gray_8); printf(" 8 bit color: %.5f\n", pm->error_color_8); + printf(" indexed: %.5f\n", pm->error_indexed); } #ifdef DO_16BIT @@ -7655,6 +7665,7 @@ perform_interlace_macro_validation(void) int main(int argc, PNG_CONST char **argv) { volatile int summary = 1; /* Print the error summary at the end */ + volatile int memstats = 0; /* Print memory statistics at the end */ /* Create the given output file on success: */ PNG_CONST char *volatile touch = NULL; @@ -7667,6 +7678,10 @@ int main(int argc, PNG_CONST char **argv) static double gammas[]={2.2, 1.0, 2.2/1.45, 1.8, 1.5, 2.4, 2.5, 2.62, 2.9}; + /* This records the command and arguments: */ + size_t cp = 0; + char command[1024]; + png_modifier pm; context(&pm.this, fault); @@ -7679,6 +7694,9 @@ int main(int argc, PNG_CONST char **argv) */ store_ensure_image(&pm.this, NULL, 2, TRANSFORM_ROWMAX, TRANSFORM_HEIGHTMAX); + /* Don't give argv[0], it's normally some horrible libtool string: */ + cp = safecat(command, sizeof command, cp, "pngvalid"); + /* Default to error on warning: */ pm.this.treat_warnings_as_errors = 1; @@ -7716,7 +7734,11 @@ int main(int argc, PNG_CONST char **argv) /* Now parse the command line options. */ while (--argc >= 1) { - if (strcmp(*++argv, "-v") == 0) + /* Record each argument for posterity: */ + cp = safecat(command, sizeof command, cp, " "); + cp = safecat(command, sizeof command, cp, *++argv); + + if (strcmp(*argv, "-v") == 0) pm.this.verbose = 1; else if (strcmp(*argv, "-l") == 0) @@ -7730,7 +7752,10 @@ int main(int argc, PNG_CONST char **argv) else if (strcmp(*argv, "--speed") == 0) pm.this.speed = 1, pm.ngammas = (sizeof gammas)/(sizeof gammas[0]), - pm.test_standard = 0; + pm.test_standard = 0, summary = 0; + + else if (strcmp(*argv, "--memory") == 0) + memstats = 1; else if (strcmp(*argv, "--size") == 0) pm.test_size = 1; @@ -7962,7 +7987,7 @@ int main(int argc, PNG_CONST char **argv) #ifdef PNG_READ_GAMMA_SUPPORTED if (pm.ngammas > 0) - perform_gamma_test(&pm, summary && !pm.this.speed); + perform_gamma_test(&pm, summary); #endif } @@ -7979,18 +8004,22 @@ int main(int argc, PNG_CONST char **argv) exit(1); } - if (summary && !pm.this.speed) + if (summary) { - printf("Results using %s point arithmetic %s\n", + printf("%s: %s: %s point arithmetic, %d errors, %d warnings\n", + (pm.this.nerrors || (pm.this.treat_warnings_as_errors && + pm.this.nwarnings)) ? "FAIL" : "PASS", + command, #if defined(PNG_FLOATING_ARITHMETIC_SUPPORTED) || PNG_LIBPNG_VER < 10500 "floating", #else "fixed", #endif - (pm.this.nerrors || (pm.this.treat_warnings_as_errors && - pm.this.nwarnings)) ? "(errors)" : (pm.this.nwarnings ? - "(warnings)" : "(no errors or warnings)") - ); + pm.this.nerrors, pm.this.nwarnings); + } + + if (memstats) + { printf("Allocated memory statistics (in bytes):\n" "\tread %lu maximum single, %lu peak, %lu total\n" "\twrite %lu maximum single, %lu peak, %lu total\n",