[OpenJDK 2D-Dev] [9] request for review: 8078382: Wrong glyph is displayed for a derived font

Alexey Ivanov alexey.ivanov at oracle.com
Wed Jun 1 17:25:41 UTC 2016


Hi Phil, Sergey,

Thank you for reviewing!

I suggest pushing the current version of the fix:
http://cr.openjdk.java.net/~bae/8078382/9/webrev.00/

It's Andrew's fix, and I'll push it under his name, giving him the credit.


As for FontFamily.getClosestStyle(), I'll submit a new bug to 
investigate the behavior. We will update getClosestStyle() if necessary.


Thanks,
Alexey

On 01.06.2016 13:21, Alexey Ivanov wrote:
> Hi Phil,
>
> On 01.06.2016 0:07, Phil Race wrote:
>> getClosestStyle() is something of a last resort.
>> Historically I think it was called only when you
>> had only a font *with* a style like bold and someone asked for 
>> plain/regular.
>> ie for the subtractive case rather than the additive case.
>> So it would not apply to the case in this bug.
>>
>> However OS X does not natively support synthetic styling so we may
>> be entering that method for the additive case on OS X.
>>
>> So  will not have this same bug since it just does not happen.
>>
>> In which case the reamaining question is, when you only have BOLD and 
>> ITALIC
>> and someone asks for BOLDITALIC and you can't synthesise at all, 
>> which do you give them ?
>
> So it needs additional testing. However, I think GDI still returns 
> Bold in this case: this way it would look consistent.
>
>>
>> I'd still lean towards ITALIC - assuming I have ITALIC, so we do not 
>> need to update that method.
>> If it can be shown otherwise we can make the change.
>
> Do you think we should push the fix as is, and leave 
> FontFamily.getClosestStyle() as it is now?
>
> Then test how native systems behave and update this method if necessary.
>
>
> Thanks,
> Alexey
>
>>
>>
>> -phil.
>>
>> On 05/31/2016 09:43 AM, Sergey Bylokhov wrote:
>>> On 31.05.16 19:30, Phil Race wrote:
>>>>> The patch changes the order of font selection: bold will be used, if
>>>>> possible, as the base for bold-italic instead of italic which is the
>>>>> current default. It also fixes the issue where italic is returned
>>>>> instead of bold.
>>>>
>>>> OK. +1
>>>
>>> I am not sure but should FontFamily.getClosestStyle() be updated also?:
>>>         case Font.BOLD|Font.ITALIC:
>>>             if (italic != null) {
>>>                 return italic;
>>>             } else if (bold != null) {
>>>                 return bold;
>>>             } else {
>>>                 return plain;
>>>             }
>>>         }
>>>
>>>>
>>>> -phil.
>>>>
>>>>>
>>>>>
>>>>> Thank you in advance,
>>>>> Alexey
>>>>>
>>>>> On 22.07.2015 19:33, Alexey Ivanov wrote:
>>>>>> Hi Phil,
>>>>>>
>>>>>> On 16.07.2015 21:38, Phil Race wrote:
>>>>>>> On 07/16/2015 06:08 AM, Andrew Brygin wrote:
>>>>>>>> Hi Phil,
>>>>>>>>
>>>>>>>>  another option to avoid the problem is to be a bit more specific
>>>>>>>> regarding the
>>>>>>>>  required font when we obtaining  lcd glyph from GDI.
>>>>>>>>  If we specify a particular name of the font which we used to
>>>>>>>> construct the
>>>>>>>>  glyph vector, then we will get glyphs exactly for desired 
>>>>>>>> characters:
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~bae/8078382/9/webrev.01/
>>>>>>>>
>>>>>>>>  This change affects only the case of lcd glyphs on windows,
>>>>>>>>  it reduces the scope of required testing.
>>>>>>>
>>>>>>> This is heading in the direction I was thinking of but since GDI is
>>>>>>> excepting a face name
>>>>>>> (what we call a family name), I am not sure if this will always 
>>>>>>> work
>>>>>>> as is.
>>>>>>> There are possible issues with using a localized name and the 
>>>>>>> length
>>>>>>> of the full name exceeding what Windows allows here.
>>>>>>> And there may be unintended consequences that are not immediately
>>>>>>> obvious.
>>>>>>> I would like to try limit this to the case where we can positively
>>>>>>> identify that the
>>>>>>> font is not the one we expected. And do it cheaply too.
>>>>>>> I do not have a quick answer here but roughly the algorithm 
>>>>>>> might be
>>>>>>> - specify family, check (somehow) if GDI selects the same base font
>>>>>>> - if not, try the facename approach (if the name fits). Did we
>>>>>>> succeed and get the same base font ?
>>>>>>> - if not, bail on GDI for this case and do the rasterisation 
>>>>>>> ourselves.
>>>>>>>
>>>>>>> "cheaply" might depend on caching a success value on the first
>>>>>>> attempt, so
>>>>>>> that we only do it once, ever, for a font. Then the problem becomes
>>>>>>> how to
>>>>>>> do the verification. One approach might be to call GetFontData() 
>>>>>>> which
>>>>>>> will give us some chunk of the font file and we can see if the name
>>>>>>> (or something
>>>>>>> else we can quickly and reliably parse) is exactly that we 
>>>>>>> expect ..
>>>>>>
>>>>>> It looks there's no easy way to detect whether GDI selected the same
>>>>>> base font or not. GetTextFace function doesn't help it: it always
>>>>>> returns the face name passed in LOGFONT except in the cases where
>>>>>> there's no such font.
>>>>>>
>>>>>> I haven't found any other API which could tell us what font GDI
>>>>>> selected. So the only alternative is to use GetFontData and parse 
>>>>>> the
>>>>>> font file itself. Yet I can't find any example how to use this
>>>>>> function. I'll keep searching in that direction.
>>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> Alexey
>>>>>>
>>>>>>>
>>>>>>> Leaving aside the 'wrong glyph' case, I have to suppose it is
>>>>>>> possible that
>>>>>>> there are other un-noticed cases where we use a different base font
>>>>>>> than
>>>>>>> that selected by GDI. The algorithms are not defined anywhere I can
>>>>>>> locate.
>>>>>>>
>>>>>>>>
>>>>>>>>  However, there seems to be a copy&paste error in FontFamily.java:
>>>>>>>>  on lines 340 - 341 we check that bold font fits our needs but use
>>>>>>>> italic
>>>>>>>>  anyway. Was it done by purpose, or this is really an error?
>>>>>>>
>>>>>>> That is  a copy/paste mistake. It should be fixed.
>>>>>>>
>>>>>>> -phil.
>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Andrew
>>>>>>>>
>>>>>>>> On 7/15/2015 7:25 PM, Phil Race wrote:
>>>>>>>>> This probably needs more examination and perhaps a more 
>>>>>>>>> complex fix.
>>>>>>>>> The observation that GDI bases bold-italic on the bold version 
>>>>>>>>> not
>>>>>>>>> the
>>>>>>>>> italic version is an implementation choice just as we had done 
>>>>>>>>> the
>>>>>>>>> opposite. It is possible some other time it does the opposite 
>>>>>>>>> or some
>>>>>>>>> other platform does the opposite. I have supposed it is harder to
>>>>>>>>> synthesise italic than to do 'over-strike'. And this GDI usage
>>>>>>>>> applies only to LCD glyphs.
>>>>>>>>>
>>>>>>>>> Maybe what we need to do is see if we can detect the cases when
>>>>>>>>> GDI and JDK  disagree on the actual font and remap the glyph id.
>>>>>>>>>
>>>>>>>>> -phil.
>>>>>>>>>
>>>>>>>>> On 7/15/15 4:12 AM, Andrew Brygin wrote:
>>>>>>>>>> Hello,
>>>>>>>>>>
>>>>>>>>>>  could you please review a fix for 8078382?
>>>>>>>>>>
>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8078382
>>>>>>>>>> webrev: http://cr.openjdk.java.net/~bae/8078382/9/webrev.00/
>>>>>>>>>>
>>>>>>>>>>  The problem is caused by following peculiarity of the Code New
>>>>>>>>>>  Roman font: this font provides plain, italic and bold variants.
>>>>>>>>>>  In bold and italic variants of the font, different glyphs
>>>>>>>>>>  correspond to the apostrophe character (0039):
>>>>>>>>>> bold: 0039 -> 0x250 (592)
>>>>>>>>>> italic: 0039 -> 0x256 (598)
>>>>>>>>>>
>>>>>>>>>>  So, we translate character to glyphs using italic variant
>>>>>>>>>>  of the font, and then request glyph images from GDI.
>>>>>>>>>>  However, GDI uses the bold variant of the font in order
>>>>>>>>>>  to compose glyph images for artificial bold-italic variant,
>>>>>>>>>>  and we have got a glyph image for ® instead of apostrophe.
>>>>>>>>>>
>>>>>>>>>>  Suggested fix is to select bold variant (if possible) as a
>>>>>>>>>>  base for artificial bold-italic.
>>>>>>>>>>
>>>>>>>>>>  There is no regression test because it requires a specific font
>>>>>>>>>>  to be installed on a test system. The font can be found here:
>>>>>>>>>>  http://www.dafont.com/code-new-roman.font
>>>>>>>>>>
>>>>>>>>>> Please take a look.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Andrew
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>




More information about the 2d-dev mailing list