[OpenJDK 2D-Dev] [9] Review request for 8073320 Windows HiDPI Graphics support

[Jim Graham]

I ran SwingSet2 and JDK-8069348 immediately jumped out as being broken 
to the point where SwingSet2 was not usable.  We should definitely make 
that a high priority to fix ASAP.  There were a couple of other very 
minor rendering issues, but they didn't really affect usability like 
that bug...

			...jim

On 11/13/2015 8:46 AM, Alexander Scherbatiy wrote:
>
>    Hello,
>
>    I have filled additional issues which has been discussed on the review:
>      JDK-8076545 Text size is twice bigger under Windows L&F on Win 8.1
> with HiDPI display
>      JDK-8142966 Wrong cursor position in text components on HiDPI display
>      JDK-8142961 Position, size and distance scaling for HiDPI graphics
> support
>      JDK-8142963 Better transformImage support for HiDPI images
>      JDK-8069348 SunGraphics2D.copyArea() does not properly work for
> scaled graphics
>      JDK-8142965 Consider the case where MRI can contains VolatileImage
>
>    Thanks,
>    Alexandr.
>
> On 11/12/2015 2:33 AM, Jim Graham wrote:
>> Hi Alexandr,
>>
>> On 11/11/15 3:08 AM, Alexander Scherbatiy wrote:
>>> On 11/10/2015 10:41 PM, Jim Graham wrote:
>>>> In drawHiDPIImage, in the VolatileImage case you ignore the transform,
>>>> but I suppose that is OK because you pass it through to the
>>>> scaleImage() case.  However if it came in with a transform then the
>>>> asserts at the top of scaleImage may fail?  Maybe it would be better
>>>> to actually handle the "non-0,0,iw,ih" case rather than assert that it
>>>> doesn't exist?
>>>       The only case where xform is not null is the drawImage(img, xform,
>>> observer) method which always use zero x, y and the same src and dst
>>> width/height.
>>>       Could we move the full support for a resolution variant
>>> transformation to a separate fix?
>>
>> Ah, sorry, I was reading the old webrev.02 and also mixing up
>> s[xy][12] (which are scaled) with d[xy][12] (which are the ones being
>> compared). You are correct there.
>>
>> The new version in webrev.03 is a little clunky with having to modify
>> the current transform, but it looks correct.
>>
>>>> BIGC.getConfig() - I don't see where you put the scale into the BIGC
>>>> when you create it.  Shouldn't the constructor with scales be used at
>>>> line 73?
>>>
>>>      It is updated in the latest webrev:
>>> http://cr.openjdk.java.net/~alexsch/8073320/webrev.03
>>>      which has been sent to the review:
>>> http://mail.openjdk.java.net/pipermail/2d-dev/2015-November/005814.html
>>
>> Found the webrev.03 and it is already fixed as you say.
>>
>>>> SGE.getScaleFactor() - suggestion: perhaps ignore an optional final
>>>> suffix "x" in the regular scale case so that they can say "=3.0x" to
>>>> mean the same as "=3"?
>>>
>>>     It should also be updated in the latest webrev.
>>
>> Found it.  Looks good...
>>
>>>> Looks almost good to go from my perspective.  What is the plan for
>>>> Swing testing and fixing those bugs?
>>
>> The latest fix looks good to go.  I was hoping to build it and test
>> some swing apps, but I don't have a decent build environment ready to
>> go yet.  We need to be proactive about understanding how this affects
>> Swing so that we can inform our developers about any changes they may
>> need to make to look good under HiDPI and/or modify the approach to
>> handle it ourselves...
>>
>>             ...jim
>