png_rgb_to_gray optimization fixes

This fixes the code that optimizes RGB to Gray transformations that only
selected on channel and clarifies the handling of max_depth in the transform
code.  It eliminates some UNTESTED cases and removes the write 'invert alpha'
UNTESTED macro because, while there are no test cases for it, code review
some months after writing it suggests it is ok.

Signed-off-by: John Bowler <jbowler@acm.org>

contrib/libtests/pngvalid.c

@@ -10960,6 +10960,11 @@ static const color_encoding test_encodings[] =
 /*red:  */ { 0.716500716779386, 0.258728243040113, 0.000000000000000 },
 /*green:*/ { 0.101020574397477, 0.724682314948566, 0.051211818965388 },
 /*blue: */ { 0.146774385252705, 0.016589442011321, 0.773892783545073} },
+/* Fake encoding which selects just the green channel */
+/*gamma:*/ { 1.45/2.2, /* the 'Mac' gamma */
+/*red:  */ { 0.716500716779386, 0.000000000000000, 0.000000000000000 },
+/*green:*/ { 0.101020574397477, 1.000000000000000, 0.051211818965388 },
+/*blue: */ { 0.146774385252705, 0.000000000000000, 0.773892783545073} },
 };
 
 /* signal handler

pngpriv.h

@@ -1193,10 +1193,10 @@ PNG_INTERNAL_FUNCTION(void,png_init_transform_control,(
 
 #ifdef PNG_READ_TRANSFORMS_SUPPORTED
 PNG_INTERNAL_FUNCTION(unsigned int,png_run_this_transform_list_forwards,
-   (png_transform_controlp tc, png_transformp *start, png_transformp *end),
+   (png_transform_controlp tc, png_transformp *start, png_transformp end),
    PNG_EMPTY);
    /* Used by the transform cache code to run a sub-list, from *start to the
-    * transform containing *end.
+    * transform end.
     */
 #endif /* READ_TRANSFORMS */
 

pngrtran.c

