<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
alexandr.scherbatiy at oracle.com
Fri Mar 11 18:54:11 UTC 2016
Hello,
I need one more reviewer for the fix.
On 2/15/2016 11:24 AM, Jim Graham wrote:
> Thanks Alexandr,
>
> The AWT changes all look good.
>
> I'll leave it to others as to whether the test case represents the
> best way to test this. I have no idea how those two font values in
> those two L&F's in a Swing button should compare - maybe a difference
> of 8 is ordinary under some circumstances? Also, the test will only
> be conclusive when run on a HiDPI display larger than a certain scale
> factor so it may not even be testing anything in most test environments.
Should I add a manual test where it will be required to run it on
HiDPI displays?
Thanks,
Alexandr.
>
> In either case, the AWT code changes look fine...
>
> ...jim
>
> On 2/12/16 12:54 PM, Alexandr Scherbatiy wrote:
>> On 2/8/2016 3:04 PM, Jim Graham wrote:
>>> I don't understand the issue with the fonts that you are saying have
>>> different sizes for different DPIs. Those are pixel sizes, aren't
>>> they? They still need to be turned into user-space units for our
>>> applications to know what to do with them. Or, are you saying that
>>> they are not representing pixel sizes as the rest of the font units do
>>> and so they are already in DPI-relative "user space" units?
>>>
>>> AT 192 DPI, oemFixed.font is 18, but are they saying 18 pixels? Which
>>> would be a 9-point font in our automatically scaled coordinate
>>> systems, wouldn't it?
>>
>> The font size in the AwtDesktopProperties is calculated as (tmHeight
>> - tmInternalLeading) values from TEXTMETRIC:
>> jint pointSize = metrics.tmHeight - metrics.tmInternalLeading;
>> According to MSDN "All sizes are specified in logical units; that is,
>> they depend on the current mapping mode of the display context."
>>
>> The calculated size value is proportional to the scale factor for the
>> fonts like win.frame.captionFont :
>> DPI 96 (scale 100%), size: 15
>> DPI 144 (scale 150%), size: 23
>> DPI 192 (scale 200%), size: 30
>>
>> That means that for each DPI it is possible to scale down the font
>> size to its base value just using the formula:
>> baseFontSize = fontSize / scaleFactor
>>
>> However, for win.ansiFixed.font the TEXTMETRIC returns the same values
>> for each display DPI:
>> DPI 96 (scale 100%), size: 13
>> DPI 144 (scale 150%), size: 13
>> DPI 192 (scale 200%), size: 13
>>
>> Now scaling the size down for DPI 192 ( 13/ 2) it does not give the
>> font size for the DPI 96.
>> That is why the scale factor 1.0 is used in this case in the fix.
>>
>> Thanks,
>> Alexandr.
>>
>>>
>>> ...jim
>>>
>>> On 2/6/16 7:19 AM, Alexander Scherbatiy wrote:
>>>> On 2/5/2016 11:37 AM, Jim Graham wrote:
>>>>> Hi Alexandr,
>>>>>
>>>>> awt_DesktopProperties.cpp, line 300 - is the "1.0f /" a typo?
>>>> Sorry. Here is the updated fix without the typo:
>>>> http://cr.openjdk.java.net/~alexsch/8076545/webrev.06/
>>>>>
>>>>> Also, is there a still a need for the setFontProperty() variants that
>>>>> don't have a scale as the last parameter?
>>>> There are fonts like win.ansiFixed.font, win.ansiVar.font, and
>>>> win.deviceDefault.font which size is the same for any display DPI.
>>>>
>>>> And there are fonts like win.oemFixed.font, win.system.font, and
>>>> win.systemFixed.font which have one size for DPI 96 and another
>>>> size for
>>>> all other DPI.
>>>> For example win.oemFixed.font:
>>>> DPI 96, size: 12
>>>> DPI 144, size: 18
>>>> DPI 192, size: 18
>>>> DPI 240, size: 18
>>>>
>>>> I left them unscaled but may be it is better to have one
>>>> precalculated scale for this fonts which is used when DPI is not 96.
>>>>
>>>> I updated the setFontProperty() method without scale parameter
>>>> usage
>>>> to call with scale 1.0f.
>>>>
>>>> Thanks,
>>>> Alexandr.
>>>>
>>>>>
>>>>> ...jim
>>>>>
>>>>> On 2/5/2016 2:12 AM, Alexandr Scherbatiy wrote:
>>>>>>
>>>>>> 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.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>
More information about the awt-dev
mailing list