<AWT Dev> [OpenJDK 2D-Dev] [9] Review request for 8073320 Windows HiDPI Graphics support
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Wed Nov 11 11:08:48 UTC 2015
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?
>
> 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
For some reasons the link to the latest webrev and comments are not
mentioned in this email.
The BIGC:73 line has been corrected to:
73 ret = new BufferedImageGraphicsConfig(bImg, null,
scaleX, scaleY);
>
> 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.
Thanks,
Alexandr.
>
> Looks almost good to go from my perspective. What is the plan for
> Swing testing and fixing those bugs?
>
> ...jim
>
> On 10/22/15 8:28 AM, Alexander Scherbatiy wrote:
>>
>> 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.
>>>>>>
>>>>
>>>
>>>
>>
More information about the awt-dev
mailing list