<AWT Dev> [OpenJDK 2D-Dev] [9] Review request for 8076545 Text size is twice bigger under Windows L&F on Win 8.1 with HiDPI display

[Alexandr Scherbatiy]

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

   - The awt_DesktopProperties.cpp file is updated to use scaleX for 
width rescaling and scaleY for height rescaling

   Thanks,
   Alexandr.

On 2/1/2016 5:51 PM, Jim Graham wrote:
> Hi Alexandr,
>
> In awt_DesktopProperties.cpp you are using the Y scaling factor to 
> scale widths still - see lines 287 (and others in that same function) 
> and then 322 in another function.  It looks like you'll need 
> getInvScaleX() and getInvScaleY().
>
> I'll leave it to Phil to comment on the unit test...
>
>             ...jim
>
> On 2/1/16 4:27 AM, Alexandr Scherbatiy wrote:
>>
>> Could you review the updated fix:
>>    http://cr.openjdk.java.net/~alexsch/8076545/webrev.04/
>>
>>   - both LOGPIXELSX and Y are used for the theme size scaling.
>>   - LOGPIXELSY is used for text metrics height rescaling
>>
>>    Thanks,
>>    Alexandr.
>>
>> On 1/29/2016 1:16 PM, Jim Graham wrote:
>>> Hi Alexandr,
>>>
>>> Thanks for investigating the behaviors.
>>>
>>> With respect to using LOGPIXELSX or Y, these methods are used to scale
>>> both horizontal and vertical measurements so we really should have 2
>>> scale values and rescale methods, one for horizontal use and one for
>>> vertical...
>>>
>>>             ...jim
>>>
>>> On 1/29/2016 10:41 AM, Alexandr Scherbatiy wrote:
>>>>   Could you review the updated fix:
>>>>    http://cr.openjdk.java.net/~alexsch/8076545/webrev.03
>>>>
>>>>   - LOGPIXELSX is changed to  LOGPIXELSY
>>>>
>>>> On 11/16/2015 1:43 PM, Jim Graham wrote:
>>>>> Note that LOGPIXELSY is global and static between reboots so it
>>>>> doesn't really matter which monitor is used to get the value.
>>>>>
>>>>> Also, the issue is that the measurements are in pixels, so if we
>>>>> convert them into a resolution independent measurement then the rest
>>>>> of the scaling in the AWT/2D will be correct regardless of any given
>>>>> monitor's resolution.  We just have to make sure the 
>>>>> "de-pixelization"
>>>>> of the measurement is an apples-to-apples calculation.
>>>>>
>>>>> The question in my mind is whether the values they get from
>>>>> GetTheme*() and SPI_GETNONCLIENTMETRICS are relative to 
>>>>> LOGPIXELSY, or
>>>>> the more recent Per-Monitor aware DPI APIs, or ...?  It would be
>>>>> interesting to see what happens to those values when you change the
>>>>> DPI settings on Windows 8.1 and not reboot. If they stay constant
>>>>> until you reboot then LOGPIXELSY on the main screen would be the 
>>>>> value
>>>>> to use to de-scale them...
>>>>
>>>>     I tried to use the "Change the size of all items" slider on 
>>>> Windows
>>>> 8.1 without rebooting.
>>>>     GetDpiForMonitor() returns the updated DPI values: 192, 144, 96
>>>>     LOGPIXELSY returns unchanged values:  192, 192, 192
>>>>     SystemParametersInfo(SPI_GETNONCLIENTMETRICS,...) returns 
>>>> unchanged
>>>> NONCLIENTMETRICS
>>>>     GetThemePartSize() returns unchanged theme part sizes
>>>>
>>>>     It seems that theme part sizes behave in the same way as the
>>>> LOGPIXELSY in this case.
>>>>
>>>>     Thanks,
>>>>     Alexandr.
>>>>
>>>>>
>>>>>             ...jim
>>>>>
>>>>> On 11/16/2015 12:51 PM, Phil Race wrote:
>>>>>> That seems better. But one more question to get a point clarified.
>>>>>> You are using getDesktopWindow() which is for the primary monitor.
>>>>>> I assume that the 'inversion' results in the value being used to be
>>>>>> independent
>>>>>> of the monitor in a multi-mon situation and so when we move to a 2nd
>>>>>> monitor
>>>>>> that inverted size remains valid ?
>>>>>>
>>>>>> If so looks good to me.
>>>>>>
>>>>>> -phil.
>>>>>>
>>>>>> On 11/16/2015 06:07 AM, Alexander Scherbatiy wrote:
>>>>>>>
>>>>>>>   Hello,
>>>>>>>
>>>>>>>   Could you review the updated fix:
>>>>>>> http://cr.openjdk.java.net/~alexsch/8076545/webrev.02
>>>>>>>
>>>>>>>   - round is used instead of ceil
>>>>>>>   - inverted scales are used
>>>>>>>
>>>>>>>   Thanks,
>>>>>>>   Alexandr.
>>>>>>>
>>>>>>>
>>>>>>> On 10/30/2015 10:40 PM, Jim Graham wrote:
>>>>>>>> In this case round may be better. ceil() is more for cases 
>>>>>>>> where you
>>>>>>>> needed "at least X amount of room", but I don't think a font 
>>>>>>>> size is
>>>>>>>> an "at least this much" kind of case.
>>>>>>>>
>>>>>>>> Also, I've been toying with the idea that use of ceil() and 
>>>>>>>> floor()
>>>>>>>> in any DPI-adjustment equations should really be "ceil(val -
>>>>>>>> epsilon)" or "floor(ceil + epsilon)" for some small value of 
>>>>>>>> epsilon
>>>>>>>> chosen just large enough to prevent various round-off errors from
>>>>>>>> affecting the outcome.  One idea is for 1/256 as the value of
>>>>>>>> epsilon
>>>>>>>> since that could equate to the smallest measurable difference in
>>>>>>>> terms of alpha or interpolation results (or 1/512 for "half the
>>>>>>>> smallest quantum")...
>>>>>>>>
>>>>>>>>             ...jim
>>>>>>>>
>>>>>>>> On 10/29/15 1:36 PM, Phil Race wrote:
>>>>>>>>> size->cx = (int)ceil(size->cx / scale);
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> So if size->cx / scale works out to be 12.0001 you will round 
>>>>>>>>> it up
>>>>>>>>> to 13?
>>>>>>>>>
>>>>>>>>> Can you check what pixel size windows gives you in such a case ?
>>>>>>>>> I'd be a little surprised if they did that rather than round.
>>>>>>>>>
>>>>>>>>> Is the SetFontProperty that does not accept a scale parameter 
>>>>>>>>> still
>>>>>>>>> used
>>>>>>>>> somewhere ?
>>>>>>>>>
>>>>>>>>> -phil.
>>>>>>>>>
>>>>>>>>> On 10/29/2015 04:53 AM, Sergey Bylokhov wrote:
>>>>>>>>>> On 17.07.15 16:27, Alexander Scherbatiy wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> - Sergey's point about multi-mon should be checked out.
>>>>>>>>>>>      Windows 8.1 has option "Let me choose one scaling level 
>>>>>>>>>>> for
>>>>>>>>>>> all my
>>>>>>>>>>> displays".
>>>>>>>>>>>      If I unset it I am able to change the size of all items.
>>>>>>>>>>> However,
>>>>>>>>>>> the DPI which returns GetDPIForMonitor is still 2 on HiDPI
>>>>>>>>>>> displays.
>>>>>>>>>>
>>>>>>>>>> This version looks fine, but I am sure it can be double 
>>>>>>>>>> checked on
>>>>>>>>>> windows 10 at some moment as well.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>