Hello List,
A few days before I ran into bug 4494651, after more than 7 years of its
existence. See
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4494651
I was curious how hard it would be to build a fixed OpenJDK, so I spent
half of the saturday to get OpenJDK build and its unit tests running.
Turned out that it wasn't that hard so here are my changes for you to
consider.
The first diff contains a trivial unit test derived from the example code
in the bug log. The code should speak for itself.
Tracing the bug to its origin, I found the interesting lookup table in
BufferedImageGraphicsConfig.java which maps each imageType supported by
BufferedImage to a BufferedImageGraphicsConfig. I have no idea why that
would be a good idea (apart from performance), so the second diff just
removes that feature and creates a new graphics configuration each time
it is requested.
As the only reason why one would create such a lookup table would be to
get around performance problems, I wrote a third patch which caches
the graphics configurations already retrieved in a weak hash map.
Comments welcome. Feel free to apply my code to the OpenJDK code base,
I'd be glad to sign the Sun Contributor Agreement if needed for such
a trivial contribution.
Thanks for all your work, I really enjoy working with Java (and even
more with its virtual machine and great libraries).
Greetings, Torsten
# HG changeset patch
# User Torsten Landschoff <tors...@landschoff.net>
# Date 1233522878 -3600
# Node ID 353e6a9b4077f40a2f4f252c1e8a617e8f1d5b12
# Parent 2113813eda623efbf130b7eaf6f2473d5dca4b5b
Add a test case for bug 4494651: Checks that the device configuration
from a BufferedImage matches the image width and height.
diff --git a/test/java/awt/image/BufferedImage/Bug4494651.java b/test/java/awt/image/BufferedImage/Bug4494651.java
new file mode 100644
--- /dev/null
+++ b/test/java/awt/image/BufferedImage/Bug4494651.java
@@ -0,0 +1,88 @@
+/*
+ * Copyright 2008 Sun Microsystems, Inc. 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 Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/* @test
+ * @bug 4494651
+ * @summary Checks that the graphics configuration of a BufferedImage
+ * reports the bounds of the associated image.
+ * @author Torsten Landschoff
+ */
+
+import java.awt.*;
+import java.awt.image.*;
+
+public class Bug4494651 {
+ public static void main(String[] args) throws Exception {
+ final Bug4494651 test = new Bug4494651();
+ test.run();
+ }
+
+ final static int imageType = BufferedImage.TYPE_INT_ARGB;
+
+ int passed, failed;
+
+ private void run() {
+ test();
+ reportResults();
+ }
+
+ private void test() {
+ BufferedImage img1 = new BufferedImage(100, 10, imageType);
+ BufferedImage img2 = new BufferedImage(200, 20, imageType);
+ Graphics2D gr1 = img1.createGraphics();
+ Graphics2D gr2 = img2.createGraphics();
+ GraphicsConfiguration gc1 = gr1.getDeviceConfiguration();
+ GraphicsConfiguration gc2 = gr2.getDeviceConfiguration();
+ check(img1.getWidth() == gc1.getBounds().getWidth(),
+ "width of image1 should match width of its device config");
+ check(img1.getHeight() == gc1.getBounds().getHeight(),
+ "height of image1 should match width of its device config");
+ check(img2.getWidth() == gc2.getBounds().getWidth(),
+ "width of image2 should match width of its device config");
+ check(img2.getHeight() == gc2.getBounds().getHeight(),
+ "height of image2 should match width of its device config");
+ check(gr1 != gr2,
+ "Distinct image instances need different graphics devices.");
+ check(gc1 != gc2,
+ "Devices with different sizes need distinct " +
+ "device configuration instances.");
+ }
+
+ private void check(boolean condition, String message) {
+ if (condition) {
+ System.out.println("Pass: " + message);
+ passed++;
+ } else {
+ System.out.println("FAIL: " + message);
+ failed++;
+ }
+ }
+
+ private void reportResults() {
+ System.out.printf("%n%nFailed = %d, passed = %d.%n", failed, passed);
+ if (failed > 0) {
+ throw new RuntimeException("Failed " + failed + " tests.");
+ }
+ }
+
+}
# HG changeset patch
# User Torsten Landschoff <tors...@landschoff.net>
# Date 1233522878 -3600
# Node ID 7ef5c9af95441b7c6ed1ef5c029c38226f3b02ce
# Parent 353e6a9b4077f40a2f4f252c1e8a617e8f1d5b12
Trivial patch for bug 4494651: Just create a new graphics configuration each
time it is queried from a BufferedImage instance. Possibly comes with a
performance penalty.
diff --git a/src/share/classes/sun/awt/image/BufferedImageGraphicsConfig.java b/src/share/classes/sun/awt/image/BufferedImageGraphicsConfig.java
--- a/src/share/classes/sun/awt/image/BufferedImageGraphicsConfig.java
+++ b/src/share/classes/sun/awt/image/BufferedImageGraphicsConfig.java
@@ -25,12 +25,10 @@
package sun.awt.image;
-import java.awt.AWTException;
import java.awt.Component;
import java.awt.Graphics2D;
import java.awt.GraphicsConfiguration;
import java.awt.GraphicsDevice;
-import java.awt.ImageCapabilities;
import java.awt.Rectangle;
import java.awt.Transparency;
import java.awt.geom.AffineTransform;
@@ -38,30 +36,24 @@
import java.awt.image.ColorModel;
import java.awt.image.DirectColorModel;
import java.awt.image.Raster;
-import java.awt.image.VolatileImage;
import java.awt.image.WritableRaster;
public class BufferedImageGraphicsConfig
extends GraphicsConfiguration
{
- private static final int numconfigs = BufferedImage.TYPE_BYTE_BINARY;
- private static BufferedImageGraphicsConfig configs[] =
- new BufferedImageGraphicsConfig[numconfigs];
-
+ /**
+ * Retrieve a graphics configuration matching a given buffered image.
+ * <p>
+ * For some reason this originally stored only one instance per
+ * imageType (see {...@link BufferedImage#BufferedImage(int, int, int)}).
+ * Perhaps that feature was intended as optimization, but this was the
+ * reason for bug 4494651. This trivial implementation now always
+ * returns a new graphics configuration for each request.
+ * @param bImg BufferedImage whose graphics configuration to get
+ * @return matching GraphicsConfiguration
+ */
public static BufferedImageGraphicsConfig getConfig(BufferedImage bImg) {
- BufferedImageGraphicsConfig ret;
- int type = bImg.getType();
- if (type > 0 && type < numconfigs) {
- ret = configs[type];
- if (ret != null) {
- return ret;
- }
- }
- ret = new BufferedImageGraphicsConfig(bImg, null);
- if (type > 0 && type < numconfigs) {
- configs[type] = ret;
- }
- return ret;
+ return new BufferedImageGraphicsConfig(bImg, null);
}
GraphicsDevice gd;
# HG changeset patch
# User Torsten Landschoff <tors...@landschoff.net>
# Date 1233522879 -3600
# Node ID 38c6db113eb978b9dcbd1e4ea423de0bf5bf8f4f
# Parent 7ef5c9af95441b7c6ed1ef5c029c38226f3b02ce
An attempt to eliminate the (possible) performance penalty of recreating a
graphics configuration on each getDeviceConfiguration call. Caches the
already created configuration objects.
diff --git a/src/share/classes/sun/awt/image/BufferedImageGraphicsConfig.java b/src/share/classes/sun/awt/image/BufferedImageGraphicsConfig.java
--- a/src/share/classes/sun/awt/image/BufferedImageGraphicsConfig.java
+++ b/src/share/classes/sun/awt/image/BufferedImageGraphicsConfig.java
@@ -37,23 +37,38 @@
import java.awt.image.DirectColorModel;
import java.awt.image.Raster;
import java.awt.image.WritableRaster;
+import java.util.WeakHashMap;
public class BufferedImageGraphicsConfig
extends GraphicsConfiguration
{
/**
+ * Maps an image to its graphics configuration when already available.
+ * This is used to reduce creation of the configuration instances.
+ */
+ static final WeakHashMap<BufferedImage, BufferedImageGraphicsConfig>
+ imageToConfigMap = new WeakHashMap<
+ BufferedImage, BufferedImageGraphicsConfig>();
+
+ /**
* Retrieve a graphics configuration matching a given buffered image.
* <p>
* For some reason this originally stored only one instance per
* imageType (see {...@link BufferedImage#BufferedImage(int, int, int)}).
* Perhaps that feature was intended as optimization, but this was the
- * reason for bug 4494651. This trivial implementation now always
- * returns a new graphics configuration for each request.
+ * reason for bug 4494651. This variant tries to use a cache of already
+ * created graphics configuration to avoid a performance regression.
+ *
* @param bImg BufferedImage whose graphics configuration to get
* @return matching GraphicsConfiguration
*/
public static BufferedImageGraphicsConfig getConfig(BufferedImage bImg) {
- return new BufferedImageGraphicsConfig(bImg, null);
+ BufferedImageGraphicsConfig config = imageToConfigMap.get(bImg);
+ if (config == null) {
+ config = new BufferedImageGraphicsConfig(bImg, null);
+ imageToConfigMap.put(bImg, config);
+ }
+ return config;
}
GraphicsDevice gd;