@@ -6048,7 +6048,7 @@ update_palette(png_structp png_ptr, png_cache_paramsp cp,
       cp->tend.dp = cache.b8;
 
       check_depth =
-         png_run_this_transform_list_forwards(&cp->tend, cp->start, cp->end);
+         png_run_this_transform_list_forwards(&cp->tend, cp->start, *cp->end);
 
       /* If we get here these two things must be true or there are been some
        * buggy difference of opinion between the INIT code and the actual run:
@@ -6308,7 +6308,7 @@ make_cache(png_structp png_ptr, png_cache_paramsp cp, unsigned int max_depth)
 
    {
       unsigned int check_depth =
-         png_run_this_transform_list_forwards(&cp->tend, cp->start, cp->end);
+         png_run_this_transform_list_forwards(&cp->tend, cp->start, *cp->end);
 
       /* This must not change: */
       affirm(PNG_TC_PIXEL_DEPTH(cp->tend) == opd && check_depth == max_depth);
@@ -6590,7 +6590,7 @@ png_read_init_transform_mech(png_structp png_ptr, png_transform_controlp tc)
     */
 {
    png_transformp *list = &png_ptr->transform_list;
-   unsigned int max_depth = 0U;
+   unsigned int max_depth;
    png_cache_params cp;
 
    /* PNG color-mapped data must be handled here so that the palette is updated
@@ -6610,6 +6610,7 @@ png_read_init_transform_mech(png_structp png_ptr, png_transform_controlp tc)
    cp.end = cp.start = list;
    cp.tend = cp.tstart = *tc;
    init_caching(png_ptr, &cp);
+   max_depth = PNG_TC_PIXEL_DEPTH(cp.tend);
 
    while (*cp.end != NULL)
    {
@@ -6660,7 +6661,9 @@ png_read_init_transform_mech(png_structp png_ptr, png_transform_controlp tc)
     */
    if (cp.tend.caching)
    {
+      png_transformp tr = *cp.end;
       handle_cache(png_ptr, &cp, max_depth);
+      affirm(tr == *cp.end);
       max_depth = PNG_TC_PIXEL_DEPTH(cp.tend);
    }
 

pngtrans.c

@@ -458,7 +458,7 @@ run_transform_list_forwards(png_transform_controlp tc, png_transformp *start,
    /* Called from the init code and below, the caller must initialize 'tc' */
 {
    png_const_structp png_ptr = tc->png_ptr;
-   unsigned int max_depth = 0;
+   unsigned int max_depth = PNG_TC_PIXEL_DEPTH(*tc);
 
    /* Caller guarantees that *start is non-NULL */
    debug(*start != NULL);
@@ -494,9 +494,9 @@ run_transform_list_forwards(png_transform_controlp tc, png_transformp *start,
 #ifdef PNG_READ_TRANSFORMS_SUPPORTED
 unsigned int /* PRIVATE */
 png_run_this_transform_list_forwards(png_transform_controlp tc,
-   png_transformp *start, png_transformp *end)
+   png_transformp *start, png_transformp end)
 {
-   return run_transform_list_forwards(tc, start, *end);
+   return run_transform_list_forwards(tc, start, end);
 }
 #endif /* READ_TRANSFORMS */
 
@@ -504,18 +504,11 @@ png_run_this_transform_list_forwards(png_transform_controlp tc,
 unsigned int /* PRIVATE */
 png_run_transform_list_forwards(png_structp png_ptr, png_transform_controlp tc)
 {
-   unsigned int max_depth = PNG_PIXEL_DEPTH(*png_ptr);
-
    if (png_ptr->transform_list != NULL)
-   {
-      unsigned int depth =
-         run_transform_list_forwards(tc, &png_ptr->transform_list, NULL);
+      return run_transform_list_forwards(tc, &png_ptr->transform_list, NULL);
 
-      if (depth > max_depth)
-         max_depth = depth;
-   }
-
-   return max_depth;
+   else
+      return PNG_PIXEL_DEPTH(*png_ptr);
 }
 #endif /* READ */
 
@@ -2284,6 +2277,36 @@ png_init_byte_ops(png_transformp *transform, png_transform_controlp tc)
 #endif /* SWAP poo */
 
 #ifdef PNG_READ_RGB_TO_GRAY_SUPPORTED
+static void
+png_init_rgb_to_gray_byte_ops(png_transformp *transform, 
+   png_transform_controlp tc)
+{
+   /* This just delay initializes the function; all the transform initialization
+    * has been done below.
+    */
+   (*transform)->fn = png_do_byte_ops_up;
+
+   /*  If this happens on a row do the transform immediately: */
+   if (!tc->init)
+      png_do_byte_ops_up(transform, tc);
+
+   else
+   {
+      /* This doing the init - update the row information here */
+#     define png_ptr (tc->png_ptr)
+      png_transform_byte_op *tr =
+         png_transform_cast(png_transform_byte_op, *transform);
+
+      debug(tc->bit_depth == 8U || tc->bit_depth == 16U);
+      debug((tc->format & PNG_FORMAT_FLAG_COLORMAP) == 0U &&
+            (tc->format & PNG_FORMAT_FLAG_COLOR) != 0U);
+
+      tc->format = tr->format;
+      tc->bit_depth = tr->bit_depth;
+#     undef png_ptr
+   }
+}
+
 void /* PRIVATE */
 png_add_rgb_to_gray_byte_ops(png_structrp png_ptr, png_transform_controlp tc,
    unsigned int index, unsigned int order)
@@ -2294,7 +2317,7 @@ png_add_rgb_to_gray_byte_ops(png_structrp png_ptr, png_transform_controlp tc,
 {
    png_transform_byte_op *tr = png_transform_cast(png_transform_byte_op,
       png_add_transform(png_ptr, sizeof (png_transform_byte_op),
-         png_do_byte_ops_up, order));
+         png_init_rgb_to_gray_byte_ops, order));
 
    affirm((tc->format & (PNG_FORMAT_FLAG_COLOR+PNG_FORMAT_FLAG_COLORMAP)) ==
             PNG_FORMAT_FLAG_COLOR &&
@@ -2305,11 +2328,8 @@ png_add_rgb_to_gray_byte_ops(png_structrp png_ptr, png_transform_controlp tc,
 
    /* For 1 byte channel [index] plus, maybe, alpha: */
    if (tc->bit_depth == 8)
-   {
       tr->codes = 8U + index +
          ((tc->format & PNG_FORMAT_FLAG_ALPHA) != 0 ? (8U+3U) << 4 : 0U);
-      UNTESTED
-   }
 
    else
    {
@@ -2320,7 +2340,6 @@ png_add_rgb_to_gray_byte_ops(png_structrp png_ptr, png_transform_controlp tc,
       tr->codes = (8U + index) + ((9U + index) << 4) +
          ((tc->format & PNG_FORMAT_FLAG_ALPHA) != 0 ?
             ((8U+6U) + ((9U+6U) << 4)) << 8 : 0U);
-      UNTESTED
    }
 }
 #endif /* READ_RGB_TO_GRAY */

pngwutil.c

@@ -1427,8 +1427,6 @@ png_write_tRNS(png_structrp png_ptr, png_const_bytep trans_alpha,
                   inverted_alpha.u32[i] = ~inverted_alpha.u32[i];
 
                trans_alpha = inverted_alpha.b8;
-
-               UNTESTED
             }
 #        endif /* WRITE_INVERT_ALPHA */