Re: [OpenJDK 2D-Dev] [PATCH] 4494651: Wrong width and height in BufferedImage GraphicsConfiguration objects
Torsten Landschoff wrote: Hi Jim, On Tue, Feb 17, 2009 at 06:27:43PM -0800, Jim Graham wrote: The width and height of a GraphicsConfig is essentially irrelevant information. If you get the GraphicsConfig of a component, it doesn't Why is there a method then to query irrelevant information? The size of a GC is very relevant for screen GCs - it defines the size of the device that the component is being rendered on. Therefore the GC.getBounds method is somewhat useful for those GCs. That was the use case for which the method was created. The GCs from BufferedImages are another story. There is no real device associated with a BufferedImage and any virtual device that you can conceive for it doesn't necessarily have a size. The closest thing to a size of the destination device you might have would be as big as you have memory or the limits (if any) that the rendering engine's algorithms have. In particular, that method is not defined as returning the size of the thing it came from, it is defined as returning the size of the universe in/on which the thing it came from lives and the size of the BufferedImage is not a relevant or analogous piece of information. The return value of GC.getBounds() had a use in mind when it was created - it just doesn't happen to be the use that it looks like you want to make of it. tell you how big that component was, so why should the GC of a BufImg bear any relation to the dimensions of the image? Why would the graphics configuration contain the size of the component? It doesn't contain it. That is what I said and you quoted. The GC from a component is not tied to the size of the component. The GC from a BufferedImage should therefore not be tied to the size of the image. The use case for me was to fix a drawing routine which made two passes over the input data to have the second pass paint over the first. This turned out to be quite slow with the main time going into loading the data. So I optimized it to paint in one pass, by using a BufferedImage to paint the overlay data and merge it later. The only object that could give me the size of the output Graphics device was - surprise - the Graphics2D instance. This works fine when drawing directly to the screen, but unfortunately not when double buffering is used. I think this is a very valid use case. I think I see your case point here, but I think it is a red herring. You say that the G2D tells you the size of the output screen device. I suppose that is true, but that information does not tell you how big the component on that screen is. So, I suppose if you are rendering full screen then a by-product of the GC returning the size of the device is that it also tells you the size of the thing you are rendering to, but if you are rendering to just a window or component, that information is not useful since you really need to know the bounds of the component/widget/window and it doesn't give you that. So, a universally useful piece of information that tells you the size of the thing you are rendering to should come from somewhere other than the size of the GC in a Graphics2D. Note that we sort of provide that information in another form. When your paint(), or paintComponent() method is called, we have set the clip to the bounds of the component and so doing a Graphics.getClipBounds() will give you the information you want. Unfortunately, I don't think the clip is set if you call Component.getGraphics() or Image.getGraphics() and there is no relevant and related Graphics.getDeviceBounds(), though maybe there should be? Historically, the bounds of a GC of a BufferedImage has never reliably returned similar information, and I don't think attempting to overload that method on GC is a good way to start providing this information. Thus, I do not wish to pursue any further any attempts to make the GC a useful item for describing the dimensions of a particular Graphics2D's destination. If anything, I'd fix it by having it report some fixed bogus (positive, large) dimensions rather than the dimensions of the first image that it was created from... Why? Code calling it will only fall over with that, perhaps even worse than the status quo when expecting valid values. Why would such code fall over? What use have developers been making of this, essentially random (random for BufferedImages) data in the past? This information hasn't been useful in the past, so changing it will not break any correctly written programs. The bounds information for a GC has not been semantically tied to the dimensions of a Graphics2D destination in the past, but it has had cases where it coincidentally returned bounds that happened to meet that need (i.e. full screen rendering). The coincidence there was not by design and it cannot be extended to all cases so that accidental synergy of that one use case should not be turned into a specification. Using
Re: [OpenJDK 2D-Dev] [PATCH] 4494651: Wrong width and height in BufferedImage GraphicsConfiguration objects
Hi Dmitri, On Tue, Feb 17, 2009 at 06:13:47PM -0800, Dmitri Trembovetski wrote: Note that this fix may have some performance implications for cases when there are caches based on the graphics configuration of the Graphics object used for rendering - these caches may grow uncontrollably. I don't see how this could happen with either of my patches: a) The trivial approach will create a new GraphicsConfiguration instance on each call to Graphics2D#getDeviceConfiguration. Which I would expect as the JavaDoc states that it Returns the device configuration associated with this Graphics2D.. Reference: http://java.sun.com/javase/6/docs/api/java/awt/Graphics2D.html#getDeviceConfiguration() b) My second patch uses a WeakHashMap to cache the already retrieved graphics configuration instances. I would expect it to be faster for very few instances cached and slower when many instances are cached. So I don't think it's worth the effort. Anyway, assuming that the WeakHashMap will clean up left-over keys, this implementation should keep at most as many graphics configuration instances as there are Graphics2D instances referring to BufferedImages, for which getDeviceConfiguration was called. What am I missing? I don't know of any other solution for this bug, but I'm not sure fixing it would do more harm than good. The bug has only 3 votes after 8 years (all of them in 2001-2002).. Is it better to leave a known bug unfixed for fears of performance regressions? While this bug goes unnoticed for long time, actually finding it (even though it is recorded) can consume a lot of developer time. Greetings, Torsten
Re: [OpenJDK 2D-Dev] [PATCH] 4494651: Wrong width and height in BufferedImage GraphicsConfiguration objects
Hi Jim,
On Tue, Feb 17, 2009 at 06:27:43PM -0800, Jim Graham wrote:
The width and height of a GraphicsConfig is essentially irrelevant
information. If you get the GraphicsConfig of a component, it doesn't
Why is there a method then to query irrelevant information?
tell you how big that component was, so why should the GC of a BufImg
bear any relation to the dimensions of the image?
Why would the graphics configuration contain the size of the component?
Quoting the docs: The GraphicsConfiguration class describes the
characteristics of a graphics destination such as a printer or monitor.
Therefore I would expect the bounds to match the size of the screen
I am painting to, not the size of the component.
The use case for me was to fix a drawing routine which made two passes
over the input data to have the second pass paint over the first. This
turned out to be quite slow with the main time going into loading the
data. So I optimized it to paint in one pass, by using a BufferedImage
to paint the overlay data and merge it later. The only object that
could give me the size of the output Graphics device was - surprise -
the Graphics2D instance.
This works fine when drawing directly to the screen, but unfortunately
not when double buffering is used. I think this is a very valid use case.
If anything, I'd fix it by having it report some fixed bogus (positive,
large) dimensions rather than the dimensions of the first image that it
was created from...
Why? Code calling it will only fall over with that, perhaps even worse
than the status quo when expecting valid values.
Using negative dimensions for example would clearly mark the data as
invalid, but in my case would lead to IllegalArgumentException in the
BufferedImage constructor (as I take the sizes at face value).
If you really want to disable usable functionality in an update to the
JDK, I'd suggest to use UnsupportedOperationException so that the
caller at least knows what is going on.
Anyway, I don't understand the fuss about such a simple code change.
Maybe it will eat some cycles and a bit of memory, but there is not
even a comment in the original BufferedImageGraphicsConfig code why
that hack there is really needed.
I find it curious that all but the indexed image type configurations
are cached and it seems not be a problem there:
47 private static final int numconfigs = BufferedImage.TYPE_BYTE_BINARY;
== the highest image type is public static final int TYPE_BYTE_INDEXED =
13;
48 private static BufferedImageGraphicsConfig configs[] =
49 new BufferedImageGraphicsConfig[numconfigs];
50
51 public static BufferedImageGraphicsConfig getConfig(BufferedImage bImg) {
52 BufferedImageGraphicsConfig ret;
53 int type = bImg.getType();
54 if (type 0 type numconfigs) {
55 ret = configs[type];
56 if (ret != null) {
On Tue, Feb 17, 2009 at 06:30:36PM -0800, Jim Graham wrote:
Note that, per my previous email - this could easily be just closed as
not a bug, though it might be nice to change it so the bounds were
completely unrelated to any specific image rather than randomly set to
the first image that was queried.
The Java API docs state that getBounds() will return the bounds in device
coordinates. You say that information is irrelevant, so it is not a bug?
What value do the API docs have then?
In other words, I'd rewrite the bug synopsis as something like:
GC reports random bounds (that happen to match first img)
and then hardcode some interesting bounds instead (perhaps the maximum
supported image bounds - which we've never implemented or documented,
but perhaps it would be a good way to introduce this concept for the
future?)...
I think that would be a slippery slope as this could depend on the amount of
virtual memory - which is hard to measure, and even then nobody would like
to have the crawling speed.
On Tue, Feb 17, 2009 at 06:39:44PM -0800, Jim Graham wrote:
In fact, I've just updated the synopsis and evaluated the bug along
these lines... ;-)
Where can I read the updated bug entry? The URL known to me is
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4494651
and its content did not change (apart from some CSS hickups the last
few days).
Greetings, Torsten
Re: [OpenJDK 2D-Dev] [PATCH] 4494651: Wrong width and height in BufferedImage GraphicsConfiguration objects
Hi Torsten, Thank you for submitting a patch. We need you to sign Sun Contributor Agreement before we can start working on your patch. The SCA information can be found here: http://openjdk.java.net/contribute/ Once we get the confirmation we will give you an update on the patch. Sincerely, Jennifer Torsten Landschoff wrote: 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
Re: [OpenJDK 2D-Dev] [PATCH] 4494651: Wrong width and height in BufferedImage GraphicsConfiguration objects
Hi Jennifer, On Tue, Feb 03, 2009 at 09:25:28AM -0800, Jennifer Godinez wrote: Thank you for submitting a patch. We need you to sign Sun Contributor Agreement before we can start working on your patch. The SCA information can be found here: http://openjdk.java.net/contribute/ Thanks. I just signed the SCA and sent it as a facsimile to the number listed there. Once we get the confirmation we will give you an update on the patch. Please note that I will be on vacation starting tomorrow up to sunday. So please excuse missing feedback from my side. Greetings, Torsten
