<Swing Dev> [9] Review request for 8164032 JViewport backing store image is not scaled on HiDPI display

[Sergey Bylokhov]

On 01.09.16 11:25, Alexandr Scherbatiy wrote:
> According to your previous comments:
> - VolatileImage.validate(GraphicsConfiguration) does not return
> IMAGE_INCOMPATIBLE for a graphics configuration which default transform
> was changed.
>   This is because the BufImgVolatileSurfaceManager has acceleration
> disabled and it skips check to the valid graphics configuration.
>   I have filled an issue on it: JDK-8165212 VolatileImage is compatible
> with GraphicsConfiguration which transform is changed

This looks fine.

>
> - JViewport.backingStoreImage field is protected so it is not possible
> to change its type. Moreover, it is not clear can a someone override
> JViewport and use backingStoreImage as a buffered image so he does not
> expect that the image content can be lost.
>
> Could we just file a new issue that the backing store image can be
> changed to VolatileImage in JViewport and return to discussion where
> graphics scale changing is checked:
>   http://cr.openjdk.java.net/~alexsch/8164032/webrev.02

I suggest to thinking about more general solution. The code which you 
write is only one correct solution to create a backbuffer in HiDPI 
mode(another on is to use VolatileImage). The code itself is not trivial 
so it will be good to provide some API for this.
in the same way as createCompatibleImage() takes care about "color model".

> On 8/30/2016 3:54 PM, Alexandr Scherbatiy wrote:
>>
>> Hello,
>>
>> Could you review the updated fix:
>>   http://cr.openjdk.java.net/~alexsch/8164032/webrev.04/
>>
>> The Component.createVolatileImage() is used to create a backing store
>> volatile image.
>>
>> On 8/29/2016 4:18 PM, Sergey Bylokhov wrote:
>>> On 19.08.16 10:15, Alexandr Scherbatiy wrote:
>>>> On 8/18/2016 3:05 PM, Alexandr Scherbatiy wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> Could you review the updated fix:
>>>>>   http://cr.openjdk.java.net/~alexsch/8164032/webrev.02
>>>>>
>>>>>   The backing store image is recreated for the case where scaled sizes
>>>>> are changed.
>>>>
>>>>   A version where a VolatileImage is used as a backing store image:
>>>>     http://cr.openjdk.java.net/~alexsch/8164032/webrev.03/
>>>
>>> The VI case looks simpler. I guess it can be simplified even more:
>>> Peobably this code  is unnecessary?
>>> } else if (vImg instanceof SunVolatileImage) {
>>>  753                     SunVolatileImage svImg = (SunVolatileImage)
>>> vImg;
>>>  754                     if
>>> (!graphicsConfig.equals(svImg.getGraphicsConfig())) {
>>>  755                         backingStoreImage = null;
>>>  756                     }
>>>  757                 }
>>>
>>> My understanding is that if VI.validate!=IMAGE_INCOMPATIBLE then we
>>> can use it for drawing even if the GC is different?
>>   Different graphics configurations can have different default
>> transforms. The backing store image should be recreated in this case.
>>
>>   Thanks,
>>   Alexandr.
>>> Probably we can reuse Component.createVolatileImage() instead of
>>> createImage()?
>>>
>>> Also can you please check a performance of these two solutions?
>>>
>>>
>>>>
>>>>   Thanks,
>>>>   Alexandr.
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>>> On 8/17/2016 5:05 PM, Sergey Bylokhov wrote:
>>>>>> On 17.08.16 14:02, Alexandr Scherbatiy wrote:
>>>>>>>
>>>>>>> Hello,
>>>>>>>
>>>>>>> Could you review the updated fix:
>>>>>>>   http://cr.openjdk.java.net/~alexsch/8164032/webrev.01
>>>>>>>
>>>>>>> The generic transform is used for the backing store image scaling.
>>>>>>
>>>>>> As far as I understand the current fix will not work if the scale was
>>>>>> changed(the JViewport will use old buffer)?
>>>>>>
>>>>>> It seems that the new local AbstractMultiResolutionImage contain only
>>>>>> one image variant inside and looks quite similar to the
>>>>>> VolatileImage, probably we can change this cache from BufferedImage
>>>>>> to VolatileImage? In this case the scale of the VI will be the same
>>>>>> as "g":
>>>>>> g.getDeviceConfiguration().createCompatibleVolatileImage()
>>>>>>
>>>>>>
>>>>>>> On 8/15/2016 4:58 PM, Sergey Bylokhov wrote:
>>>>>>>> Hi, Alexandr.
>>>>>>>> I doubt that getScaleX/getScaleY can be used here because the scale
>>>>>>>> can be generic(translate+rotate+scale). How this cache will work if
>>>>>>>> transform will be changed after we save "backingStoreImage"?
>>>>>>>>
>>>>>>>> On 15.08.16 16:16, Alexandr Scherbatiy wrote:
>>>>>>>>>
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> Could you review the fix:
>>>>>>>>>   bug: https://bugs.openjdk.java.net/browse/JDK-8164032
>>>>>>>>>   webrev: http://cr.openjdk.java.net/~alexsch/8164032/webrev.00
>>>>>>>>>
>>>>>>>>>   The fix scales the JViewport backing store image when graphics
>>>>>>>>> transform is not identity.
>>>>>>>>>
>>>>>>>>>  Thanks,
>>>>>>>>>  Alexandr.
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>


-- 
Best regards, Sergey.