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\#MultiResolutionRenderingHints[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
