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

[Alexander Scherbatiy]

   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.