<div dir="ltr">Recommend the unpremultiply be written like this:<div><br></div><div>   if (a > 0 && a < 1) {</div><div>      ... r/a ...<br></div><div>   } else {</div><div>      .... r, g, b, (a>0)?1:0;</div><div>   }</div><div><br></div><div>The purpose is to avoid the division for a > 1.0, which will happen when various filtering operations are done but will produce the wrong color. The above code also leaves the color alone on transparent and treats NaN in the alpha as 0.0, both of which I have also found useful.</div><div><br></div><div>It would really be nice if cairo provided an option to treat png as premultiplied, but it is the toy interface. Just be warned that a huge number of them are premultipled (the spec is ignored a lot!).</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 6, 2018 at 7:59 AM, Maarten Lankhorst <span dir="ltr"><<a href="mailto:maarten.lankhorst@linux.intel.com" target="_blank">maarten.lankhorst@linux.intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">_cairo_image_surface_coerce will round down the image to a lower<br>
bpp when using one of the floating point formats, so don't coerce those.<br>
This makes the code actually work for those formats.<br>
<br>
Because a float takes more storage than u16, we have to convert float<br>
to u16 before calling png_write_image, because png_write<br>
doesn't give us back the original row data, but an in-place copy.<br>
<br>
With these changes we can dump floating point files with the highest<br>
possible accuracy, with floats clamped between 0 and 1.<br>
<br>
Signed-off-by: Maarten Lankhorst <<a href="mailto:maarten.lankhorst@linux.intel.com">maarten.lankhorst@linux.<wbr>intel.com</a>><br>
---<br>
 src/cairo-png.c | 108 ++++++++++++++++++++++++++++++<wbr>+++++++++++++-----<br>
 1 file changed, 97 insertions(+), 11 deletions(-)<br>
<br>
diff --git a/src/cairo-png.c b/src/cairo-png.c<br>
index b9fc9160a8ab..2e0520ae8fac 100644<br>
--- a/src/cairo-png.c<br>
+++ b/src/cairo-png.c<br>
@@ -105,6 +105,57 @@ unpremultiply_data (png_structp png, png_row_infop row_info, png_bytep data)<br>
     }<br>
 }<br>
<br>
+static uint16_t f_to_u16(float val)<br>
+{<br>
+    if (val < 0)<br>
+       return 0;<br>
+    else if (val > 1)<br>
+       return 65535;<br>
+    else<br>
+       return (uint16_t)(val * 65535.f);<br>
+}<br>
+<br>
+static void<br>
+unpremultiply_float (float *f, uint16_t *d16, unsigned width)<br>
+{<br>
+    unsigned int i;<br>
+<br>
+    for (i = 0; i < width; i++) {<br>
+       float r, g, b, a;<br>
+<br>
+       r = *f++;<br>
+       g = *f++;<br>
+       b = *f++;<br>
+       a = *f++;<br>
+<br>
+       if (a > 0) {<br>
+           *d16++ = f_to_u16(r / a);<br>
+           *d16++ = f_to_u16(g / a);<br>
+           *d16++ = f_to_u16(b / a);<br>
+           *d16++ = f_to_u16(a);<br>
+       } else {<br>
+           *d16++ = 0;<br>
+           *d16++ = 0;<br>
+           *d16++ = 0;<br>
+           *d16++ = 0;<br>
+       }<br>
+    }<br>
+}<br>
+<br>
+<br>
+static void<br>
+convert_float_to_u16 (float *f, uint16_t *d16, unsigned int width)<br>
+{<br>
+    unsigned int i;<br>
+<br>
+    for (i = 0; i < width; i++) {<br>
+       *d16++ = f_to_u16(*f++);<br>
+       *d16++ = f_to_u16(*f++);<br>
+       *d16++ = f_to_u16(*f++);<br>
+       *d16++ = 0;<br>
+    }<br>
+}<br>
+<br>
 /* Converts native endian xRGB => RGBx bytes */<br>
 static void<br>
 convert_data_to_bytes (png_structp png, png_row_infop row_info, png_bytep data)<br>
