[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [9] Review request for 8142966 Wrong cursor position in text components on HiDP
From:       Sergey Bylokhov <Sergey.Bylokhov () oracle ! com>
Date:       2016-03-25 22:24:01
Message-ID: 56F5BA81.2000801 () oracle ! com
[Download RAW message or body]

On 26.03.16 0:53, Phil Race wrote:
> On 3/25/2016 2:29 PM, Sergey Bylokhov wrote:
>> On 25.03.16 21:35, Alexandr Scherbatiy wrote:
>>>
>>>     I generated difference for all fonts from local graphics environment
>>> with size 12 between jdk with and without the fix for "low-dpi" display.
>>>     Results are in file [1] with simple format:
>>
>> It will be useful to check the difference when the FontMetrics are ON.
>>
>
> Do you mean FRACTIONALMETRICS=ON ?

Yes.

>
> I think that the difference in that case could (or should) be zero.

I think so, but it will be good to check.

> But FM_OFF is the important case for normal Swing-type text so
> the differences seen appear to be demonstrating what I am concerned about.

It depends on where this text is used, for example in a situation when 
the Swing will try to paint a selected words in the middle of text(like 
in the textfield):
"the quick brown fox jumps over the lazy dog."
Imagine "uick brown fo" should be selected and painted with a different 
background. In this situation the swing should start to paint after the 
"q" but before the "uick" uses integral coordinate in the userspace. If 
the real coordinate will be fractional then the selected text will be 
shifted(the selected text cannot be drawn in the same place as unselected).

We also should take into account that the current big(?) difference can 
come from issues in the fix(like wrong round or incorrect assumptions 
somewhere.)

>
> -phil.
>
>
>
>>>    ------------------------
>>> Font: java.awt.Font[family=Aharoni,name=Aharoni
>>> Bold,style=plain,size=12]
>>> [upper text width: 305] [upper text width: 306]
>>> [long  text width: 553] [long  text width: 554]
>>> [char [A] = 8] [char [A] = 9]
>>>
>>> Font: java.awt.Font[family=Andalus,name=Andalus,style=plain,size=12]
>>> [lower text width: 222] [lower text width: 220]
>>> [upper text width: 279] [upper text width: 275]
>>> [long  text width: 501] [long  text width: 495]
>>> [char [t] = 4] [char [t] = 3]
>>> [char [B] = 7] [char [B] = 6]
>>> [char [J] = 4] [char [J] = 3]
>>> [char [R] = 7] [char [R] = 6]
>>>    ------------------------
>>>
>>>    Where lower text width is width of the text "the quick brown fox
>>> jumps over the lazy dog", upper text width is width of the same text
>>> with upper case and long text is sum of these both texts.
>>>
>>>    166 fonts out of 362 have difference.
>>>
>>>    Links [2] and [3] contains some images with difference between drawn
>>> text before and after the fix. The text on the top is text before the
>>> fix, the text on the bottom is text after the fix and text between them
>>> shows the difference by red color.
>>>
>>>    [1]
>>> http://cr.openjdk.java.net/~alexsch/8142966/font-size-diff/font-size-diff_00.txt
>>>
>>>
>>>    [2]
>>> http://cr.openjdk.java.net/~alexsch/8142966/diff-images.00/low-case
>>>    [3]
>>> http://cr.openjdk.java.net/~alexsch/8142966/diff-images.00/upper-case
>>>
>>>    Thanks,
>>>    Alexandr.
>>>>
>>>> -phil.
>>>>
>>>> On 03/09/2016 04:43 AM, Alexander Scherbatiy wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> Could you review the proposed fix?
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>>> On 13/02/16 00:16, Alexander Scherbatiy wrote:
>>>>>> On 09/02/16 18:56, Sergey Bylokhov wrote:
>>>>>>> Also probably it will be possible to test this via the public api
>>>>>>> only(using the mix of the graphics transform + font transform).
>>>>>>>
>>>>>>> On 09.02.16 17:47, Sergey Bylokhov wrote:
>>>>>>>> Some additional information.
>>>>>>>>   The Swing calculates the size of the components and location
>>>>>>>> of the
>>>>>>>> cursor based on the text metrics. And these text metrics based on
>>>>>>>> advance. The Swing do not know to what destination it will be
>>>>>>>> painted,
>>>>>>>> it calculates the size based on the non-scaled(non-retina)
>>>>>>>> destination.
>>>>>>>> The problem occurs when we paint such component to the hidpi
>>>>>>>> screen,
>>>>>>>> because we get small round errors per glyph -> this causes a
>>>>>>>> visible
>>>>>>>> issueы when the text string is long. As a solution on the macosx
>>>>>>>> the
>>>>>>>> round operation was done in the users space instead of dev space.
>>>>>>>>
>>>>>>>> For example before the fix the size 1.4px was rounded to 3 (1.4
>>>>>>>> * 2 =
>>>>>>>> 2.8 -> 3), but after the fix it will be 2 (1.4-> 1 * 2 = 2). So the
>>>>>>>> Swing will be able to calculate correct size/location without
>>>>>>>> information of the device scale. Note that the 3px(in dev space)
>>>>>>>> is a
>>>>>>>> kind of fractional coordinate in the user space(1.5). And the
>>>>>>>> Swing does
>>>>>>>> not work properly when fractional metrics are used, because it use
>>>>>>>> ints
>>>>>>>> as a coordinates.
>>>>>>>>
>>>>>>>> Note that the fix should be applied only when fractional metrics
>>>>>>>> is off,
>>>>>>>> otherwise we should not use any rounding. I am not sure that the
>>>>>>>> current
>>>>>>>> fix take it into account.
>>>>>>  The proposed fix is applied only when the fractional metrics are
>>>>>> off.
>>>>>>>>
>>>>>>>>
>>>>>>>> On 08.02.16 22:14, Jim Graham wrote:
>>>>>>>>> Isn't the problem there that we are returning an integer as the
>>>>>>>>> advance?
>>>>>>>>>   Why aren't we returning 7.35 as a value instead of 8?
>>>>>>     7.35 is returned when fractional metrics are on.
>>>>>>>>>
>>>>>>>>> Also, shouldn't 7.35 round to 7 and 14.7 round to 15?
>>>>>>    This happens because when fractional metrics are on the glyph
>>>>>> linearAdvance value is returned (for current case
>>>>>> ftglyph->linearHoriAdvance = 7.35).
>>>>>>    But when fractional metrics are off the glyph advance is returned
>>>>>> (ftglyph->advance.x = 8).
>>>>>>    This calculation is done by freetype library.
>>>>>>
>>>>>>  Thanks,
>>>>>>  Alexandr.
>>>>>>
>>>>>>>>>
>>>>>>>>>                  ...jim
>>>>>>>>>
>>>>>>>>> On 2/6/2016 7:28 AM, Alexander Scherbatiy wrote:
>>>>>>>>>> On 05/02/16 23:39, Phil Race wrote:
>>>>>>>>>>> Two things strike me when I read this
>>>>>>>>>>>
>>>>>>>>>>> 1) Initial surprise at how deep into the font code it goes.
>>>>>>>>>>> Perhaps there are alternatives.
>>>>>>>>>>>
>>>>>>>>>>> 2) That it changes to using the linear metrics for measuring
>>>>>>>>>>> advance.
>>>>>>>>>>> Regardless of (1) I do not think (2) is correct. I am fairly
>>>>>>>>>>> sure this
>>>>>>>>>>> will lead to changes in text advance. It seems like it must
>>>>>>>>>>> throw
>>>>>>>>>>> away adjusted metrics as a result of glyph hinting.
>>>>>>>>>>>
>>>>>>>>>>> I don't know what the fix should be, since I have not looked at
>>>>>>>>>>> the
>>>>>>>>>>> problem top-down, I am just seeing the bottom-up proposed
>>>>>>>>>>> solution.
>>>>>>>>>>> So all I can say for now is that it needs to be at least
>>>>>>>>>>> somewhat
>>>>>>>>>>> different.
>>>>>>>>>>
>>>>>>>>>>    There was the same issue on Mac OS X which has been fixed
>>>>>>>>>> in the
>>>>>>>>>> similar way:
>>>>>>>>>>    8013569 [macosx] JLabel preferred size incorrect on retina
>>>>>>>>>> displays
>>>>>>>>>> with non-default font size
>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8013569
>>>>>>>>>> http://cr.openjdk.java.net/~serb/7190349/webrev.04/
>>>>>>>>>>
>>>>>>>>>>    The problem is that in many case for UI scale 2 we return to
>>>>>>>>>> a user
>>>>>>>>>> an unscaled value.
>>>>>>>>>>    But a value for transformed glyph advance, rounded and
>>>>>>>>>> descaled can
>>>>>>>>>> differ from just rounded glyph advance.
>>>>>>>>>>
>>>>>>>>>>   For example, font[dialog, plain, 12] char 'a':
>>>>>>>>>>   transform: 12, advance: 7.35,  rounded advance: 8
>>>>>>>>>>   transform: 24, advance: 14.70 round advance: 14
>>>>>>>>>>
>>>>>>>>>>   and 8 does not equal 14 / 2.
>>>>>>>>>>
>>>>>>>>>>    The solution for Mac OS X was to get the glyph advance using
>>>>>>>>>> only
>>>>>>>>>> font transform, round it and then apply the dev transform:
>>>>>>>>>>
>>>>>>>>>> CGGlyphImages.m:
>>>>>>>>>>   481     advance = CGSizeApplyAffineTransform(advance,
>>>>>>>>>> strike->fFontTx);
>>>>>>>>>>   482     if
>>>>>>>>>> (!JRSFontStyleUsesFractionalMetrics(strike->fStyle)) {
>>>>>>>>>>   483         advance.width = round(advance.width);
>>>>>>>>>>   484         advance.height = round(advance.height);
>>>>>>>>>>   485     }
>>>>>>>>>>   486     advance = CGSizeApplyAffineTransform(advance,
>>>>>>>>>> strike->fDevTx);
>>>>>>>>>>
>>>>>>>>>>    Thanks,
>>>>>>>>>>    Alexandr.
>>>>>>>>>>>
>>>>>>>>>>> -phil.
>>>>>>>>>>>
>>>>>>>>>>> On 01/27/2016 01:26 PM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hello,
>>>>>>>>>>>>
>>>>>>>>>>>> Could you review the fix:
>>>>>>>>>>>>   bug: https://bugs.openjdk.java.net/browse/JDK-8142966
>>>>>>>>>>>>   webrev: http://cr.openjdk.java.net/~alexsch/8142966/webev.00/
>>>>>>>>>>>>
>>>>>>>>>>>>  The proposed fix rounds a glyph advance first and then scales
>>>>>>>>>>>> it if
>>>>>>>>>>>> UI scales do not equal to one.
>>>>>>>>>>>>
>>>>>>>>>>>>  Thanks,
>>>>>>>>>>>>  Alexandr.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>>
>


-- 
Best regards, Sergey.
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic