[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] [PATCH] 8236996: Incorrect Roboto font rendering on Windows with subpixel antia
From: Dmitry Batrak <dmitry.batrak () jetbrains ! com>
Date: 2020-01-28 8:30:59
Message-ID: CAET5FPvxeVCx=o5++7_9cLQrFuit99XjEJNRFanERigTmDuA5Q () mail ! gmail ! com
[Download RAW message or body]
Thanks!
I also have a question about
https://bugs.openjdk.java.net/browse/JDK-8237085, which was assigned to me.
I'm not planning to work on it in the foreseeable future, beyond what was
already done in JDK-8236996.
What should I do with that ticket? Should I close that ticked as fixed,
mentioning that JDK-8236996 'mostly' fixes it? Should I remove myself from
the assignee field? Or should I just leave things as they are,
assuming that if someone will want to work on it, the issue will be
reassigned then?
Best regards,
Dmitry Batrak
On Mon, Jan 27, 2020 at 8:45 PM Philip Race <philip.race@oracle.com> wrote:
> I'll do this for you.
>
> -phil.
>
> On 1/27/20, 1:10 AM, Dmitry Batrak wrote:
>
> Thanks!
> Now that that change has two +1's, would anyone be able to push it?
>
> Best regards,
> Dmitry Batrak
>
> On Tue, Jan 21, 2020 at 8:05 AM Sergey Bylokhov <
> Sergey.Bylokhov@oracle.com> wrote:
>
>> Looks fine.
>> Thank you for contribution!
>>
>> On 1/20/20 12:14 am, Dmitry Batrak wrote:
>> > > Ok approved. Seems it is making a few things better if not ideal,
>> but nothing worse.
>> >
>> > Thanks!
>> > Anyone else volunteering to review?
>> >
>> > Best regards,
>> > Dmitry Batrak
>> >
>> > On Tue, Jan 14, 2020 at 8:58 PM Phil Race <philip.race@oracle.com
>> <mailto:philip.race@oracle.com>> wrote:
>> >
>> > Ok approved. Seems it is making a few things better if not ideal,
>> but nothing worse.
>> >
>> > -phil.
>> >
>> > On 1/14/20 8:14 AM, Dmitry Batrak wrote:
>> >> > So this is a workaround for a buggy font that doesn't play well
>> with GDI ?
>> >>
>> >> This is a workaround for all cases (or the vast majority of them)
>> of broken
>> >> rendering reported by our customers. The case with Roboto is just
>> the one we
>> >> have steps to reproduce for. There can be other cases where GDI's
>> logic is not
>> >> matched by JDK. Even if all of them are caused by
>> 'mis-constructed' fonts, I'm
>> >> afraid, this will not be considered as a good excuse by our
>> customers, as only
>> >> Java applications have such problems with these fonts.
>> >>
>> >> See JDK-8192972, still unsolved in OpenJDK, as an example of the
>> problems which
>> >> will be, at least partially, solved with this fix (correct glyphs
>> will be
>> >> rendered, albeit using FreeType).
>> >>
>> >> I did test the fix with fonts preinstalled in Windows 10. Fallback
>> was actually
>> >> triggered for one font (bold italic 'Segoe UI Semibold'), which is
>> not a 'false'
>> >> positive, but actually a manifestation of another JDK bug from the
>> same family -
>> >> I've just raised https://bugs.openjdk.java.net/browse/JDK-8237085
>> for it. That's
>> >> yet another example of an issue which will be (mostly) solved by
>> the proposed
>> >> fix.
>> >>
>> >> Using file length as a 'checksum' certainly doesn't guarantee we
>> choose the
>> >> right font, but the probability of error is very low, and this
>> value seems to be
>> >> the best candidate in our circumstances in terms of cost vs.
>> benefit. Even if
>> >> the validation mistreats a different font (having the same length)
>> as a correct
>> >> one, we'll not be in a worse position than before.
>> >>
>> >> Of course, there's a certain risk that rendering for unaffected
>> fonts might
>> >> change, but, given quite straightforward contract of GetFontData
>> function, I
>> >> would consider it very low.
>> >>
>> >> > Since you aren't retrieving the data, just asking what the size
>> is, I'd expect
>> >> > it to be unmeasurable.
>> >>
>> >> Well, we don't know how GetFontData works exactly, but it does
>> seem to add some
>> >> overhead. On my Windows 10 machine OpenJDK with the proposed fix
>> yields about 7%
>> >> larger result for the following benchmark program. The reported
>> value does
>> >> fluctuate from run to run, but the impact of the fix seems to be
>> larger than the
>> >> fluctuations.
>> >>
>> >> --- Benchmark source code ----
>> >> import java.awt.*;
>> >> import java.awt.font.GlyphVector;
>> >> import java.awt.image.BufferedImage;
>> >>
>> >> public class PerfTestOneFont {
>> >> private static final Font FONT = new Font("Segoe UI",
>> Font.PLAIN, 12);
>> >>
>> >> public static void main(String[] args) {
>> >> FONT.getFamily(); // preload font
>> >>
>> >> BufferedImage image = new BufferedImage(1, 1,
>> BufferedImage.TYPE_INT_RGB);
>> >> Graphics2D g = image.createGraphics();
>> >> g.setRenderingHint(RenderingHints.KEY_TEXT_ANTIALIASING,
>> >> RenderingHints.VALUE_TEXT_ANTIALIAS_LCD_HRGB);
>> >> int glyphCount = FONT.getNumGlyphs();
>> >> long startTime = System.currentTimeMillis();
>> >> for (int glyphCode = 0; glyphCode < glyphCount;
>> glyphCode++) {
>> >> GlyphVector gv =
>> FONT.createGlyphVector(g.getFontRenderContext(),
>> >> new int[]{glyphCode});
>> >> g.drawGlyphVector(gv, 0, 0);
>> >> }
>> >> long endTime = System.currentTimeMillis();
>> >> g.dispose();
>> >> System.out.println(endTime - startTime);
>> >> }
>> >> }
>> >> ------------------------------
>> >>
>> >> Best regards,
>> >> Dmitry Batrak
>> >>
>> >> On Mon, Jan 13, 2020 at 11:09 PM Phil Race <philip.race@oracle.com
>> <mailto:philip.race@oracle.com>> wrote:
>> >>
>> >> So this is a workaround for a buggy font that doesn't play
>> well with GDI ?
>> >>
>> >> It does rely on the fonts always being different sizes which
>> is highly
>> >> likely if not guaranteed.
>> >> I suppose it is OK so long as we aren't getting any "false"
>> positives.
>> >>
>> >> What I mean is that almost no one will have these Roboto fonts
>> >> installed, so the fix
>> >> is solving a problem they don't have, but if it is wrong in
>> some way,
>> >> then they could lose
>> >> GDI rendering of LCD glyphs and that could affect a lot of
>> people.
>> >>
>> >> So have you tested this with the full set of Windows 10 fonts -
>> >> including Indic, CJK, etc - to be sure
>> >> there are no cases where it fails for these or other spurious
>> failures.
>> >>
>> >> > As for performance impact, during testing I didn't observe
>> average
>> >> glyph generation time increase of more than 15%.
>> >>
>> >> Since you aren't retrieving the data, just asking what the
>> size is, I'd
>> >> expect it to be unmeasurable.
>> >>
>> >> -phil.
>> >>
>> >> On 1/13/20 1:25 AM, Dmitry Batrak wrote:
>> >> > Hello,
>> >> >
>> >> > I'd like to submit a patch for JDK-8236996. I'm not a
>> Committer, so
>> >> > I'll need someone to sponsor this change.
>> >> >
>> >> > Issue: https://bugs.openjdk.java.net/browse/JDK-8236996
>> >> > Webrev:
>> http://cr.openjdk.java.net/~dbatrak/8236996/webrev.00/
>> >> >
>> >> > The problem described in JDK-8236996 is from a group of
>> issues (see
>> >> > also e.g. JDK-8078382 and JDK-8192972), where JDK
>> >> > uses one font to perform char-to-glyph conversion, but GDI,
>> when asked
>> >> > to render the glyph is picking a different font,
>> >> > leading to completely random glyphs being rendered, as
>> char-to-glyph
>> >> > mapping obviously differs for different fonts.
>> >> >
>> >> > Specific version of Roboto font, mentioned in JDK-8236996,
>> is most
>> >> > probably causing the issue because it's not following
>> >> > the naming guidelines from OpenType specification
>> >> > (
>> https://docs.microsoft.com/en-us/typography/opentype/spec/name),
>> >> > having more than 4 variants (regular, bold, italic and bold
>> italic)
>> >> > with the same 'Font Family name' (name ID = 1). So,
>> >> > GDI gets confused and picks Roboto Black for rendering, when
>> asked to
>> >> > choose a regular font from Roboto family (Roboto
>> >> > Black having weight of 400, just like Roboto Regular,
>> probably adds to
>> >> > the confusion).
>> >> >
>> >> > But the reasoning, given above, about the issue cause is
>> only a guess.
>> >> > GDI is not an open-source subsystem, so we cannot
>> >> > know for sure how it selects the font for rendering, and
>> cannot
>> >> > implement matching logic in JDK. Ideally, we'd want to
>> >> > select the font by specifying its file path, but that's not
>> possible
>> >> > with GDI. Luckily, it allows us to query file data
>> >> > for the selected font using GetFontData function
>> >> > (
>> https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-getfontdata
>> ),
>> >> > which we can use to validate that the
>> >> > selected font is the one we need.
>> >> >
>> >> > The proposed solution is to check the file size of the font,
>> selected
>> >> > by GDI, before using it for rendering. If a mismatch
>> >> > is detected, fallback to FreeType is performed. It can
>> produce a
>> >> > somewhat different glyph representation, but, at least,
>> >> > the correct glyph will be rendered. For members of font
>> collections,
>> >> > file size for validation is calculated in a special
>> >> > way, in accordance with GetFontData logic described in the
>> >> > documentation. I've verified that it works for font
>> collections
>> >> > bundled with Windows 10.
>> >> >
>> >> > As for performance impact, during testing I didn't observe
>> average
>> >> > glyph generation time increase of more than 15%.
>> >> > Taking glyph caching into account, it shouldn't be that
>> significant
>> >> > for typical UI applications, I think. Performance
>> >> > impact can be made even smaller - by performing the
>> validation only
>> >> > once per font, but, I believe, having a Java
>> >> > application always render correct glyphs (even if fonts are
>> added or
>> >> > removed while application is running) is more
>> >> > important.
>> >> >
>> >> > Proposed patch doesn't add any tests, as reproducing the
>> issue
>> >> > requires installation of fonts. Existing automated
>> >> > OpenJDK tests pass after the fix. Proposed approach has been
>> used in
>> >> > JetBrains Runtime without known issues for about 3
>> >> > months in testing and for about 1 month in production.
>> >> >
>> >> > Best regards,
>> >> > Dmitry Batrak
>> >>
>> >>
>> >
>> >
>> >
>>
>>
>> --
>> Best regards, Sergey.
>>
>
>
>
>
[Attachment #3 (text/html)]
<div dir="ltr"><div>Thanks!<br><br></div><div>I also have a question about <a \
href="https://bugs.openjdk.java.net/browse/JDK-8237085">https://bugs.openjdk.java.net/browse/JDK-8237085</a>, \
which was assigned to me. I'm not planning to work on it in the foreseeable \
future, beyond what was already done in JDK-8236996.</div><div>What should I do with \
that ticket? Should I close that ticked as fixed, mentioning that JDK-8236996 \
'mostly' fixes it? Should I remove myself from the assignee field? Or should \
I just leave things as they are,</div><div>assuming that if someone will want to work \
on it, the issue will be reassigned then?</div><div><br></div><div>Best \
regards,</div><div>Dmitry Batrak<br></div><br><div class="gmail_quote"><div dir="ltr" \
class="gmail_attr">On Mon, Jan 27, 2020 at 8:45 PM Philip Race <<a \
href="mailto:philip.race@oracle.com">philip.race@oracle.com</a>> \
wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
I'll do this for you.<br>
<br>
-phil.<br>
<br>
On 1/27/20, 1:10 AM, Dmitry Batrak wrote:
<blockquote type="cite">
<div dir="ltr">
<div>Thanks!<br>
</div>
<div>Now that that change has two +1's, would anyone be able to
push it?</div>
<div><br>
</div>
<div>Best regards,</div>
<div>Dmitry Batrak<br>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, Jan 21, 2020 at 8:05
AM Sergey Bylokhov <<a href="mailto:Sergey.Bylokhov@oracle.com" \
target="_blank">Sergey.Bylokhov@oracle.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Looks fine.<br> Thank \
you for contribution!<br> <br>
On 1/20/20 12:14 am, Dmitry Batrak wrote:<br>
> > Ok approved. Seems it is making a few things
better if not ideal, but nothing worse.<br>
> <br>
> Thanks!<br>
> Anyone else volunteering to review?<br>
> <br>
> Best regards,<br>
> Dmitry Batrak<br>
> <br>
> On Tue, Jan 14, 2020 at 8:58 PM Phil Race <<a \
href="mailto:philip.race@oracle.com" \
target="_blank">philip.race@oracle.com</a>
<mailto:<a href="mailto:philip.race@oracle.com" \
target="_blank">philip.race@oracle.com</a>>> wrote:<br>
> <br>
> Ok approved. Seems it is making a few things better
if not ideal, but nothing worse.<br>
> <br>
> -phil.<br>
> <br>
> On 1/14/20 8:14 AM, Dmitry Batrak wrote:<br>
>> > So this is a workaround for a buggy font
that doesn't play well with GDI ?<br>
>><br>
>> This is a workaround for all cases (or the vast
majority of them) of broken<br>
>> rendering reported by our customers. The case
with Roboto is just the one we<br>
>> have steps to reproduce for. There can be other
cases where GDI's logic is not<br>
>> matched by JDK. Even if all of them are caused
by 'mis-constructed' fonts, I'm<br>
>> afraid, this will not be considered as a good
excuse by our customers, as only<br>
>> Java applications have such problems with these
fonts.<br>
>><br>
>> See JDK-8192972, still unsolved in OpenJDK, as
an example of the problems which<br>
>> will be, at least partially, solved with this
fix (correct glyphs will be<br>
>> rendered, albeit using FreeType).<br>
>><br>
>> I did test the fix with fonts preinstalled in
Windows 10. Fallback was actually<br>
>> triggered for one font (bold italic 'Segoe UI
Semibold'), which is not a 'false'<br>
>> positive, but actually a manifestation of
another JDK bug from the same family -<br>
>> I've just raised <a \
href="https://bugs.openjdk.java.net/browse/JDK-8237085" rel="noreferrer" \
target="_blank">https://bugs.openjdk.java.net/browse/JDK-8237085</a> for it. \
That's<br> >> yet another example of an issue which will be
(mostly) solved by the proposed<br>
>> fix.<br>
>><br>
>> Using file length as a 'checksum' certainly
doesn't guarantee we choose the<br>
>> right font, but the probability of error is
very low, and this value seems to be<br>
>> the best candidate in our circumstances in
terms of cost vs. benefit. Even if<br>
>> the validation mistreats a different font
(having the same length) as a correct<br>
>> one, we'll not be in a worse position than
before.<br>
>><br>
>> Of course, there's a certain risk that
rendering for unaffected fonts might<br>
>> change, but, given quite straightforward
contract of GetFontData function, I<br>
>> would consider it very low.<br>
>><br>
>> > Since you aren't retrieving the data, just
asking what the size is, I'd expect<br>
>> > it to be unmeasurable.<br>
>><br>
>> Well, we don't know how GetFontData works
exactly, but it does seem to add some<br>
>> overhead. On my Windows 10 machine OpenJDK with
the proposed fix yields about 7%<br>
>> larger result for the following benchmark
program. The reported value does<br>
>> fluctuate from run to run, but the impact of
the fix seems to be larger than the<br>
>> fluctuations.<br>
>><br>
>> --- Benchmark source code ----<br>
>> import java.awt.*;<br>
>> import java.awt.font.GlyphVector;<br>
>> import java.awt.image.BufferedImage;<br>
>><br>
>> public class PerfTestOneFont {<br>
>> private static final Font FONT = new
Font("Segoe UI", Font.PLAIN, 12);<br>
>><br>
>> public static void main(String[] args) {<br>
>> FONT.getFamily(); // preload font<br>
>><br>
>> BufferedImage image = new
BufferedImage(1, 1, BufferedImage.TYPE_INT_RGB);<br>
>> Graphics2D g = image.createGraphics();<br>
>>
g.setRenderingHint(RenderingHints.KEY_TEXT_ANTIALIASING,<br>
>> RenderingHints.VALUE_TEXT_ANTIALIAS_LCD_HRGB);<br>
>> int glyphCount = FONT.getNumGlyphs();<br>
>> long startTime =
System.currentTimeMillis();<br>
>> for (int glyphCode = 0; glyphCode <
glyphCount; glyphCode++) {<br>
>> GlyphVector gv =
FONT.createGlyphVector(g.getFontRenderContext(),<br>
>> \
new int[]{glyphCode});<br>
>> g.drawGlyphVector(gv, 0, 0);<br>
>> }<br>
>> long endTime =
System.currentTimeMillis();<br>
>> g.dispose();<br>
>> System.out.println(endTime -
startTime);<br>
>> }<br>
>> }<br>
>> ------------------------------<br>
>><br>
>> Best regards,<br>
>> Dmitry Batrak<br>
>><br>
>> On Mon, Jan 13, 2020 at 11:09 PM Phil Race <<a \
href="mailto:philip.race@oracle.com" \
target="_blank">philip.race@oracle.com</a>
<mailto:<a href="mailto:philip.race@oracle.com" \
target="_blank">philip.race@oracle.com</a>>> wrote:<br>
>><br>
>> So this is a workaround for a buggy font
that doesn't play well with GDI ?<br>
>><br>
>> It does rely on the fonts always being
different sizes which is highly<br>
>> likely if not guaranteed.<br>
>> I suppose it is OK so long as we aren't
getting any "false" positives.<br>
>><br>
>> What I mean is that almost no one will have
these Roboto fonts<br>
>> installed, so the fix<br>
>> is solving a problem they don't have, but
if it is wrong in some way,<br>
>> then they could lose<br>
>> GDI rendering of LCD glyphs and that could
affect a lot of people.<br>
>><br>
>> So have you tested this with the full set
of Windows 10 fonts -<br>
>> including Indic, CJK, etc - to be sure<br>
>> there are no cases where it fails for these
or other spurious failures.<br>
>><br>
>> > As for performance impact, during
testing I didn't observe average<br>
>> glyph generation time increase of more than
15%.<br>
>><br>
>> Since you aren't retrieving the data, just
asking what the size is, I'd<br>
>> expect it to be unmeasurable.<br>
>><br>
>> -phil.<br>
>><br>
>> On 1/13/20 1:25 AM, Dmitry Batrak wrote:<br>
>> > Hello,<br>
>> ><br>
>> > I'd like to submit a patch for
JDK-8236996. I'm not a Committer, so<br>
>> > I'll need someone to sponsor this
change.<br>
>> ><br>
>> > Issue: <a \
href="https://bugs.openjdk.java.net/browse/JDK-8236996" rel="noreferrer" \
target="_blank">https://bugs.openjdk.java.net/browse/JDK-8236996</a><br> >> \
> Webrev: <a href="http://cr.openjdk.java.net/%7Edbatrak/8236996/webrev.00/" \
rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~dbatrak/8236996/webrev.00/</a><br>
>> ><br>
>> > The problem described in JDK-8236996
is from a group of issues (see<br>
>> > also e.g. JDK-8078382 and
JDK-8192972), where JDK<br>
>> > uses one font to perform char-to-glyph
conversion, but GDI, when asked<br>
>> > to render the glyph is picking a
different font,<br>
>> > leading to completely random glyphs
being rendered, as char-to-glyph<br>
>> > mapping obviously differs for
different fonts.<br>
>> ><br>
>> > Specific version of Roboto font,
mentioned in JDK-8236996, is most<br>
>> > probably causing the issue because
it's not following<br>
>> > the naming guidelines from OpenType
specification<br>
>> > (<a \
href="https://docs.microsoft.com/en-us/typography/opentype/spec/name" \
rel="noreferrer" target="_blank">https://docs.microsoft.com/en-us/typography/opentype/spec/name</a>),<br>
>> > having more than 4 variants (regular,
bold, italic and bold italic)<br>
>> > with the same 'Font Family name' (name
ID = 1). So,<br>
>> > GDI gets confused and picks Roboto
Black for rendering, when asked to<br>
>> > choose a regular font from Roboto
family (Roboto<br>
>> > Black having weight of 400, just like
Roboto Regular, probably adds to<br>
>> > the confusion).<br>
>> ><br>
>> > But the reasoning, given above, about
the issue cause is only a guess.<br>
>> > GDI is not an open-source subsystem,
so we cannot<br>
>> > know for sure how it selects the font
for rendering, and cannot<br>
>> > implement matching logic in JDK.
Ideally, we'd want to<br>
>> > select the font by specifying its file
path, but that's not possible<br>
>> > with GDI. Luckily, it allows us to
query file data<br>
>> > for the selected font using
GetFontData function<br>
>> > (<a \
href="https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-getfontdata" \
rel="noreferrer" target="_blank">https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-getfontdata</a>),<br>
>> > which we can use to validate that the<br>
>> > selected font is the one we need.<br>
>> ><br>
>> > The proposed solution is to check the
file size of the font, selected<br>
>> > by GDI, before using it for rendering.
If a mismatch<br>
>> > is detected, fallback to FreeType is
performed. It can produce a<br>
>> > somewhat different glyph
representation, but, at least,<br>
>> > the correct glyph will be rendered.
For members of font collections,<br>
>> > file size for validation is calculated
in a special<br>
>> > way, in accordance with GetFontData
logic described in the<br>
>> > documentation. I've verified that it
works for font collections<br>
>> > bundled with Windows 10.<br>
>> ><br>
>> > As for performance impact, during
testing I didn't observe average<br>
>> > glyph generation time increase of more
than 15%.<br>
>> > Taking glyph caching into account, it
shouldn't be that significant<br>
>> > for typical UI applications, I think.
Performance<br>
>> > impact can be made even smaller - by
performing the validation only<br>
>> > once per font, but, I believe, having
a Java<br>
>> > application always render correct
glyphs (even if fonts are added or<br>
>> > removed while application is running)
is more<br>
>> > important.<br>
>> ><br>
>> > Proposed patch doesn't add any tests,
as reproducing the issue<br>
>> > requires installation of fonts.
Existing automated<br>
>> > OpenJDK tests pass after the fix.
Proposed approach has been used in<br>
>> > JetBrains Runtime without known issues
for about 3<br>
>> > months in testing and for about 1
month in production.<br>
>> ><br>
>> > Best regards,<br>
>> > Dmitry Batrak<br>
>><br>
>><br>
> <br>
> <br>
> <br>
<br>
<br>
-- <br>
Best regards, Sergey.<br>
</blockquote>
</div>
<br clear="all">
<br>
<br>
</div>
</blockquote>
</div>
</blockquote></div><br clear="all"><br></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic