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.
