[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    [OpenJDK 2D-Dev] [PATCH] 4494651: Wrong width and height	in	BufferedImage GraphicsConfiguration obje
From:       Jim.A.Graham () Sun ! COM (Jim Graham)
Date:       2009-02-23 23:55:02
Message-ID: 0KFJ001VXMFLRSF0 () fe-sfbay-09 ! sun ! com
[Download RAW message or body]

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 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).

You should not be using this information to create BufferedImages, 
therefore I am not interested in how it would negatively impact your code.

> 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.

I can see some sense in that, but I think that max bounds or a 
definitive "the renderer cannot support bounds greater than X by Y" 
would make more sense.  I think an exception or error would make more 
sense if "no image" could be allocated, rather than "any image".

> 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. 

The fuss is:

- It wastes objects

- It does not match the spec for the method (which isn't very well 
documented, but the various code samples in the javadocs combined with 
the doc comment on the method collectively indicate that the bounds of a 
GC are not tied to a specific destination).

- It sets a precedence of returning information which seems useful from 
a context that was not designed to return such information.

- It takes a direction which is not only different from other existing 
uses of the same class (i.e. Component.getGC.getBounds()), but is also a 
direction that they cannot and will not follow, creating a schism in how 
these objects would be used.

- We've long needed an API that is specifically designed to return the 
information you want and we should create just such a specific API for 
that purpose, rather than to start side-effecting some other API to help 
out in some cases.

> I find it curious that all but the indexed image type configurations 
> are cached and it seems not be a problem there:

Indexed images are not typically used as rendering destinations so this 
method is much less likely to be called on them.  Inherent in their 
"configuration" is the need to match their colormodel, which depends on 
their color palette and so they cannot be cached.  It would be nice if 
they could be, but they can't.  The fact that they can't should not be 
interpreted as open season for removing all other caches, especially 
given the practical difference in how they tend to be used.

> 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?

The method returns the bounds of the *GraphicsConfiguration* in the 
device coordinates.  It does not return the bounds of a specific 
drawable object created on that configuration.  The value of the bounds 
it does return is useful for finding where to place a window (among 
other uses).  The value of the API docs are to explain to users wanting 
to perform that task how to do it.  The value of this method was never 
intended to be to explain how to allocate rendering buffers for a 
specific destination and so the value of these docs are not for 
explaining that usage.

>> 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.

But, if the renderer flat-out fails if any dimension is more than, say 
2^24 whether or not you've run out of memory, then that would be good 
information to have, no?

That information seems closer to the way it is used for screen 
components - "things larger than that created for this GC will exceed 
the maximum supported bounds of the device they live in" - where, in the 
case of BufferedImages, the device would be defined as the memory and 
dimensions within that memory that the software can correctly support 
rendering to.  It wouldn't be a guarantee that creating an image of 
those dimensions would succeed, but a warning that images of larger 
dimension won't work right.

> 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).

It must be taking some time to percolate to the external bug view...

			...jim


[prev in list] [next in list] [prev in thread] [next in thread]