[OpenJDK 2D-Dev] [9] Review request for 8142966 Wrong cursor position in text components on HiDPI display

[Alexander Scherbatiy]

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.
>>>>>>>
>>>>>>
>>>>>
>>>
>>>
>>
>>
>