bug#37153: 26.1; some png images scrambled

bug#37153: 26.1; some png images scrambled

[Roland Winkler]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 934 bytes --]


put attached file into ~/picture.png
start emacs -Q
evaluate
(insert-image (create-image "~/picture.png"))
The image is scrambled.  With imagemagick the image displays correctly
(insert-image (create-image "~/picture.png" 'imagemagick))
Of course, not all png images are scrambled.  So it seems to be a bug
that is triggered only by certain png images.


In GNU Emacs 26.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.18.9)
 of 2018-05-29 built on regnitz
Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
System Description:	Ubuntu 16.04.6 LTS

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT
ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 THREADS LCMS2

Important settings:
  value of $LC_COLLATE: C
  value of $LANG: en_US.utf8
  value of $XMODIFIERS: 
  locale-coding-system: utf-8-unix

Major mode: Fundamental


[-- Attachment #2: picture.png --]
[-- Type: image/png, Size: 379 bytes --]

bug#37153: 26.1; some png images scrambled

[Eli Zaretskii]

> Date: Thu, 22 Aug 2019 23:49:00 -0500
> From: "Roland Winkler" <winkler@gnu.org>
> 
> put attached file into ~/picture.png
> start emacs -Q
> evaluate
> (insert-image (create-image "~/picture.png"))
> The image is scrambled.  With imagemagick the image displays correctly
> (insert-image (create-image "~/picture.png" 'imagemagick))
> Of course, not all png images are scrambled.  So it seems to be a bug
> that is triggered only by certain png images.

Are you sure this is not a libpng bug?

bug#37153: 26.1; some png images scrambled

[Roland Winkler]

On Fri Aug 23 2019 Eli Zaretskii wrote:
> Are you sure this is not a libpng bug?

No, I do not know the culprit.  How can I find out?

(On my GNU/linux computer, eog displays the image correctly.  Which
other software uses libpng similar to emacs?)

bug#37153: 26.1; some png images scrambled

[Eli Zaretskii]

> Date: Fri, 23 Aug 2019 06:39:10 -0500
> From: "Roland Winkler" <winkler@gnu.org>
> Cc: 37153@debbugs.gnu.org
> 
> On Fri Aug 23 2019 Eli Zaretskii wrote:
> > Are you sure this is not a libpng bug?
> 
> No, I do not know the culprit.  How can I find out?

By asking on the libpng-related forum (assuming there is such a
thing)?

> (On my GNU/linux computer, eog displays the image correctly.  Which
> other software uses libpng similar to emacs?)

I don't know.  Anyone?

bug#37153: 26.1; some png images scrambled

[Roland Winkler]

On Fri Aug 23 2019 Eli Zaretskii wrote:
> > No, I do not know the culprit.  How can I find out?
> 
> By asking on the libpng-related forum (assuming there is such a
> thing)?
> 
> > (On my GNU/linux computer, eog displays the image correctly.  Which
> > other software uses libpng similar to emacs?)
> 
> I don't know.  Anyone?

See

https://sourceforge.net/p/png-mng/mailman/message/36746994/

bug#37153: 26.1; some png images scrambled

[Eli Zaretskii]

> Date: Fri, 23 Aug 2019 10:07:52 -0500
> From: "Roland Winkler" <winkler@gnu.org>
> Cc: 37153@debbugs.gnu.org
> 
> https://sourceforge.net/p/png-mng/mailman/message/36746994/

Thanks, let's see what this brings up.

bug#37153: 26.1; some png images scrambled

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 465 bytes --]

I didn't reproduce the problem with Emacs master on Ubuntu 18.04.3 LTS. The 
Emacs display agrees with the Gimp's display of that tiny image. Also, the rpng 
test program of the librpng 1.6.37 (the current version) also displays the image 
the same way. I had to patch librpng with the attached to get the test program 
to compile.

Do you see the same problem when using Emacs master? If not, then perhaps we can 
just marked the bug as fixed in the next release.

[-- Attachment #2: libpng.patch --]
[-- Type: text/x-patch, Size: 599 bytes --]

diff -pru libpng-1.6.37/contrib/gregbook/readpng.c libpng-1.6.37-fixed/contrib/gregbook/readpng.c
--- libpng-1.6.37/contrib/gregbook/readpng.c	2019-04-14 11:10:32.000000000 -0700
+++ libpng-1.6.37-fixed/contrib/gregbook/readpng.c	2019-08-23 12:14:13.106180379 -0700
@@ -266,7 +266,7 @@ uch *readpng_get_image(double display_ex

     /* Guard against integer overflow */
     if (height > ((size_t)(-1))/rowbytes) {
-        fprintf(stderr, "readpng:  image_data buffer would be too large\n",
+        fprintf(stderr, "readpng:  image_data buffer would be too large\n");
         return NULL;
     }

bug#37153: 26.1; some png images scrambled

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 23 Aug 2019 12:37:12 -0700
> Cc: 37153@debbugs.gnu.org
> 
> I didn't reproduce the problem with Emacs master on Ubuntu 18.04.3 LTS. The 
> Emacs display agrees with the Gimp's display of that tiny image. Also, the rpng 
> test program of the librpng 1.6.37 (the current version) also displays the image 
> the same way. I had to patch librpng with the attached to get the test program 
> to compile.

Please show the screenshot of the image as you saw it.  Otherwise,
it's hard to understand what do you mean by "not reproducing" the
problem.

Thanks.

bug#37153: 26.1; some png images scrambled

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

Eli Zaretskii wrote:
> Please show the screenshot of the image as you saw it.

My screenshot software outputs .png so if there's a problem with the original 
.png image there will quite possibly be a problem with the screenshot too. But 
anyway, attached is a screenshot of how Gimp displays the test image, magnified 
16x. The checkerboard is how Gimp displays absent parts of the image, and the 
gray frame around the edge is also part of Gimp and not part of the image.

[-- Attachment #2: Screenshot from 2019-08-23 13-29-06.png --]
[-- Type: image/png, Size: 3241 bytes --]

bug#37153: 26.1; some png images scrambled

[Roland Winkler]

On Fri Aug 23 2019 Paul Eggert wrote:
> Do you see the same problem when using Emacs master? If not, then
> perhaps we can just marked the bug as fixed in the next release.

The problem with my test png image is that it has transparent parts
that need to be displayed with a light color so that one can
recognize the nontransparent dark parts.  But the image uses fully
transparent black as background.  The behavior in such a case is not
strictly defined by the png standard, see

https://sourceforge.net/p/png-mng/mailman/message/36747189/

Emacs displays such fully transparent background with its original
color specified by the image.  But I believe it would make more
sense if emacs used instead a background color that the user can
choose (the background color of the frame), similar to, for example,
eog.

Should I submit a separate feature request for this?

bug#37153: 26.1; some png images scrambled

[Paul Eggert]

Roland Winkler wrote:
> I believe it would make more
> sense if emacs used instead a background color that the user can
> choose (the background color of the frame), similar to, for example,
> eog.
> 
> Should I submit a separate feature request for this?

Can you do this with faces? They let you specify background colors. (I don't use 
this stuff so you'll have to be the expert.)

If there's no way to do it in Emacs now, I suggest retitling this bug report and 
changing it to a feature request, which you can do as described here:

https://debbugs.gnu.org/server-control.html

bug#37153: 26.1; some png images scrambled

[Roland Winkler]

On Fri Aug 23 2019 Paul Eggert wrote:
> Can you do this with faces? They let you specify background
> colors. (I don't use this stuff so you'll have to be the expert.)

I am not an expert either.  When I created a buffer with my png
image plus ordinary text, I could change the background color of the
text via add-face-text-property.  But this did not affect the
background color of the image.  So I believe the answer is no.

> If there's no way to do it in Emacs now, I suggest retitling this
> bug report and changing it to a feature request, which you can do
> as described here:
> 
> https://debbugs.gnu.org/server-control.html

Thanks, I'll wait a day or two.  If nobody tells me that I have
overlooked something significant, I'll turn this bug report into a
feature request.

bug#37153: 26.1; some png images scrambled

[Eli Zaretskii]

> Cc: winkler@gnu.org, 37153@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 23 Aug 2019 13:33:11 -0700
> 
> Eli Zaretskii wrote:
> > Please show the screenshot of the image as you saw it.
> 
> My screenshot software outputs .png so if there's a problem with the original 
> .png image there will quite possibly be a problem with the screenshot too. But 
> anyway, attached is a screenshot of how Gimp displays the test image, magnified 
> 16x. The checkerboard is how Gimp displays absent parts of the image, and the 
> gray frame around the edge is also part of Gimp and not part of the image.

This is not how this should be displayed, AFAIU.  Try opening the same
image in Firefox, and you will see a much more reasonable display.  I
presume that's what the OP wanted.

bug#37153: 26.1; some png images scrambled

[Eli Zaretskii]

> Date: Fri, 23 Aug 2019 22:17:58 -0500
> From: "Roland Winkler" <winkler@gnu.org>
> Cc: 37153@debbugs.gnu.org
> 
> The problem with my test png image is that it has transparent parts
> that need to be displayed with a light color so that one can
> recognize the nontransparent dark parts.  But the image uses fully
> transparent black as background.  The behavior in such a case is not
> strictly defined by the png standard, see
> 
> https://sourceforge.net/p/png-mng/mailman/message/36747189/
> 
> Emacs displays such fully transparent background with its original
> color specified by the image.  But I believe it would make more
> sense if emacs used instead a background color that the user can
> choose (the background color of the frame), similar to, for example,
> eog.

Please take a look at image.c in the PNG area: it includes quite a few
references to "transparent" and what we should be/are doing with that.

> Should I submit a separate feature request for this?

I don't think I understand the feature you want to request.  Please
don't forget that the same PNG image can be displayed on places that
have different background colors.  I also don't think I see the use
cases for such images in Emacs.

bug#37153: 26.1; some png images scrambled

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 23 Aug 2019 20:39:17 -0700
> Cc: 37153@debbugs.gnu.org
> 
> Roland Winkler wrote:
> > I believe it would make more
> > sense if emacs used instead a background color that the user can
> > choose (the background color of the frame), similar to, for example,
> > eog.
> > 
> > Should I submit a separate feature request for this?
> 
> Can you do this with faces?

No, not AFAIK.

bug#37153: 26.1; some png images scrambled

[mituharu]

> On Fri Aug 23 2019 Paul Eggert wrote:
>> Do you see the same problem when using Emacs master? If not, then
>> perhaps we can just marked the bug as fixed in the next release.
>
> The problem with my test png image is that it has transparent parts
> that need to be displayed with a light color so that one can
> recognize the nontransparent dark parts.  But the image uses fully
> transparent black as background.  The behavior in such a case is not
> strictly defined by the png standard, see
>
> https://sourceforge.net/p/png-mng/mailman/message/36747189/
>
> Emacs displays such fully transparent background with its original
> color specified by the image.  But I believe it would make more
> sense if emacs used instead a background color that the user can
> choose (the background color of the frame), similar to, for example,
> eog.

I suspect there is a longstanding typo (or thinko) in PNG
transparency handling code.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

diff --git a/src/image.c b/src/image.c
index 81d8cb4e2b2..819e058f7e1 100644
--- a/src/image.c
+++ b/src/image.c
@@ -6680,7 +6680,7 @@ png_load_body (struct frame *f, struct image *img,
struct png_load_context *c)
   /* Create an image and pixmap serving as mask if the PNG image
      contains an alpha channel.  */
   if (channels == 4
-      && !transparent_p
+      && transparent_p
       && !image_create_x_image_and_pixmap (f, img, width, height, 1,
 					   &mask_img, 1))
     {

bug#37153: 26.1; some png images scrambled

[Eli Zaretskii]

> Date: Sat, 24 Aug 2019 18:01:46 +0900
> From: mituharu@math.s.chiba-u.ac.jp
> Cc: 37153@debbugs.gnu.org, Paul Eggert <eggert@cs.ucla.edu>
> 
> I suspect there is a longstanding typo (or thinko) in PNG
> transparency handling code.
> 
> 				     YAMAMOTO Mitsuharu
> 				mituharu@math.s.chiba-u.ac.jp
> 
> diff --git a/src/image.c b/src/image.c
> index 81d8cb4e2b2..819e058f7e1 100644
> --- a/src/image.c
> +++ b/src/image.c
> @@ -6680,7 +6680,7 @@ png_load_body (struct frame *f, struct image *img,
> struct png_load_context *c)
>    /* Create an image and pixmap serving as mask if the PNG image
>       contains an alpha channel.  */
>    if (channels == 4
> -      && !transparent_p
> +      && transparent_p
>        && !image_create_x_image_and_pixmap (f, img, width, height, 1,
>  					   &mask_img, 1))
>      {
> 

That didn't change anything in how I see the image in question, FWIW.

bug#37153: 26.1; some png images scrambled

[YAMAMOTO Mitsuharu]

[-- Attachment #1: Type: text/plain, Size: 1142 bytes --]

On Sat, 24 Aug 2019 19:17:31 +0900,
Eli Zaretskii wrote:
> 
> > Date: Sat, 24 Aug 2019 18:01:46 +0900
> > From: mituharu@math.s.chiba-u.ac.jp
> > Cc: 37153@debbugs.gnu.org, Paul Eggert <eggert@cs.ucla.edu>
> > 
> > I suspect there is a longstanding typo (or thinko) in PNG
> > transparency handling code.
> > 
> > 				     YAMAMOTO Mitsuharu
> > 				mituharu@math.s.chiba-u.ac.jp
> > 
> > diff --git a/src/image.c b/src/image.c
> > index 81d8cb4e2b2..819e058f7e1 100644
> > --- a/src/image.c
> > +++ b/src/image.c
> > @@ -6680,7 +6680,7 @@ png_load_body (struct frame *f, struct image *img,
> > struct png_load_context *c)
> >    /* Create an image and pixmap serving as mask if the PNG image
> >       contains an alpha channel.  */
> >    if (channels == 4
> > -      && !transparent_p
> > +      && transparent_p
> >        && !image_create_x_image_and_pixmap (f, img, width, height, 1,
> >  					   &mask_img, 1))
> >      {
> > 
> 
> That didn't change anything in how I see the image in question, FWIW.

Attached is a screenshot on X11.  Probably W32 needs some more work.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

[-- Attachment #2: tRNS.png --]
[-- Type: image/png, Size: 13814 bytes --]

bug#37153: 26.1; some png images scrambled

[Roland Winkler]

On Sat Aug 24 2019 mituharu@math.s.chiba-u.ac.jp wrote:
> I suspect there is a longstanding typo (or thinko) in PNG
> transparency handling code.

It may be helpful to check the "official" test-suite for PNG

http://www.schaik.com/pngsuite2011/pngsuite.html

which contains also png files with transparency

http://www.schaik.com/pngsuite2011/pngsuite_trn_png.html

Somehow emacs ignores the transparency for image type png, see for
example the file tm3n3p02.png with multiple levels of transparency.

Transparency is honored for these images when emacs uses imagemagick.

bug#37153: 26.1; some png images scrambled

[YAMAMOTO Mitsuharu]

On Sun, 25 Aug 2019 13:52:32 +0900,
Roland Winkler wrote:
> 
> On Sat Aug 24 2019 mituharu@math.s.chiba-u.ac.jp wrote:
> > I suspect there is a longstanding typo (or thinko) in PNG
> > transparency handling code.
> 
> It may be helpful to check the "official" test-suite for PNG
> 
> http://www.schaik.com/pngsuite2011/pngsuite.html
> 
> which contains also png files with transparency
> 
> http://www.schaik.com/pngsuite2011/pngsuite_trn_png.html
> 
> Somehow emacs ignores the transparency for image type png, see for
> example the file tm3n3p02.png with multiple levels of transparency.
> 
> Transparency is honored for these images when emacs uses imagemagick.

It seems to be necessary to look into the tRNS chunk data for paletted
images.  Again, W32 would need some more work.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

diff --git a/src/image.c b/src/image.c
index 81d8cb4e2b2..7f1561d7a68 100644
--- a/src/image.c
+++ b/src/image.c
@@ -6589,10 +6589,28 @@ png_load_body (struct frame *f, struct image *img, struct png_load_context *c)
 
   /* If image contains simply transparency data, we prefer to
      construct a clipping mask.  */
+  transparent_p = false;
   if (png_get_valid (png_ptr, info_ptr, PNG_INFO_tRNS))
-    transparent_p = 1;
-  else
-    transparent_p = 0;
+    {
+      if (color_type != PNG_COLOR_TYPE_PALETTE)
+	transparent_p = true;
+      else
+	{
+	  /* Paletted images can specify transparency levels. */
+	  png_bytep trans;
+	  int i, num_trans;
+
+	  if (png_get_tRNS (png_ptr, info_ptr, &trans, &num_trans, NULL)
+	      == PNG_INFO_tRNS)
+	    {
+	      for (i = 0; i < num_trans; i++)
+		if (!(trans[i] == 0 || trans[i] == 255))
+		  break;
+	      if (i == num_trans)
+		transparent_p = true;
+	    }
+	}
+    }
 
   /* This function is easier to write if we only have to handle
      one data format: RGB or RGBA with 8 bits per channel.  Let's
@@ -6680,7 +6698,7 @@ png_load_body (struct frame *f, struct image *img, struct png_load_context *c)
   /* Create an image and pixmap serving as mask if the PNG image
      contains an alpha channel.  */
   if (channels == 4
-      && !transparent_p
+      && transparent_p
       && !image_create_x_image_and_pixmap (f, img, width, height, 1,
 					   &mask_img, 1))
     {

bug#37153: 26.1; some png images scrambled

[Eli Zaretskii]

> Date: Sun, 25 Aug 2019 13:19:24 +0900
> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> Cc: winkler@gnu.org,
> 	37153@debbugs.gnu.org,
> 	eggert@cs.ucla.edu
> 
> > > --- a/src/image.c
> > > +++ b/src/image.c
> > > @@ -6680,7 +6680,7 @@ png_load_body (struct frame *f, struct image *img,
> > > struct png_load_context *c)
> > >    /* Create an image and pixmap serving as mask if the PNG image
> > >       contains an alpha channel.  */
> > >    if (channels == 4
> > > -      && !transparent_p
> > > +      && transparent_p
> > >        && !image_create_x_image_and_pixmap (f, img, width, height, 1,
> > >  					   &mask_img, 1))
> > >      {
> > > 
> > 
> > That didn't change anything in how I see the image in question, FWIW.
> 
> Attached is a screenshot on X11.  Probably W32 needs some more work.

I always see the 3rd image, no matter what face is specified, FWIW.

bug#37153: 26.1; some png images scrambled

[Roland Winkler]

On Sun Aug 25 2019 YAMAMOTO Mitsuharu wrote:
> It seems to be necessary to look into the tRNS chunk data for paletted
> images.  Again, W32 would need some more work.

Thank you, I can confirm that your patch works for me (under GNU
Linux), including the transparent images from the png test suite.

bug#37153: 26.1; some png images scrambled

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 627 bytes --]

YAMAMOTO Mitsuharu wrote:

> It seems to be necessary to look into the tRNS chunk data for paletted
> images.  Again, W32 would need some more work.

Thanks, I confirmed that this fixes the misdisplay for me as well. I attempted 
to port it to w32 (as well as to unusual libpng builds that do not define 
PNG_tRNS_SUPPORTED) and installed the attached into master.

With this patch on Ubuntu 18.04.3 I always see the first image shown in your 
tRNS.pg screenshot <https://bugs.gnu.org/37153#56>. So it may be that more work 
needs to be done to support other faces. Still, this patch is an improvement so 
it's worth going in.

[-- Attachment #2: 0001-Fix-misdisplay-of-PNG-paletted-images.patch --]
[-- Type: text/x-patch, Size: 3729 bytes --]

From 64aa756cfa5943c8b3494ac1fbb4db712c50ae64 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 25 Aug 2019 10:01:46 -0700
Subject: [PATCH] Fix misdisplay of PNG paletted images

Problem reported by Roland Winkler (Bug#37153).
Derived from a patch suggested by YAMAMOTO Mitsuharu (Bug#37153#62).
* src/image.c (png_get_valid) [WINDOWSNT]:
Do not dynamically link this function.
(png_get_tRNS) [WINDOWSNT && PNG_tRNS_SUPPORTED]:
Dynamically link this function instead.
(png_load_body): Do not assume that every paletted image supplies
only transparency data.  Fix typo in use of transparent_p.
---
 src/image.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/src/image.c b/src/image.c
index 81d8cb4e2b..18495612e9 100644
--- a/src/image.c
+++ b/src/image.c
@@ -6234,7 +6234,10 @@ DEF_DLL_FN (void, png_read_info, (png_structp, png_infop));
 DEF_DLL_FN (png_uint_32, png_get_IHDR,
 	    (png_structp, png_infop, png_uint_32 *, png_uint_32 *,
 	     int *, int *, int *, int *, int *));
-DEF_DLL_FN (png_uint_32, png_get_valid, (png_structp, png_infop, png_uint_32));
+#  ifdef PNG_tRNS_SUPPORTED
+DEF_DLL_FN (png_uint_32, png_get_tRNS, (png_structp, png_infop, png_bytep *,
+					int *, png_color_16p *));
+#  endif
 DEF_DLL_FN (void, png_set_strip_16, (png_structp));
 DEF_DLL_FN (void, png_set_expand, (png_structp));
 DEF_DLL_FN (void, png_set_gray_to_rgb, (png_structp));
@@ -6273,7 +6276,9 @@ init_png_functions (void)
   LOAD_DLL_FN (library, png_set_sig_bytes);
   LOAD_DLL_FN (library, png_read_info);
   LOAD_DLL_FN (library, png_get_IHDR);
-  LOAD_DLL_FN (library, png_get_valid);
+#  ifdef PNG_tRNS_SUPPORTED
+  LOAD_DLL_FN (library, png_get_tRNS);
+#  endif
   LOAD_DLL_FN (library, png_set_strip_16);
   LOAD_DLL_FN (library, png_set_expand);
   LOAD_DLL_FN (library, png_set_gray_to_rgb);
@@ -6304,7 +6309,7 @@ init_png_functions (void)
 #  undef png_get_IHDR
 #  undef png_get_io_ptr
 #  undef png_get_rowbytes
-#  undef png_get_valid
+#  undef png_get_tRNS
 #  undef png_longjmp
 #  undef png_read_end
 #  undef png_read_image
@@ -6329,7 +6334,7 @@ init_png_functions (void)
 #  define png_get_IHDR fn_png_get_IHDR
 #  define png_get_io_ptr fn_png_get_io_ptr
 #  define png_get_rowbytes fn_png_get_rowbytes
-#  define png_get_valid fn_png_get_valid
+#  define png_get_tRNS fn_png_get_tRNS
 #  define png_longjmp fn_png_longjmp
 #  define png_read_end fn_png_read_end
 #  define png_read_image fn_png_read_image
@@ -6589,10 +6594,21 @@ png_load_body (struct frame *f, struct image *img, struct png_load_context *c)
 
   /* If image contains simply transparency data, we prefer to
      construct a clipping mask.  */
-  if (png_get_valid (png_ptr, info_ptr, PNG_INFO_tRNS))
-    transparent_p = 1;
-  else
-    transparent_p = 0;
+  transparent_p = false;
+# ifdef PNG_tRNS_SUPPORTED
+  png_bytep trans_alpha;
+  int num_trans;
+  if (png_get_tRNS (png_ptr, info_ptr, &trans_alpha, &num_trans, NULL)
+      && trans_alpha)
+    {
+      int i;
+      for (i = 0; i < num_trans; i++)
+	if (0 < trans_alpha[i] && trans_alpha[i] < 255)
+	  break;
+      if (! (i < num_trans))
+	transparent_p = true;
+    }
+# endif
 
   /* This function is easier to write if we only have to handle
      one data format: RGB or RGBA with 8 bits per channel.  Let's
@@ -6680,7 +6696,7 @@ png_load_body (struct frame *f, struct image *img, struct png_load_context *c)
   /* Create an image and pixmap serving as mask if the PNG image
      contains an alpha channel.  */
   if (channels == 4
-      && !transparent_p
+      && transparent_p
       && !image_create_x_image_and_pixmap (f, img, width, height, 1,
 					   &mask_img, 1))
     {
-- 
2.17.1

bug#37153: 26.1; some png images scrambled

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 25 Aug 2019 11:38:07 -0700
> Cc: 37153@debbugs.gnu.org
> 
> > It seems to be necessary to look into the tRNS chunk data for paletted
> > images.  Again, W32 would need some more work.
> 
> Thanks, I confirmed that this fixes the misdisplay for me as well. I attempted 
> to port it to w32 (as well as to unusual libpng builds that do not define 
> PNG_tRNS_SUPPORTED) and installed the attached into master.

Thanks, this builds on MS-Windows, but the display of that PNG image
is still incorrect.

My guess is that some changes are needed in
image_create_x_image_and_pixmap_1, where there's a separate
implementation for HAVE_NTGUI.  But I have no idea what changes are
needed there.

bug#37153: 26.1; some png images scrambled

[YAMAMOTO Mitsuharu]

On Mon, 26 Aug 2019 03:38:07 +0900,
Paul Eggert wrote:
> 
> YAMAMOTO Mitsuharu wrote:
> 
> > It seems to be necessary to look into the tRNS chunk data for paletted
> > images.  Again, W32 would need some more work.
> 
> Thanks, I confirmed that this fixes the misdisplay for me as well. I
> attempted to port it to w32 (as well as to unusual libpng builds that
> do not define PNG_tRNS_SUPPORTED) and installed the attached into
> master.
> 
> With this patch on Ubuntu 18.04.3 I always see the first image shown
> in your tRNS.pg screenshot <https://bugs.gnu.org/37153#56>. So it may
> be that more work needs to be done to support other faces. Still, this
> patch is an improvement so it's worth going in.

At least, your patch misses the case of non-paletted transparent PNGs
(tb???c?? in http://www.schaik.com/pngsuite2011/pngsuite_trn_png.html).

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

diff --git a/src/image.c b/src/image.c
index 18495612e98..8c4ab096938 100644
--- a/src/image.c
+++ b/src/image.c
@@ -6598,15 +6598,19 @@ png_load_body (struct frame *f, struct image *img, struct png_load_context *c)
 # ifdef PNG_tRNS_SUPPORTED
   png_bytep trans_alpha;
   int num_trans;
-  if (png_get_tRNS (png_ptr, info_ptr, &trans_alpha, &num_trans, NULL)
-      && trans_alpha)
+  if (png_get_tRNS (png_ptr, info_ptr, &trans_alpha, &num_trans, NULL))
     {
-      int i;
-      for (i = 0; i < num_trans; i++)
-	if (0 < trans_alpha[i] && trans_alpha[i] < 255)
-	  break;
-      if (! (i < num_trans))
+      if (!trans_alpha)
 	transparent_p = true;
+      else
+	{
+	  int i;
+	  for (i = 0; i < num_trans; i++)
+	    if (0 < trans_alpha[i] && trans_alpha[i] < 255)
+	      break;
+	  if (! (i < num_trans))
+	    transparent_p = true;
+	}
     }
 # endif
 

bug#37153: 26.1; some png images scrambled

[YAMAMOTO Mitsuharu]

On Mon, 26 Aug 2019 08:02:23 +0900,
YAMAMOTO Mitsuharu wrote:
> 
> On Mon, 26 Aug 2019 03:38:07 +0900,
> Paul Eggert wrote:
> > 
> > YAMAMOTO Mitsuharu wrote:
> > 
> > > It seems to be necessary to look into the tRNS chunk data for paletted
> > > images.  Again, W32 would need some more work.
> > 
> > Thanks, I confirmed that this fixes the misdisplay for me as well. I
> > attempted to port it to w32 (as well as to unusual libpng builds that
> > do not define PNG_tRNS_SUPPORTED) and installed the attached into
> > master.
> > 
> > With this patch on Ubuntu 18.04.3 I always see the first image shown
> > in your tRNS.pg screenshot <https://bugs.gnu.org/37153#56>. So it may
> > be that more work needs to be done to support other faces. Still, this
> > patch is an improvement so it's worth going in.
> 
> At least, your patch misses the case of non-paletted transparent PNGs
> (tb???c?? in http://www.schaik.com/pngsuite2011/pngsuite_trn_png.html).
 
And you would need to turn off font-lock-mode (or start Emacs with -D)
to replicate my tRNS.png screenshot.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

bug#37153: 26.1; some png images scrambled

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 464 bytes --]

YAMAMOTO Mitsuharu wrote:

> At least, your patch misses the case of non-paletted transparent PNGs
> (tb???c?? in http://www.schaik.com/pngsuite2011/pngsuite_trn_png.html).

Thanks for catching my mistake. I installed the attached patch, which is closer 
to the code you originally suggested. With it, displaying one of the images you 
suggested looks like the attached image; hope it's what you're looking for. (I 
disabled font-lock-mode as you also suggested.)

[-- Attachment #2: 0001-Fix-bug-with-non-paletted-transparent-PNGs.patch --]
[-- Type: text/x-patch, Size: 1297 bytes --]

From d3b17dd5f5e3805342dbe4ae20706e9520c48179 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 25 Aug 2019 16:35:43 -0700
Subject: [PATCH] Fix bug with non-paletted transparent PNGs

Adapted from a fix by YAMAMOTO Mitsuharu (Bug#37153#77).
* src/image.c (png_load_body): Fix bug with non-paletted
transparent images.
---
 src/image.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/image.c b/src/image.c
index 18495612e9..fe7bd90b05 100644
--- a/src/image.c
+++ b/src/image.c
@@ -6598,15 +6598,16 @@ png_load_body (struct frame *f, struct image *img, struct png_load_context *c)
 # ifdef PNG_tRNS_SUPPORTED
   png_bytep trans_alpha;
   int num_trans;
-  if (png_get_tRNS (png_ptr, info_ptr, &trans_alpha, &num_trans, NULL)
-      && trans_alpha)
+  if (png_get_tRNS (png_ptr, info_ptr, &trans_alpha, &num_trans, NULL))
     {
-      int i;
-      for (i = 0; i < num_trans; i++)
-	if (0 < trans_alpha[i] && trans_alpha[i] < 255)
-	  break;
-      if (! (i < num_trans))
-	transparent_p = true;
+      transparent_p = true;
+      if (trans_alpha)
+	for (int i = 0; i < num_trans; i++)
+	  if (0 < trans_alpha[i] && trans_alpha[i] < 255)
+	    {
+	      transparent_p = false;
+	      break;
+	    }
     }
 # endif
 
-- 
2.17.1


[-- Attachment #3: display-tbrn2c08.png --]
[-- Type: image/png, Size: 18246 bytes --]

bug#37153: 26.1; some png images scrambled

[Lars Ingebrigtsen]

Paul Eggert <eggert@cs.ucla.edu> writes:

> Thanks for catching my mistake. I installed the attached patch, which
> is closer to the code you originally suggested. With it, displaying
> one of the images you suggested looks like the attached image; hope
> it's what you're looking for. (I disabled font-lock-mode as you also
> suggested.)

The PNGs in question display well for me as well, so was the conclusion
here that this is now fixed?

Except possibly on Windows?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37153: 26.1; some png images scrambled

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Tue, 24 Sep 2019 18:23:58 +0200
> Cc: 37153@debbugs.gnu.org, Roland Winkler <winkler@gnu.org>
> 
> The PNGs in question display well for me as well, so was the conclusion
> here that this is now fixed?
> 
> Except possibly on Windows?

Yes, they don't display correctly on Windows.