Skip to content

Commit acc8a76

Browse files
Jeremy Woodprrace
Jeremy Wood
authored and
prrace
committed
8357034: GifImageDecoder can produce wrong transparent pixels
Reviewed-by: jdv, prr
1 parent 5a37374 commit acc8a76

File tree

5 files changed

+147
-39
lines changed

5 files changed

+147
-39
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,7 @@ private void readHeader() throws IOException, ImageFormatException {
343343
private short[] prefix = new short[4096];
344344
private byte[] suffix = new byte[4096];
345345
private byte[] outCode = new byte[4097];
346+
private boolean isSavedModelReliable = true;
346347

347348
private static native void initIDs();
348349

@@ -396,7 +397,7 @@ private int sendPixels(int x, int y, int width, int height,
396397
int off = y * global_width + x2;
397398
boolean save = (curframe.disposal_method == GifFrame.DISPOSAL_SAVE);
398399
if (trans_pixel >= 0 && !curframe.initialframe) {
399-
if (saved_image != null && model.equals(saved_model)) {
400+
if (saved_image != null && model.equals(saved_model) && isSavedModelReliable) {
400401
for (int i = rasbeg; i < rasend; i++, off++) {
401402
byte pixel = rasline[i];
402403
if ((pixel & 0xff) == trans_pixel) {
@@ -406,6 +407,8 @@ private int sendPixels(int x, int y, int width, int height,
406407
}
407408
}
408409
} else {
410+
isSavedModelReliable = false;
411+
409412
// We have to do this the hard way - only transmit
410413
// the non-transparent sections of the line...
411414
// Fix for 6301050: the interlacing is ignored in this case
@@ -597,6 +600,7 @@ private boolean readImage(boolean first, int disposal_method, int delay)
597600
}
598601
return false;
599602
}
603+
600604
boolean ret = parseImage(x, y, width, height,
601605
interlace, initCodeSize,
602606
block, rasline, model);

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,16 @@ public record FrameDescription(Disposal disposal, boolean
6565
/**
6666
* This creates a sample gif image based on a series of FrameDescriptions,
6767
* and the calls {@link GifComparison#run(URL)}
68+
*
69+
* @param frameDir an optional directory to write all frames as PNGs to.
70+
* See {@link GifComparison#run(URL, File)}
6871
*/
69-
public static void test(FrameDescription... frameDescriptions)
72+
public static void test(FrameDescription[] frameDescriptions,
73+
File frameDir)
7074
throws Throwable {
7175
File file = createTestFile(frameDescriptions);
7276
try {
73-
GifComparison.run(file.toURI().toURL());
77+
GifComparison.run(file.toURI().toURL(), frameDir);
7478
} finally {
7579
file.delete();
7680
}

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

Lines changed: 69 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import java.awt.image.ColorModel;
3939
import java.awt.image.ImageConsumer;
4040
import java.awt.image.IndexColorModel;
41+
import java.io.File;
4142
import java.net.URL;
4243
import java.util.Hashtable;
4344
import java.util.LinkedList;
@@ -59,55 +60,89 @@ public class GifComparison {
5960
* if ImageIO and ToolkitImage produce different BufferedImage renderings.
6061
*
6162
* @param srcURL the URL of the image to inspect
63+
* @param frameDir an optional directory to write frames to as PNG images.
64+
* The frames should render identically whether we use
65+
* the ImageIO model or the ToolkitImage model. If they're
66+
* identical, then we only output one image, such as
67+
* "frame_0.png". If they're different then we'll
68+
* output two images: "frame_0_iio.png" and
69+
* "frame_0_awt.png".
6270
*
6371
* @return the last frame encoded as a TYPE_INT_ARGB image.
6472
* <p>
6573
* Unit tests may further inspect this image to make sure certain
6674
* conditions are met.
6775
*/
68-
public static BufferedImage run(URL srcURL) throws Throwable {
76+
public static BufferedImage run(URL srcURL, File frameDir)
77+
throws Throwable {
6978
System.out.println("Comparing ImageIO vs ToolkitImage rendering of " +
7079
srcURL);
7180
ImageIOModel ioModel = new ImageIOModel(srcURL);
7281
AWTModel awtModel = new AWTModel(srcURL);
7382

7483
BufferedImage lastImage = null;
7584

76-
int a = ioModel.frames.size() - 1;
77-
BufferedImage ioImg = ioModel.getFrame(a);
78-
BufferedImage awtImage = awtModel.getFrame(a);
79-
80-
lastImage = awtImage;
81-
82-
if (!(ioImg.getWidth() == awtImage.getWidth() &&
83-
ioImg.getHeight() == awtImage.getHeight()))
84-
throw new Error("These images are not the same size: " +
85-
ioImg.getWidth() + "x" + ioImg.getHeight() + " vs " +
86-
awtImage.getWidth() + "x" + awtImage.getHeight());
87-
88-
for (int y = 0; y < ioImg.getHeight(); y++) {
89-
for (int x = 0; x < ioImg.getWidth(); x++) {
90-
int argb1 = ioImg.getRGB(x, y);
91-
int argb2 = awtImage.getRGB(x, y);
92-
93-
int alpha1 = (argb1 & 0xff000000) >> 24;
94-
int alpha2 = (argb2 & 0xff000000) >> 24;
95-
if (alpha1 == 0 && alpha2 == 0) {
96-
continue;
97-
} else if (alpha1 == 0 || alpha2 == 0) {
98-
throw new Error("pixels at (" + x + ", " + y +
99-
") have different opacities: " +
100-
Integer.toUnsignedString(argb1, 16) + " vs " +
101-
Integer.toUnsignedString(argb2, 16));
85+
// if frameDir exists: test & export all frames.
86+
// Otherwise: only test the last frame
87+
int startIndex = frameDir == null ? ioModel.frames.size() - 1 : 0;
88+
89+
for (int a = startIndex; a < ioModel.frames.size(); a++) {
90+
BufferedImage ioImg = ioModel.getFrame(a);
91+
BufferedImage awtImage = awtModel.getFrame(a);
92+
93+
lastImage = awtImage;
94+
95+
try {
96+
if (!(ioImg.getWidth() == awtImage.getWidth() &&
97+
ioImg.getHeight() == awtImage.getHeight()))
98+
throw new Error("These images are not the same size: " +
99+
ioImg.getWidth() + "x" + ioImg.getHeight() +
100+
" vs " +
101+
awtImage.getWidth() + "x" + awtImage.getHeight());
102+
103+
for (int y = 0; y < ioImg.getHeight(); y++) {
104+
for (int x = 0; x < ioImg.getWidth(); x++) {
105+
int argb1 = ioImg.getRGB(x, y);
106+
int argb2 = awtImage.getRGB(x, y);
107+
108+
int alpha1 = (argb1 & 0xff000000) >> 24;
109+
int alpha2 = (argb2 & 0xff000000) >> 24;
110+
if (alpha1 == 0 && alpha2 == 0) {
111+
continue;
112+
} else if (alpha1 == 0 || alpha2 == 0) {
113+
throw new Error("pixels at (" + x + ", " + y +
114+
") have different opacities: " +
115+
Integer.toUnsignedString(argb1, 16) +
116+
" vs " +
117+
Integer.toUnsignedString(argb2, 16));
118+
}
119+
int rgb1 = argb1 & 0xffffff;
120+
int rgb2 = argb2 & 0xffffff;
121+
if (rgb1 != rgb2) {
122+
throw new Error("pixels at (" + x + ", " + y +
123+
") have different opaque RGB values: " +
124+
Integer.toUnsignedString(rgb1, 16) +
125+
" vs " +
126+
Integer.toUnsignedString(rgb2, 16));
127+
}
128+
}
129+
}
130+
131+
if (frameDir != null) {
132+
// the two models are identical, so simply write one image:
133+
File pngFile = new File(frameDir, "frame_" + a + ".png");
134+
ImageIO.write(ioImg, "png", pngFile);
135+
System.out.println("\tWrote " + pngFile);
102136
}
103-
int rgb1 = argb1 & 0xffffff;
104-
int rgb2 = argb2 & 0xffffff;
105-
if (rgb1 != rgb2) {
106-
throw new Error("pixels at (" + x + ", " + y +
107-
") have different opaque RGB values: " +
108-
Integer.toUnsignedString(rgb1, 16) + " vs " +
109-
Integer.toUnsignedString(rgb2, 16));
137+
} catch (Throwable t) {
138+
if (frameDir != null) {
139+
File f1 = new File(frameDir, "frame_" + + a + "_iio.png");
140+
File f2 = new File(frameDir, "frame_" + + a + "_awt.png");
141+
ImageIO.write(ioImg, "png", f1);
142+
ImageIO.write(awtImage, "png", f2);
143+
System.out.println("\tWrote " + f1 + " vs " + f2);
110144
}
145+
throw t;
111146
}
112147
}
113148
System.out.println("Passed");

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,26 @@
2828
* the disposal method changes from 2 to 1
2929
*/
3030

31+
import java.io.File;
32+
3133
public class GifEmptyBackgroundTest {
3234
public static void main(String[] args) throws Throwable {
33-
GifBuilder.test(
35+
GifBuilder.FrameDescription[] frames =
36+
new GifBuilder.FrameDescription[] {
3437
new GifBuilder.FrameDescription(
3538
GifBuilder.Disposal.restoreToBackgroundColor, false),
3639
new GifBuilder.FrameDescription(
3740
GifBuilder.Disposal.doNotDispose, false),
3841
new GifBuilder.FrameDescription(
39-
GifBuilder.Disposal.doNotDispose, false) );
42+
GifBuilder.Disposal.doNotDispose, false)
43+
};
44+
45+
File dir = null;
46+
47+
// un-comment to visually inspect the frames:
48+
// dir = new File("8356137-frames");
49+
// dir.mkdir();
50+
51+
GifBuilder.test(frames, dir);
4052
}
4153
}

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/*
25+
* @test
26+
* @bug 8357034
27+
* @summary This test verifies that when the transparent pixel index changes
28+
* and we're rendering on top of another frame we respect the new transparency.
29+
*/
30+
31+
import java.io.File;
32+
33+
public class GifSavedImageTransparentTest {
34+
public static void main(String[] args) throws Throwable {
35+
GifBuilder.FrameDescription[] frames =
36+
new GifBuilder.FrameDescription[] {
37+
new GifBuilder.FrameDescription(
38+
GifBuilder.Disposal.doNotDispose, false),
39+
new GifBuilder.FrameDescription(
40+
GifBuilder.Disposal.doNotDispose, true),
41+
new GifBuilder.FrameDescription(
42+
GifBuilder.Disposal.doNotDispose, true)
43+
};
44+
45+
File dir = null;
46+
47+
// un-comment to visually inspect the frames:
48+
// dir = new File("8357034-frames");
49+
// dir.mkdir();
50+
51+
GifBuilder.test(frames, dir);
52+
}
53+
}

0 commit comments

Comments
 (0)