[OpenJDK 2D-Dev] [9] request for review: 8078382: Wrong glyph is displayed for a derived font
Alexey Ivanov
alexey.ivanov at oracle.com
Fri Jun 3 12:18:59 UTC 2016
Hi Phil, Sergey,
I pushed the fix and submitted a new bug
https://bugs.openjdk.java.net/browse/JDK-8158637
Regards,
Alexey
On 01.06.2016 20:25, Alexey Ivanov wrote:
> 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