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

[Alexander Scherbatiy]

  Could you review the updated fix:
     http://cr.openjdk.java.net/~alexsch/8073320/webrev.02/


On 10/21/2015 5:40 PM, Sergey Bylokhov wrote:
> I have two small suggestions:
>  - Probably it will be better to move the new properties from 
> Win32GraphicsDevice.java to SunGraphicsEnvironment.java, at least 
> "sun.java2d.uiScale" should be implemented on linux, and can be 
> implemented on osx later.
      I have moved the uiScale properties initialization to the 
SunGraphicsEnvironment and Win32GraphicsEnvironment classes.
>  - I am worried about a rounding in 
> awt_Win32GraphicsDevice.ScaleDownXY/ScaleUpXY when these methods are 
> applied to the x/y and width/height.

     Could I move this update to a separate fix?

    Thanks,
    Alexandr.

>
> On 21.10.15 16:34, Alexander Scherbatiy wrote:
>>
>>   Could you review the updated fix:
>>     http://cr.openjdk.java.net/~alexsch/8073320/webrev.01/
>>
>>   - xform is concatenated with transform in SG2D.getResolutionVariant()
>> method
>>   - imagepipe.transformImage() is used for drawImage(Image,
>> AffineTransform,...) case
>>   - scale factors are made final in BufImgSurfaceData
>>   - two arrays are used for configs caching in BIGC
>>   - scale factors intialized from properties are moved to the static
>> block in Win32GD
>>
>> On 10/10/2015 3:14 AM, Jim Graham wrote:
>>> Hi Alexandr,
>>>
>>> Just 2 potential issues I found:
>>>
>>> SG2D.java - getResolutionVariant() doesn't take a transform so it
>>> can't take into account the scaling in the transform handed to
>>> drawImage(img, xform). We can leave that as a follow-on bug fix (it
>>> won't do bad things, but it may not pick the best resolution source
>>> image).
>>>
>>> WGLOffscreenSD (in WGLSD.java) - does peer.getBounds() cache the 
>>> bounds?
>>
>>     No. peer.getBounds() directly calls targetComponent.getBounds()
>> which returns a copy of original bounds.
>>
>>    Thanks,
>>    Alexandr.
>>> If so, then every time we call get bounds on the WGLWSD we will
>>> permanently modify the peer's bounds. Would it be better to not make
>>> the assumption and just make a protected copy here (perhaps with scale
>>> != 1?).
>>> WGLWindowSD - same comment
>>> D3DWindowSD - same comment
>>> GDIWindowSD - same comment
>>>
>>> The rest of these are tweaks and optimizations.
>>>
>>> BISD.java - why do you have default initializers for the scale fields?
>>> (There are only 2 constructors to cover.)
>>>
>>> BIGC.java - [2][NUMTYPES] would involve a lot fewer objects than
>>> [NUMTYPES][2]...
>>> BIGC.java - Or...  If we only have 2 variations of each type, why not
>>> just have 2 static arrays instead of a double-indexed array?
>>>     int index = (scaleX && scaleY) ? 0 : 1;
>>> becomes:
>>>     BIGC configarray = (scaleX && scaleY) ? standardArray : 
>>> scaledArray;
>>>
>>> SG2D.java - Another way of dealing with transform in scaleImage would
>>> be to make a copy of the incoming transform and adjust it to match the
>>> request, as in:
>>>     // You can probably assert that dxywh == 0,0,imgW,imgH, or:
>>>     AffineTransform renderTX;
>>>     if (dxywh == 0,0,imgW,imgH) {
>>>         renderTX = xform;
>>>     } else {
>>>         // Should never happen in practice...
>>>         AffineTransform renderTX = new AffineTransform(xform);
>>>         renderTX.translate(dx1, dy1);
>>>         renderTX.scale((dx2 - dx1) / imgW, (dy2 - dy1) / imgH);
>>>     }
>>>     double-nested-try {
>>>         imagepipe.transformImage(..., renderTX);
>>>     } catch (InvalidPipe...) {...}
>>> It would be more code, though, since you'd have to have 2 sets of
>>> "double-nested-try-catch(InvalidPipe)" blocks.
>>>
>>> Win32GD.java - is there a need to have static initializers for the
>>> scale fields?
>>>
>>> I'll leave the native code review to someone more familiar with that
>>> code base...
>>>
>>>             ...jim
>>>
>>> On 9/22/15 2:33 AM, Alexander Scherbatiy wrote:
>>>>
>>>> Hello,
>>>>
>>>> Could you review the fix:
>>>>    bug: https://bugs.openjdk.java.net/browse/JDK-8073320
>>>>    webrev: http://cr.openjdk.java.net/~alexsch/8073320/webrev.00
>>>>
>>>>    This is an initial part of the HiDPI Graphics support on Windows 
>>>> for
>>>> the JEP 263: HiDPI Graphics on Windows and Linux
>>>>      http://openjdk.java.net/jeps/263
>>>>
>>>>    - scale factors are added to surface dates
>>>>    - window size, events coordinates and font are scaled on native 
>>>> side
>>>>    - backup buffered image is scaled in SunVolatileImage
>>>>    - AwtRobot MouseMove() and GetRGBPixels() methods are updated
>>>>    - GetDpiForMonitor function is used to query the specified monitor
>>>> for the horizontal and vertical DPI values.
>>>>      If it is not available ID2D1Factory::GetDesktopDpi method is used
>>>> instead.
>>>>    - "sun.java2d.uiScale.enabled", "sun.java2d.win.uiScale",
>>>> "sun.java2d.win.uiScaleX", and "sun.java2d.win.uiScaleY" options are
>>>> added for the testing purposes.
>>>>
>>>> Thanks,
>>>> Alexandr.
>>>>
>>
>
>