<AWT Dev> [9] Review request for 8033534 Get MultiResolution image from native system

[Petr Pchelko]

Hello, Alexander.

In CImage.m:430 - Do we really want to describe and clear the exception? 
May be it's better to simply return NULL and let Java handle the exception?
This made sense when you were continuing the loop, but now doesn't.

With best regards. Petr.

On 04.03.2014, at 15:04, Alexander Scherbatiy <alexandr.scherbatiy at oracle.com> wrote:

> 
>  Hello,
> 
> Could you review the updated fix:
>   http://cr.openjdk.java.net/~alexsch/8033534/webrev.04
> 
>  - NULL is returned for all cases from the nativeGetNSImageRepresentationsCount method
>  - long lines are split
>  - SetObjectArrayElement can throw ArrayIndexOutOfBoundsException and ArrayStoreException exceptions.
>    We do not expect neither of them because the same length and type is used for the array creation and element settings.
>    I updated the exception handling to return NULL if an exception occurs.
> 
>  Thanks,
>  Alexandr.
> 
> 
> On 3/3/2014 11:48 PM, Sergey Bylokhov wrote:
>> Hi, Alexander.
>> nativeGetNSImageRepresentationsCount three times return different values in case of error (0, NULL, nil).
>> What exception we expect from the SetObjectArrayElement and why we continue in this case?
>> Also please split soooooooooooooo long lines in these files.
>> 
>> On 2/26/14 6:40 PM, Alexander Scherbatiy wrote:
>>> 
>>> Hello,
>>> 
>>>  Could you review the updated fix:
>>> http://cr.openjdk.java.net/~alexsch/8033534/webrev.03/
>>> 
>>> On 2/26/2014 4:54 PM, Petr Pchelko wrote:
>>>> Hello, Alexander.
>>>> 
>>>> I have a couple of comments:
>>>> 
>>>> 1. You could replace the first loop with indexOfObjectPassingTest method.. Not sure if this would look cleaner, up to you.
>>>    Updated. There is one more way to use one loop instead of two. All of them does not look simple.
>>> 
>>>>  2. I suppose JNFNewObjectArray could throw the OOM and we would get a parfait warning, could you please add CHECK_NULL_RETURN after it.
>>>    CHECK_NULL_RETURN is added.
>>>> 3. In CImage.java you are setting the currentImageIndex to the biggest image representation smaller that the one requested and this representation
>>>> would be used as a base one in the MultiResolutionBufferedImage. However in MultiResolutionBufferedImage getResolutionVariant you are returning
>>>> the smallest variant bigger than the requested one. Is this correct?
>>>    I think that it is correct. Assume that an image with size 300x300 is requested but there are only resolution variants with sizes [250x250] and [350x350].
>>>    The resolution variant with  [350x350] size is used as the base image.  Now we need to draw the image to region [300x300]. The resolution variant
>>>    with size [350x350] will be used from the MultiResolution image.
>>> 
>>>  Thanks,
>>>  Alexandr.
>>> 
>>> 
>>>> 
>>>> Thank you.
>>>> With best regards. Petr.
>>>> 
>>>> On 26.02.2014, at 16:08, Alexander Scherbatiy <alexandr.scherbatiy at oracle.com> wrote:
>>>> 
>>>>>  Hello,
>>>>> 
>>>>>  Could you review the updated fix:
>>>>> http://cr.openjdk.java.net/~alexsch/8033534/webrev.02/
>>>>> 
>>>>>  This is the same fix. The only difference is that the MultiResolutionBufferedImage class is used from the fix JDK-8035069.
>>>>> 
>>>>>  Thanks,
>>>>>  Alexandr.
>>>>> 
>>>>> 
>>>>> On 2/10/2014 7:05 PM, Scott Palmer wrote:
>>>>>> Just to be clear, "the image representations are chosen to be closest to the requested size" is not accurate.
>>>>>> This change returns the smallest image representation that is greater than or equal to the requested size.  (Which I believe is the correct thing to do.)
>>>>>> A smaller image representation may be closer to the requested size, but it makes more sense to return a larger image since scaling down to size should produce better results than scaling up.
>>>>>> 
>>>>>> Scott
>>>>>> 
>>>>>> 
>>>>>> On Mon, Feb 10, 2014 at 8:53 AM, Alexander Scherbatiy <alexandr.scherbatiy at oracle.com <mailto:alexandr.scherbatiy at oracle.com>> wrote:
>>>>>> 
>>>>>> 
>>>>>>    Could you review the updated fix:
>>>>>> http://cr.openjdk.java.net/~alexsch/8033534/webrev.01
>>>>>> <http://cr.openjdk.java.net/%7Ealexsch/8033534/webrev.01>
>>>>>> 
>>>>>>     - The image representations are chosen to be closest to the
>>>>>>    requested size.
>>>>>> 
>>>>>>      Thanks,
>>>>>>      Alexandr.
>>>>>> 
>>>>>> 
>>>>>>    On 2/4/2014 5:00 PM, Sergey Bylokhov wrote:
>>>>>> 
>>>>>>        Hi, Alexander.
>>>>>>        I think that getResolutionVariant should return an image which
>>>>>>        is close as much as possible to the requested size.
>>>>>> 
>>>>>>        On 04.02.2014 16:42, Alexander Scherbatiy wrote:
>>>>>> 
>>>>>> 
>>>>>>            Hello,
>>>>>> 
>>>>>>            Could you review the fix:
>>>>>>              bug: https://bugs.openjdk.java.net/browse/JDK-8033534
>>>>>>              webrev:
>>>>>> http://cr.openjdk.java.net/~alexsch/8033534/webrev.00
>>>>>> <http://cr.openjdk.java.net/%7Ealexsch/8033534/webrev.00>
>>>>>> 
>>>>>>            - The method that gets a sorted array of NSImage
>>>>>>            representaion pixel sizes for the given image size is added
>>>>>>            - A MultiResolution image is created if an NSImage has
>>>>>>            several representations for the given image size
>>>>>> 
>>>>>>            Thanks,
>>>>>>            Alexandr.
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>> 
>> 
>> 
>> -- 
>> Best regards, Sergey.
>