<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.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.
>>