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