On 2/1/2016 1:04 AM, Jayathirth D V wrote:
Hi Alex,
Since two separate bugs are created for the suggestions given by Jim.
And you are okay with the fix provided for the bug
https://bugs.openjdk.java.net/browse/JDK-8147413 under webrev
http://cr.openjdk.java.net/~jdv/8147413/webrev.00/
Can we push the fix into JDK9 after one more approval?
Two approvals are usually enough for the fix pushing.
Thanks,
Alexandr.
Thanks,
Jay
-----Original Message-----
From: Alexander Scherbatiy
Sent: Friday, January 22, 2016 5:05 PM
To: Jim Graham; Phil Race; Jayathirth D V
Cc: 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] Review request for JDK-8147413 :
api/java_awt/Image/MultiResolutionImage/index.html\#MultiResolutionRenderingHints[test_VALUE_RESOLUTION_VARIANT_BASE]
started to fail
I have created two separated issues on it:
8148045 Handle null resolution variant in SunGraphics2D
https://bugs.openjdk.java.net/browse/JDK-8148045
8148046 Investigate the case where MRI.getResolutionVariant() returns
MultiResolutionImage
https://bugs.openjdk.java.net/browse/JDK-8148046
Thanks,
Alexandr.
On 22/01/16 00:34, Jim Graham wrote:
BASE essentially just means to draw the 1:1 image. The problem is
that returning null is not the way to achieve that. Returning null
means "MRI doesn't apply here, just use the original image", but in
the case of BASE we have to make sure it isn't an MRI first otherwise
the caller will get an exception.
The change does achieve that since BASE handling is built in to the
helper getResolutionVariant() method in SG2D and so it should ask for
the 1:1 image, but if the getRV() helper method returns null then we
still return null from the drawHiDPIImage() method and that causes the
caller to use the original MRI in a normal-image pipeline. There
should perhaps be an else clause for the "if (rV != img && rV != null)
{ } else { ???; }". What that "???" is depends on why the helper
method returned the original image and/or a null, but it should not be
"return null;". (Also, there is no check if it returned another
different MRI. That should probably not be allowed, but we never state
that in the interface description...)
The remaining question is why the helper method would return either
the original image or null.
I don't think the helper method ever returns the original MRI
knowingly, but the MRI itself might do that. That would then fall
under the "what to do if the MRI returns any sort of non-standard
image" case, and perhaps the exception is appropriate there? We
should probably detect that, though, and throw a more obvious
exception (or recurse?). Recursion can get ugly, so perhaps a very
explicit error with a reasonable description would be better than
letting the DrawImage pipeline fail in various ways.
The helper function can return null if the operation is a NOP (sxy1 ==
sxy2 - i.e. an empty subimage rectangle), but that case should be
handled here and end up with returning ?true? which matches what many
of the drawImage() variants return when they detect a trivial NOP case.
The helper function can also return null if the srcWidth/Height are
negative which would imply an uninitialized image. I'm not sure what
to do there because normally an uninitialized image is tracked by the
DrawImage pipe and the observer is notified as the image data is
loaded, but we have no mechanism to do that with an MRI. This case
may require more work, or something explicit in the MRI spec (what
happens when someone lazily loads an image with an @x2 variant now?).
It also returns null if the return value from the MRI is a
ToolkitImage with an error - but I think that is wrong, it should just
return the ToolkitImage and let the caller send it to the DrawImage
pipeline to detect the error and respond in the appropriate manner.
Or perhaps it should cause the drawHiDPIImage() method to return
whatever boolean is appropriate for an error to save time, but there
is no way to orchestrate that from the helper method since it can only
return a (non-MRI) image. It's probably best to just return the
errored TKImage in that case...?
The last way it can return null is if the MRI returned null from its
getRV() method, but the interface doesn't discuss what that means. I
suppose it might mean that all of the resolution variants are being
loaded asynchronously and none of them are available yet, but we are
then missing a mechanism to inform the observer of when the variants
are loaded. (Is there a bugid for that? I seem to recall having a
discussion about that missing mechanism in the past...?)
So, I think the change made here is along the right track, but we have
a couple more holes to plug before we can consider this fixed...
...jim
On 1/20/2016 12:57 PM, Phil Race wrote:
It seems like the expectation was that BASE could be drawn by the old
imaging path.
which would be lower overhead.
I think we should ask Alexandr what the intention was here and
whether the code that handles the base image needs to be taught how
to extract data from a MultiResolutionImage.
-phil.
On 01/20/2016 08:20 AM, Jayathirth D V wrote:
Hi,
__
_Please review the following fix in JDK9:_
Bug : https://bugs.openjdk.java.net/browse/JDK-8147413
Webrev : http://cr.openjdk.java.net/~jdv/8147413/webrev.00/
<http://cr.openjdk.java.net/%7Ejdv/8147413/webrev.00/>
Issue : JCK testcase
api/java_awt/Image/MultiResolutionImage/index.html\#MultiResolutionR
enderingHints[test_VALUE_RESOLUTION_VARIANT_BASE]
is failing from b96 JDK9 build.
Root cause : In getManager API of SurfaceManager.java we are trying
to typecast BaseMultiResolutionImage to BufferedImage and it is
causing ClassCastException and in turn IllegalArgumentException seen
in result of test case. It is happening because of change made in
JDK-8073320 <https://bugs.openjdk.java.net/browse/JDK-8073320> in
SunGraphics2D.java. In case of VALUE_RESOLUTION_VARIANT_BASE type we
are not trying to convert MultiResolutionImage to BufferedImage.
Solution : Modify the condition present in drawHiDPIImage API to
convert all MultiResolutionImage to BufferedImage irrespective of
KEY_RESOLUTION_VARIANT type.
Thanks,
Jay
