<AWT Dev> [8] Review request for 8028212 Custom cursor HiDPI support

[Sergey Bylokhov]

Hi, Alexander.
The fix looks good.
On 12/5/13 6:06 PM, Alexander Scherbatiy wrote:
>
>   Could you review the updated fix:
>     http://cr.openjdk.java.net/~alexsch/8028212/webrev.01/
>
>   - CImage.createFromImage() method is updated to handle 
> MultiResolutionImage
>   - resizeRepresentations() method is moved from Creator to the CImage 
> class
>   - nativeResizeImageRepresentations method is renamed to 
> nativeResizeNSImageRepresentations in the CImage class
>   - ImageReprese-M-tations typo is fixed in the CImage native code
>
>   Thanks,
>   Alexandr.
>
> On 12/4/2013 8:24 PM, Anthony Petrov wrote:
>> Thanks for the update, Alexander. I understand.
>>
>> So, my only concerns that remain are:
>>
>> 1. The membership of the resizeImageRepresentations() method.
>>
>> 2. The encapsulation of the logic handling the 
>> Image/MultiResolutionImage case (see at the bottom of my original 
>> message).
>>
>> -- 
>> best regards,
>> Anthony
>>
>> On 12/04/2013 06:59 PM, Alexander Scherbatiy wrote:
>>> On 12/3/2013 9:06 PM, Anthony Petrov wrote:
>>>> Hi Alexander,
>>>>
>>>> If we go with this fix, I suggest to move the
>>>> CImage.Creator.resizeImageRepresentations() to the CImage class and
>>>> make it a member method, so that you don't need to pass a CImage
>>>> reference to it as an argument.
>>>      I will update this.
>>>>
>>>> Also, there's the CImage.resize() method already. Why does it not work
>>>> for us? Having a specification for both methods (or for one, if the
>>>> second is unneeded) might be helpful.
>>>
>>>       I do not know why, but SetNSImageSize (  [i
>>> setScalesWhenResized:TRUE], [i setSize:NSMakeSize(w, h)]) just does not
>>> work in this case. The high resolution representation is not chosen on
>>>       my Mac OS X with enabled Quartz Debug.
>>>
>>>      I tried to narrow the problem and write the cocoa code. It is
>>> described in https://bugs.openjdk.java.net/browse/JDK-8028212
>>>
>>>     Thanks,
>>>     Alexandr.
>>>
>>>>
>>>> However, I'm not sure if we really want to resize each representation
>>>> of an NSImage object to the same size. Why would we want to do that?
>>>> In fact, one of the representations might already have the correct
>>>> size, and we could use just that whenever we need it w/o wasting
>>>> resources on resizing each of them. If there's no representations of
>>>> suitable size, then we should choose the closest one and resize just
>>>> it to the desired size. Or am I misunderstanding anything?
>>>>
>>>> Also, in CCustomCursor.getImageData(), could we somehow encapsulate a
>>>> part (or all) of the Image vs. MultiResolutionImage logic in the
>>>> CImage.Creator class?
>>>>
>>>> PS. I'm not really an expert in Image handling code. I'd suggest
>>>> someone from the 2D team to review this as well. Maybe Jim could help?
>>>>
>>>> -- 
>>>> best regards,
>>>> Anthony
>>>>
>>>> On 12/03/2013 08:32 PM, Alexander Scherbatiy wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> Could you review the fix:
>>>>>    bug: https://bugs.openjdk.java.net/browse/JDK-8028212
>>>>>    webrev: http://cr.openjdk.java.net/~alexsch/8028212/webrev.00
>>>>>
>>>>>    - MultiResolutionImage interface is used from the fix 8011059
>>>>>    - NSImage with representations are created for the 
>>>>> multi-resolution
>>>>> cursor
>>>>>    - NSImage representations are rescaled to the base cursor size
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>
>


-- 
Best regards, Sergey.