[OpenJDK 2D-Dev] [PATCH] 8236996: Incorrect Roboto font rendering on Windows with subpixel antialiasing
Philip Race
philip.race at oracle.com
Mon Jan 27 17:46:00 UTC 2020
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 at oracle.com <mailto:Sergey.Bylokhov at 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 at oracle.com <mailto:philip.race at oracle.com>
> <mailto:philip.race at oracle.com <mailto:philip.race at 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 at oracle.com <mailto:philip.race at oracle.com>
> <mailto:philip.race at oracle.com <mailto:philip.race at 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/
> <http://cr.openjdk.java.net/%7Edbatrak/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.
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/2d-dev/attachments/20200127/ac1581fc/attachment-0001.htm>
More information about the 2d-dev
mailing list