Bug 63359 - cairo-1.12.14 - (Incorrectly?) treat libpng warnings as error, causing issues in applications
Summary: cairo-1.12.14 - (Incorrectly?) treat libpng warnings as error, causing issues...
Status: RESOLVED FIXED
Alias: None
Product: cairo
Classification: Unclassified
Component: png functions (show other bugs)
Version: 1.12.14
Hardware: Other All
: medium normal
Assignee: Carl Worth
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-10 07:21 UTC by Richard Grenville
Modified: 2013-04-16 10:16 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
cairo-ignore-libpng-warnings.patch (474 bytes, patch)
2013-04-12 09:17 UTC, Richard Grenville
Details | Splinter Review

Description Richard Grenville 2013-04-10 07:21:23 UTC
Problem:

When reading a PNG file, cairo treats libpng warnings as errors.

cairo-1.12.12/src/cairo-png.c:
----
static void
png_simple_warning_callback (png_structp png,
	                     png_const_charp error_msg)
{
    cairo_status_t *error = png_get_error_ptr (png);

    /* default to the most likely error */
    if (*error == CAIRO_STATUS_SUCCESS)
	*error = _cairo_error (CAIRO_STATUS_NO_MEMORY);

    /* png does not expect to abort and will try to tidy up after a warning */
}

// ...

static cairo_surface_t *
read_png (struct png_read_closure_t *png_closure)
{

// ...
    png = png_create_read_struct (PNG_LIBPNG_VER_STRING,
                                  &status,
	                          png_simple_error_callback,
	                          png_simple_warning_callback);

// ...
    if (unlikely (status)) { /* catch any early warnings */
	surface = _cairo_surface_create_in_error (status);
	goto BAIL;
    }
----

This prevents some PNGs with problems to be loaded with cairo_image_surface_create_from_png(). Not sure if it's intentional or a mistake, but it's causing breakages in fcitx-4.2.7 recently.

How to reproduce the issue:

Here's slightly broken PNG image, included in fcitx-4.2.7: https://raw.github.com/fcitx/fcitx/be9f3af1aa5eb4e037ba6e6dee098edb7e41e9cc/skin/default/cn.png

Small test program to load a PNG with cairo, taken from < http://permalink.gmane.org/gmane.comp.lib.cairo/23741 >:
----
#include <stdio.h>
#include <cairo/cairo.h>

int main(int argc, char *argv[])
{
  cairo_surface_t *surface;

  surface = cairo_image_surface_create_from_png(argv[1]);
  if (cairo_surface_status(surface))  
    fprintf(stderr, "Error reading PNG image %s: %s\n",
            argv[1], cairo_status_to_string (cairo_surface_status(surface)));
  else
    fprintf(stderr, "PNG image read.\n");

  return 0;
}
----

With <libpng-1.6, cairo loads this image correctly; with libpng-1.6.1, cairo complains "Error reading PNG image cn.png: out of memory". (There's another issue regarding libpng-1.6, #62779, that could cause the same result, and the patch there needs to be applied firstly.)

Background - fcitx-4.2.7 segfaults because of the problem:

After upgrading to libpng-1.6.1, I found fcitx segfaulting immediately after execution. It looks like an issue in cairo and applying the patch from #62779 to cairo-1.12.12 fixed it. But later I found fcitx is still segfaulting sometimes, because it fails to load some particular PNG files with cairo_image_surface_create_from_png() and it isn't handling the failure gracefully. The PNG file used to work correctly with cairo and libpng-1.5, but with libpng-1.6.1 cairo could no longer load it. Further debugging reveals cairo refuses to load the file because libpng-1.6.1 emits a warning:

----
Breakpoint 5, png_simple_warning_callback (png=0x555555759c00, 
    error_msg=0x7fffffffd870 "iCCP: known incorrect sRGB profile") at cairo-png.c:151
----

"iCCP: known incorrect sRGB profile" isn't actually a fatal issue in a PNG file, and <libpng-1.6 probably doesn't emit this warning. Yet since cairo treats libpng warnings as errors, it refuses to the load the file.

I've already reported the issue to fcitx devs < http://code.google.com/p/fcitx/issues/detail?id=678 >, but there are many, many, PNGs in the world that are just slightly broken. Maybe cairo could be a little more tolerant on libpng warnings? I'm personally using a custom patch to ignore libpng warnings in png_simple_warning_callback() as a workaround.
Comment 1 Richard Grenville 2013-04-12 09:17:58 UTC
Created attachment 77853 [details] [review]
cairo-ignore-libpng-warnings.patch

cairo-ignore-libpng-warnings.patch. Just an ugly patch to ignore libpng warnings right now.
Comment 2 Richard Grenville 2013-04-13 10:20:27 UTC
The PNG images bundled with fcitx have been fixed ( https://github.com/fcitx/fcitx/commit/dc36c1a88f863971f81a76b8d9ec0db23b0d9387 ), yet I was told by the fcitx developer that many other PNG images have an incorrect ICC color profile. He suggested me to search in /usr/share/icons `(find /usr/share/icons -iname '*.png' -type f -exec identify -verbose '{}' +) |& grep 'known incorrect sRGB profile'` and I found like 7-8 images with the same problem under libpng-1.6. Thus with libpng-1.6, cairo will unfortunately still refuse to load many images.
Comment 3 Chris Wilson 2013-04-16 10:16:04 UTC
commit 2dd2c826a5b367d32cf2d48ed69754795990c5db
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Apr 16 10:58:56 2013 +0100

    png: Avoid marking the surface as in error after a png warning

Missed this bug report (amongst the various reports of the libpng16 api breakage), sorry.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.