8357034: GifImageDecoder can produce wrong transparent pixels

Reviewed-by: jdv, prr

src/java.desktop/share/classes/sun/awt/image/GifImageDecoder.java

@@ -343,6 +343,7 @@ public class GifImageDecoder extends ImageDecoder {
     private short[] prefix  = new short[4096];
     private byte[]  suffix  = new byte[4096];
     private byte[]  outCode = new byte[4097];
+    private boolean isSavedModelReliable = true;
 
     private static native void initIDs();
 
@@ -396,7 +397,7 @@ public class GifImageDecoder extends ImageDecoder {
         int off = y * global_width + x2;
         boolean save = (curframe.disposal_method == GifFrame.DISPOSAL_SAVE);
         if (trans_pixel >= 0 && !curframe.initialframe) {
-            if (saved_image != null && model.equals(saved_model)) {
+            if (saved_image != null && model.equals(saved_model) && isSavedModelReliable) {
                 for (int i = rasbeg; i < rasend; i++, off++) {
                     byte pixel = rasline[i];
                     if ((pixel & 0xff) == trans_pixel) {
@@ -406,6 +407,8 @@ public class GifImageDecoder extends ImageDecoder {
                     }
                 }
             } else {
+                isSavedModelReliable = false;
+
                 // We have to do this the hard way - only transmit
                 // the non-transparent sections of the line...
                 // Fix for 6301050: the interlacing is ignored in this case
@@ -597,6 +600,7 @@ public class GifImageDecoder extends ImageDecoder {
             }
             return false;
         }
+
         boolean ret = parseImage(x, y, width, height,
                                  interlace, initCodeSize,
                                  block, rasline, model);

test/jdk/sun/awt/image/gif/GifBuilder.java

@@ -65,12 +65,16 @@ public class GifBuilder {
     /**
      * This creates a sample gif image based on a series of FrameDescriptions,
      * and the calls {@link GifComparison#run(URL)}
+     *
+     * @param frameDir an optional directory to write all frames as PNGs to.
+     *                 See {@link GifComparison#run(URL, File)}
      */
-    public static void test(FrameDescription... frameDescriptions)
+    public static void test(FrameDescription[] frameDescriptions,
+                            File frameDir)
             throws Throwable {
         File file = createTestFile(frameDescriptions);
         try {
-            GifComparison.run(file.toURI().toURL());
+            GifComparison.run(file.toURI().toURL(), frameDir);
         } finally {
             file.delete();
         }

test/jdk/sun/awt/image/gif/GifComparison.java

@@ -38,6 +38,7 @@ import java.awt.image.BufferedImage;
 import java.awt.image.ColorModel;
 import java.awt.image.ImageConsumer;
 import java.awt.image.IndexColorModel;
+import java.io.File;
 import java.net.URL;
 import java.util.Hashtable;
 import java.util.LinkedList;
@@ -59,13 +60,21 @@ public class GifComparison {
      * if ImageIO and ToolkitImage produce different BufferedImage renderings.
      *
      * @param srcURL the URL of the image to inspect
+     * @param frameDir an optional directory to write frames to as PNG images.
+     *                 The frames should render identically whether we use
+     *                 the ImageIO model or the ToolkitImage model. If they're
+     *                 identical, then we only output one image, such as
+     *                 "frame_0.png". If they're different then we'll
+     *                 output two images: "frame_0_iio.png" and
+     *                 "frame_0_awt.png".
      *
      * @return the last frame encoded as a TYPE_INT_ARGB image.
      *         <p>
      *         Unit tests may further inspect this image to make sure certain
      *         conditions are met.
      */
-    public static BufferedImage run(URL srcURL) throws Throwable {
+    public static BufferedImage run(URL srcURL, File frameDir)
+            throws Throwable {
         System.out.println("Comparing ImageIO vs ToolkitImage rendering of " +
                 srcURL);
         ImageIOModel ioModel = new ImageIOModel(srcURL);
@@ -73,41 +82,67 @@ public class GifComparison {
 
         BufferedImage lastImage = null;
 
-        int a = ioModel.frames.size() - 1;
-        BufferedImage ioImg = ioModel.getFrame(a);
-        BufferedImage awtImage = awtModel.getFrame(a);
+        // if frameDir exists: test & export all frames.
+        // Otherwise: only test the last frame
+        int startIndex = frameDir == null ? ioModel.frames.size() - 1 : 0;
 
-        lastImage = awtImage;
+        for (int a = startIndex; a < ioModel.frames.size(); a++) {
+            BufferedImage ioImg = ioModel.getFrame(a);
+            BufferedImage awtImage = awtModel.getFrame(a);
 
-        if (!(ioImg.getWidth() == awtImage.getWidth() &&
-                ioImg.getHeight() == awtImage.getHeight()))
-            throw new Error("These images are not the same size: " +
-                    ioImg.getWidth() + "x" + ioImg.getHeight() + " vs " +
-                    awtImage.getWidth() + "x" + awtImage.getHeight());
+            lastImage = awtImage;
 
-        for (int y = 0; y < ioImg.getHeight(); y++) {
-            for (int x = 0; x < ioImg.getWidth(); x++) {
-                int argb1 = ioImg.getRGB(x, y);
-                int argb2 = awtImage.getRGB(x, y);
+            try {
+                if (!(ioImg.getWidth() == awtImage.getWidth() &&
+                        ioImg.getHeight() == awtImage.getHeight()))
+                    throw new Error("These images are not the same size: " +
+                            ioImg.getWidth() + "x" + ioImg.getHeight() +
+                            " vs " +
+                            awtImage.getWidth() + "x" + awtImage.getHeight());
 
-                int alpha1 = (argb1 & 0xff000000) >> 24;
-                int alpha2 = (argb2 & 0xff000000) >> 24;
-                if (alpha1 == 0 && alpha2 == 0) {
-                    continue;
-                } else if (alpha1 == 0 || alpha2 == 0) {
-                    throw new Error("pixels at (" + x + ", " + y +
-                            ") have different opacities: " +
-                            Integer.toUnsignedString(argb1, 16) + " vs " +
-                            Integer.toUnsignedString(argb2, 16));
+                for (int y = 0; y < ioImg.getHeight(); y++) {
+                    for (int x = 0; x < ioImg.getWidth(); x++) {
+                        int argb1 = ioImg.getRGB(x, y);
+                        int argb2 = awtImage.getRGB(x, y);
+
+                        int alpha1 = (argb1 & 0xff000000) >> 24;
+                        int alpha2 = (argb2 & 0xff000000) >> 24;
+                        if (alpha1 == 0 && alpha2 == 0) {
+                            continue;
+                        } else if (alpha1 == 0 || alpha2 == 0) {
+                            throw new Error("pixels at (" + x + ", " + y +
+                                    ") have different opacities: " +
+                                    Integer.toUnsignedString(argb1, 16) +
+                                    " vs " +
+                                    Integer.toUnsignedString(argb2, 16));
+                        }
+                        int rgb1 = argb1 & 0xffffff;
+                        int rgb2 = argb2 & 0xffffff;
+                        if (rgb1 != rgb2) {
+                            throw new Error("pixels at (" + x + ", " + y +
+                                    ") have different opaque RGB values: " +
+                                    Integer.toUnsignedString(rgb1, 16) +
+                                    " vs " +
+                                    Integer.toUnsignedString(rgb2, 16));
+                        }
+                    }
                 }
-                int rgb1 = argb1 & 0xffffff;
-                int rgb2 = argb2 & 0xffffff;
-                if (rgb1 != rgb2) {
-                    throw new Error("pixels at (" + x + ", " + y +
-                            ") have different opaque RGB values: " +
-                            Integer.toUnsignedString(rgb1, 16) + " vs " +
-                            Integer.toUnsignedString(rgb2, 16));
+
+                if (frameDir != null) {
+                    // the two models are identical, so simply write one image:
+                    File pngFile = new File(frameDir, "frame_" + a + ".png");
+                    ImageIO.write(ioImg, "png", pngFile);
+                    System.out.println("\tWrote " + pngFile);
                 }
+            } catch (Throwable t) {
+                if (frameDir != null) {
+                    File f1 = new File(frameDir, "frame_" + + a + "_iio.png");
+                    File f2 = new File(frameDir, "frame_" + + a + "_awt.png");
+                    ImageIO.write(ioImg, "png", f1);
+                    ImageIO.write(awtImage, "png", f2);
+                    System.out.println("\tWrote " + f1 + " vs " + f2);
+                }
+                throw t;
             }
         }
         System.out.println("Passed");

test/jdk/sun/awt/image/gif/GifEmptyBackgroundTest.java

@@ -28,14 +28,26 @@
  * the disposal method changes from 2 to 1
  */
 
+import java.io.File;
+
 public class GifEmptyBackgroundTest {
     public static void main(String[] args) throws Throwable {
-        GifBuilder.test(
+        GifBuilder.FrameDescription[] frames =
+            new GifBuilder.FrameDescription[] {
                 new GifBuilder.FrameDescription(
                         GifBuilder.Disposal.restoreToBackgroundColor, false),
                 new GifBuilder.FrameDescription(
                         GifBuilder.Disposal.doNotDispose, false),
                 new GifBuilder.FrameDescription(
-                        GifBuilder.Disposal.doNotDispose, false) );
+                        GifBuilder.Disposal.doNotDispose, false)
+        };
+
+        File dir = null;
+
+        // un-comment to visually inspect the frames:
+//        dir = new File("8356137-frames");
+//        dir.mkdir();
+
+        GifBuilder.test(frames, dir);
     }
 }

test/jdk/sun/awt/image/gif/GifSavedImageTransparentTest.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8357034
+ * @summary This test verifies that when the transparent pixel index changes
+ * and we're rendering on top of another frame we respect the new transparency.
+ */
+
+import java.io.File;
+
+public class GifSavedImageTransparentTest {
+    public static void main(String[] args) throws Throwable {
+        GifBuilder.FrameDescription[] frames =
+                new GifBuilder.FrameDescription[] {
+                        new GifBuilder.FrameDescription(
+                                GifBuilder.Disposal.doNotDispose, false),
+                        new GifBuilder.FrameDescription(
+                                GifBuilder.Disposal.doNotDispose, true),
+                        new GifBuilder.FrameDescription(
+                                GifBuilder.Disposal.doNotDispose, true)
+                };
+
+        File dir = null;
+
+        // un-comment to visually inspect the frames:
+//        dir = new File("8357034-frames");
+//        dir.mkdir();
+
+        GifBuilder.test(frames, dir);
+    }
+}