@@ -182,6 +233,7 @@ write_png (cairo_surface_t  *surface,<br>
     png_color_16 white;<br>
     int png_color_type;<br>
     int bpc;<br>
+    unsigned char *volatile u16_copy = NULL;<br>
<br>
     status = _cairo_surface_acquire_source_<wbr>image (surface,<br>
                                                  &image,<br>
@@ -198,11 +250,22 @@ write_png (cairo_surface_t        *surface,<br>
        goto BAIL1;<br>
     }<br>
<br>
-    /* Handle the various fallback formats (e.g. low bit-depth XServers)<br>
-     * by coercing them to a simpler format using pixman.<br>
-     */<br>
-    clone = _cairo_image_surface_coerce (image);<br>
-    status = clone->base.status;<br>
+    /* Don't coerce to a lower resolution format */<br>
+    if (image->format == CAIRO_FORMAT_RGB96F ||<br>
+        image->format == CAIRO_FORMAT_RGBA128F) {<br>
+       u16_copy = _cairo_malloc_ab (image->width * 8, image->height);<br>
+       if (!u16_copy) {<br>
+           status = _cairo_error (CAIRO_STATUS_NO_MEMORY);<br>
+           goto BAIL1;<br>
+       }<br>
+       clone = (cairo_image_surface_t *)cairo_surface_reference (&image->base);<br>
+    } else {<br>
+         /* Handle the various fallback formats (e.g. low bit-depth XServers)<br>
+         * by coercing them to a simpler format using pixman.<br>
+         */<br>
+         clone = _cairo_image_surface_coerce (image);<br>
+         status = clone->base.status;<br>
+    }<br>
     if (unlikely (status))<br>
         goto BAIL1;<br>
<br>
@@ -212,8 +275,22 @@ write_png (cairo_surface_t *surface,<br>
        goto BAIL2;<br>
     }<br>
<br>
-    for (i = 0; i < clone->height; i++)<br>
-       rows[i] = (png_byte *) clone->data + i * clone->stride;<br>
+    if (!u16_copy) {<br>
+       for (i = 0; i < clone->height; i++)<br>
+           rows[i] = (png_byte *)clone->data + i * clone->stride;<br>
+    } else {<br>
+       for (i = 0; i < clone->height; i++) {<br>
+           float *float_line = (float *)&clone->data[i * clone->stride];<br>
+           uint16_t *u16_line = (uint16_t *)&u16_copy[i * clone->width * 8];<br>
+<br>
+           if (image->format == CAIRO_FORMAT_RGBA128F)<br>
+               unpremultiply_float (float_line, u16_line, clone->width);<br>
+           else<br>
+               convert_float_to_u16 (float_line, u16_line, clone->width);<br>
+<br>
+           rows[i] = (png_byte *)u16_line;<br>
+       }<br>
+    }<br>
<br>
     png = png_create_write_struct (PNG_LIBPNG_VER_STRING, &status,<br>
                                   png_simple_error_callback,<br>
@@ -263,10 +340,16 @@ write_png (cairo_surface_t        *surface,<br>
        png_set_packswap (png);<br>
 #endif<br>
        break;<br>
-    case CAIRO_FORMAT_INVALID:<br>
-    case CAIRO_FORMAT_RGB16_565:<br>
     case CAIRO_FORMAT_RGB96F:<br>
+       bpc = 16;<br>
+       png_color_type = PNG_COLOR_TYPE_RGB;<br>
+       break;<br>
     case CAIRO_FORMAT_RGBA128F:<br>
+       bpc = 16;<br>
+       png_color_type = PNG_COLOR_TYPE_RGB_ALPHA;<br>
+       break;<br>
+    case CAIRO_FORMAT_INVALID:<br>
+    case CAIRO_FORMAT_RGB16_565:<br>
     default:<br>
        status = _cairo_error (CAIRO_STATUS_INVALID_FORMAT);<br>
        goto BAIL4;<br>
@@ -298,9 +381,11 @@ write_png (cairo_surface_t *surface,<br>
     png_write_info (png, info);<br>
<br>
     if (png_color_type == PNG_COLOR_TYPE_RGB_ALPHA) {<br>
-       png_set_write_user_transform_<wbr>fn (png, unpremultiply_data);<br>
+       if (clone->format != CAIRO_FORMAT_RGBA128F)<br>
+           png_set_write_user_transform_<wbr>fn (png, unpremultiply_data);<br>
     } else if (png_color_type == PNG_COLOR_TYPE_RGB) {<br>
-       png_set_write_user_transform_<wbr>fn (png, convert_data_to_bytes);<br>
+       if (clone->format != CAIRO_FORMAT_RGB96F)<br>
+           png_set_write_user_transform_<wbr>fn (png, convert_data_to_bytes);<br>
        png_set_filler (png, 0, PNG_FILLER_AFTER);<br>
     }<br>
<br>
@@ -313,6 +398,7 @@ BAIL3:<br>
     free (rows);<br>
 BAIL2:<br>
     cairo_surface_destroy (&clone->base);<br>
+    free (u16_copy);<br>
 BAIL1:<br>
     _cairo_surface_release_source_<wbr>image (surface, image, image_extra);<br>
<span class="HOEnZb"><font color="#888888"> <br>
-- <br>
2.18.0<br>
<br>
-- <br>
cairo mailing list<br>
<a href="mailto:cairo@cairographics.org">cairo@cairographics.org</a><br>
<a href="https://lists.cairographics.org/mailman/listinfo/cairo" rel="noreferrer" target="_blank">https://lists.cairographics.<wbr>org/mailman/listinfo/cairo</a></font></span></blockquote></div><br></div>