<AWT Dev> [OpenJDK 2D-Dev] [9] Review Request: JDK-8029455 JLightweightFrame: support scaled painting

[Anton V. Tarasov]

Thanks for the review,  Anthony!

Regards,
Anton.

On 04.02.2014 22:55, Anthony Petrov wrote:
> Hi Anton,
>
> I skimmed through the code that I'm familiar with, and the changes look good to me. Someone from 
> Swing and 2D should review their parts, too.
>
> -- 
> best regards,
> Anthony
>
> On 2/3/2014 6:36 PM, Anton V. Tarasov wrote:
>> Hi Jim,
>>
>> Please look at the updated version:
>>
>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.4
>>
>> On 01.02.2014 5:35, Jim Graham wrote:
>>> Hi Anton,
>>>
>>> On 1/31/14 6:37 AM, Anton V. Tarasov wrote:
>>>> My understanding is that, unless the fix is absolutely irrelevant (whic
>>>> I hope it isn't), we should integrate it into 9/8u20 to support
>>>> SwingNode in its current implementation on Retina displays.
>>>>
>>>> What do you think?
>>>
>>> I agree with this sentiment.  My suggestion for reducing its footprint
>>> aside (which it looks like you are investigating), the main thing I
>>> will be looking for is whether or not we return an object to a
>>> developer (i.e. it isn't just managed internally to our own code)
>>> where they can do "instanceof BufferedImage" and then see something
>>> odd coming from its width/height.
>>>
>>> It looks like if we simply use getLayoutWH() internally then they
>>> would never ever see anything odd from getWidthHeight() anyway. The
>>> only additional "gotcha" would be if they grab the image and render it
>>> directly and it comes out an unexpected size to them (because, for
>>> instance, they didn't check the layout dims like we do internally).
>>> That's a pretty minor issue, though, and I think we could live with it.
>>
>> I've refactored the fix with this concern in mind. Here is what I've done:
>>
>> - eliminated the new OffscreenHiDPIImage class (as you suggested) and
>> put all of its "scale" logic into the OffscreenImage.
>> - made the scaled OffscreenImage return the physical size (like an
>> ordinary BufferredImage does) by default .
>> - renamed the "set/isHiDPIEnabled" method to "set/isReturnLayoutSize".
>> - made the setReturnLayoutSize(true) be called internally (where we
>> don't leak the OffscreenImage).
>>
>> In SG2D, the drawHiDPIImage, for instance, makes a call to
>> op.filter(img, null); where the img is expected to return its layout
>> size. That's why I still prefer to use the setReturnLayoutSize
>> "switcher", in order not to go deep into the 2D rendering code, casting
>> here and there to OffscreenImage and calling getLayoutWidth/Height.
>>
>>>
>>> For 9.0 perhaps we could add the LayoutWH() as new API on
>>> BufferedImage and then most of this would be public?  I'd have to mull
>>> over if that makes sense and I'm not entirely sure of the naming yet...
>>
>> That's a good idea, however we need a 8u working solution as well... As
>> to the naming, I'm ready for any suggestions.
>>
>> Thanks,
>> Anton.
>>
>>>
>>>             ...jim
>>