JDK-8215799 : Complex text is not rendered by webkit on Windows
  • Type: Bug
  • Component: javafx
  • Sub-Component: web
  • Affected Version: openjfx12
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2018-12-21
  • Updated: 2020-01-31
  • Resolved: 2019-01-18
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 Other
8u211Fixed openjfx11.0.3Fixed
Related Reports
Relates :  
JDK-8214119 - Update to 607.1 version of WebKit
Comments
Changeset: 32c0fa36d169 Author: arajkumar Date: 2019-01-18 23:14 +0530 URL: http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/32c0fa36d169 8215799: Complex text is not rendered by webkit on Windows Reviewed-by: mbilla, kcr, prr
18-01-2019
+1 and I filed https://bugs.openjdk.java.net/browse/JDK-8217392 about having better coverage in composite fonts on windows.
18-01-2019
[~kcr], [~prr], Updated the PR to include Phil's suggestion. Below are the new perf results, || site || with || without || regression || | oracle.com | 109/855 | 65/735 | 44% | | google.com | 136/58 | 47/79 | 300% | | aljazeera.net | 341/3550 | 54/1779 | 216% | | amazon.com | 38/512 | 49/161 | 0% | | font_perf_test | 6/1575 | 6/1575 | 0% |
17-01-2019
[~kcr], [~prr] Github PR is updated with above mentioned fix. Please take a look. Perf report with new fix: || site || with || without || regression || | oracle.com | 70/770 | 30/770 | 133% | | google.com | 68/58 | 64/79 | 44% | | aljazeera.net | 63/1595 | 52/1595 | 21% | | amazon.com | 51/164 | 55/154 | 0% | | font_perf_test | 9/1575 | 7/1575 | 22% |
14-01-2019
I tried to call text layout only when glyphs are missing for the given chars. It didn't seem to be working because for certain chars, glyph index are valid in a given char array. diff -r 35d5364af76a modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCFontImpl.java --- a/modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCFontImpl.java Wed Jan 09 11:41:49 2019 +0530 +++ b/modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCFontImpl.java Fri Jan 11 09:20:44 2019 +0000 @@ -129,6 +129,12 @@ int[] glyphs = new int[chars.length]; CharToGlyphMapper mapper = getFontStrike().getFontResource().getGlyphMapper(); mapper.charsToGlyphs(chars.length, chars, glyphs); + long validGlyphCount = Arrays.stream(glyphs).filter(g -> g != 0).count(); + if (validGlyphCount == 0) { + // Call charsToGlyphs once again after doing layout if all glyph indexs are zero + TextUtilities.createLayout(new String(chars), getPlatformFont()).getRuns(); + mapper.charsToGlyphs(chars.length, chars, glyphs); + } return glyphs; } Below is the simplified version without major logics, diff -r 35d5364af76a modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCFontImpl.java --- a/modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCFontImpl.java Wed Jan 09 11:41:49 2019 +0530 +++ b/modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCFontImpl.java Fri Jan 11 10:12:36 2019 +0000 @@ -125,12 +125,18 @@ return getFontStrike().getMetrics().getXHeight(); } - @Override public int[] getGlyphCodes(char[] chars) { + private int[] getGlyphCodes0(char[] chars) { int[] glyphs = new int[chars.length]; CharToGlyphMapper mapper = getFontStrike().getFontResource().getGlyphMapper(); mapper.charsToGlyphs(chars.length, chars, glyphs); return glyphs; } + @Override public int[] getGlyphCodes(char[] chars) { + int [] glyphs = getGlyphCodes0(chars); + TextUtilities.createLayout(new String(chars), getPlatformFont()).getRuns(); + glyphs = getGlyphCodes0(chars); + return glyphs; + } public float getAscent() { // REMIND: This method needs to require a render context. So the problem seems to be with "charsToGlyphs". CompositeGlyphMapper has "glyphMap" which caches the glyph index for missing glyph during the initial pass itself. Below one fixes that too, diff -r 35d5364af76a modules/javafx.graphics/src/main/java/com/sun/javafx/font/CompositeGlyphMapper.java --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/font/CompositeGlyphMapper.java Wed Jan 09 11:41:49 2019 +0530 +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/font/CompositeGlyphMapper.java Fri Jan 11 12:16:22 2019 +0000 @@ -100,7 +100,6 @@ return glyphCode; } } - glyphMap.put(unicode, missingGlyph); return missingGlyph; }
11-01-2019
>> That's a 10x performance hit. I have a hard time imaging that this won't be noticeable in some environments. Arun: "font_perf_test.html" is a micro benchmark which specifically targets the WCFontImpl.getGlyphCodes. That was the major reason for performance spike, it is hardly noticeablebecause the method execution takes only 0.06 ms(60000 ns). >> How easy would it be to call into the layout code only in the case where there is a missing glyph? Arun: That would be straightforward as [~prr] mentioned.
10-01-2019
That should be possible and I expect will resolve the performance regression for the common cases.
09-01-2019
> | font_perf_test.html | 85/1575 | 7/1575 | 1114% | That's a 10x performance hit. I have a hard time imaging that this won't be noticeable in some environments. How easy would it be to call into the layout code only in the case where there is a missing glyph?
09-01-2019
>> 5) Is there *really* no performance impact ? I did a quick sanity testing by loading various Indic WebPages and I didn't observe any noticible performance regressions. That isn't the right test. The right test is if pages that have only latin are slower ... and noticeable can't be "I loaded it once and it was OK". It needs to be a proper performance test of that code path.
09-01-2019
Thanks @prr for your review comments, PSB, >> 1) >> java.io.FileNotFoundException: Unable to create FontResource for file D:\Java\jdk-11\lib\fontsLucidaSansRegular.ttf >> >> Where's the "\" between "fonts" and the name of the file ? >> Is that a copy/paste problem or do we have a bigger problem ? Nope. Looks like we have an another problem to fix. >> 2) Are we still hard-coding LucidaSansRegular.ttf somewhere ? >> Seems like we must be. Not other than PrismFontFactory & FontConfigManager >> 3) Arial Unicode MS is not a standard Windows font. It might be installed >> by MS Word or something, but relying on it here seems a bad idea Exactly. >> 4) "TextLayout.getRuns" finds & adds the native fallback font. >> This might be OK for now, but it seems like you are relying on an internal side-effect. >> The direct write layout code asks for the "slot" for the font that DW said is being >> used for a code point and if it isn't found looks up that font and dynamically adds >> it to the logical font. So it means calling layout can be used to incrementally >> add to the logical font up to the maximum available slots. >> We aren't likely to remove that code so I suppose it is OK but as much as possible >> we should make sure that important code points are covered without resorting to >> such things. True, as of now I'm planning to use the workaround, I will file a new bug to provide a proper fallback for the indic unicode points. >> 5) Is there *really* no performance impact ? I did a quick sanity testing by loading various Indic WebPages and I didn't observe any noticible performance regressions. >> It seems like it depends on what "char sequence" means when you say >> "this method will be called only once during the entire life of WebKit for the' >> char sequence + Font + size + weight combination." >> And does that mean webkit caches such information *forever* ? Yes, it does caches forever. >> 6) Why is this just about complex text ? And it is probably not just about webkit either. >> Lucida Sans being gone will affect more than that. >> I suppose the uniqueness is that it was only the complex code points that got >> any benefit from this .. because it has to be complex text that went down the >> layout path - other missing code points stayed that way. >> Aside from this issue with webkit deciding that certain codepoints (and I'm sure >> its not just complex text) can't be rendered, we may want to add a list of >> standard windows fonts to replace the usage of LucidaSansRegular. >> The windows registry "linked fonts" generally have focused on adding CJK support >> and not much else. So we should add Indic + symbol fonts at least. >> A bigger reworking might be to find what DW uses for fallbacks and use that to >> build the logical font instead. But perhaps DW really does things dynamically so >> that may not be possible. >> But we definitely need follow on bugs to >> - remove the references to Lucida Sans Regular >> - add some additional fallbacks to replace what it used to provide >> Maybe this can be done as one bug. Sure. I will file a new bug to address this. >> 7) Since I am not sure that this is only about "complex text" or where it is is specific >> to webkit, maybe the synopsis could be tweaked slightly to stay focused on >> what you are addressing here. Would this work ? >> "Complex text is not rendered by webkit on Windows" Sure. I will update the title. >> 8) Question: what do you mean by "GlyphPage" and "GlyphTable" ? Are these some >> webkit internal structures ? Yes, those types are specfic to WebKit. If you get some time, Please take a look at "modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/GlyphPage.h". AFAIK, it is used by WebKit text rendering path to do a high level text breaking.
09-01-2019
1) java.io.FileNotFoundException: Unable to create FontResource for file D:\Java\jdk-11\lib\fontsLucidaSansRegular.ttf Where's the "\" between "fonts" and the name of the file ? Is that a copy/paste problem or do we have a bigger problem ? 2) Are we still hard-coding LucidaSansRegular.ttf somewhere ? Seems like we must be. 3) Arial Unicode MS is not a standard Windows font. It might be installed by MS Word or something, but relying on it here seems a bad idea 4) "TextLayout.getRuns" finds & adds the native fallback font. This might be OK for now, but it seems like you are relying on an internal side-effect. The direct write layout code asks for the "slot" for the font that DW said is being used for a code point and if it isn't found looks up that font and dynamically adds it to the logical font. So it means calling layout can be used to incrementally add to the logical font up to the maximum available slots. We aren't likely to remove that code so I suppose it is OK but as much as possible we should make sure that important code points are covered without resorting to such things. 5) Is there *really* no performance impact ? It seems like it depends on what "char sequence" means when you say "this method will be called only once during the entire life of WebKit for the' char sequence + Font + size + weight combination." And does that mean webkit caches such information *forever* ? 6) Why is this just about complex text ? And it is probably not just about webkit either. Lucida Sans being gone will affect more than that. I suppose the uniqueness is that it was only the complex code points that got any benefit from this .. because it has to be complex text that went down the layout path - other missing code points stayed that way. Aside from this issue with webkit deciding that certain codepoints (and I'm sure its not just complex text) can't be rendered, we may want to add a list of standard windows fonts to replace the usage of LucidaSansRegular. The windows registry "linked fonts" generally have focused on adding CJK support and not much else. So we should add Indic + symbol fonts at least. A bigger reworking might be to find what DW uses for fallbacks and use that to build the logical font instead. But perhaps DW really does things dynamically so that may not be possible. But we definitely need follow on bugs to - remove the references to Lucida Sans Regular - add some additional fallbacks to replace what it used to provide Maybe this can be done as one bug. 7) Since I am not sure that this is only about "complex text" or where it is is specific to webkit, maybe the synopsis could be tweaked slightly to stay focused on what you are addressing here. Would this work ? "Complex text is not rendered by webkit on Windows" 8) Question: what do you mean by "GlyphPage" and "GlyphTable" ? Are these some webkit internal structures ?
09-01-2019
Thanks for your response [~prr]. I just enabled WebKit's font perf logger to get more detailed result: || site || with || without || regression || | oracle.com | 240/754 | 73/787 | 243% | | google.com | 141/58 | 45/79 | 326% | | aljazeera.net | 195/1636 | 52/1636 | 275% | | amazon.com | 154/663 | 42/107 | 40% | | font_perf_test.html | 85/1575 | 7/1575 | 1114% | * format (time taken in milli seconds / no.of calls) Yes, there is a performance regression, but not visually observable.
09-01-2019
I agree with Kevin on Approach #2 as long as it doesn't impact performance (on other platforms), but that has been answered at https://github.com/javafxports/openjdk-jfx/pull/337#discussion_r245259112
04-01-2019
Approach #2 seems cleaner to me, although I'd want Phil to comment on this.
02-01-2019
Created Github PRs based on the approaches mentioned earlier, Approach1 : https://github.com/javafxports/openjdk-jfx/pull/336 Approach2: https://github.com/javafxports/openjdk-jfx/pull/337
02-01-2019
Proposal #1: diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFactory.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFactory.java index 3717d741a45..329814a3d75 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFactory.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFactory.java @@ -1024,6 +1024,10 @@ public abstract class PrismFontFactory implements FontFactory { fontRegInfo[0].add(jreFontDir + jreDefaultFontFile); fontRegInfo[1].add(jreDefaultFont); + // Add Arial Unicode to Windows fallback list + fontRegInfo[0].add(getPathNameWindows("arialuni.ttf")); + fontRegInfo[1].add("Arial Unicode MS"); + if (PlatformUtil.isWinVistaOrLater()) { // CJK Ext B Supplementary character fallbacks. fontRegInfo[0].add(getPathNameWindows("mingliub.ttc")); Proposal #2: diff --git a/modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCFontImpl.java b/modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCFontImpl.java index 36d6fc19266..f2bd9d932ca 100644 --- a/modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCFontImpl.java +++ b/modules/javafx.web/src/main/java/com/sun/javafx/webkit/prism/WCFontImpl.java @@ -126,6 +126,9 @@ final class WCFontImpl extends WCFont { } @Override public int[] getGlyphCodes(char[] chars) { + // Calling TextLayout.getRuns will lead to loading correct fallback + // font from native font engine. + TextUtilities.createLayout(new String(chars), getPlatformFont()).getRuns(); int[] glyphs = new int[chars.length]; CharToGlyphMapper mapper = getFontStrike().getFontResource().getGlyphMapper(); mapper.charsToGlyphs(chars.length, chars, glyphs); diff --git a/modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/ComplexTextControllerJava.cpp b/modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/ComplexTextControllerJava.cpp index a108cb883f6..dadc20cce3b 100644 --- a/modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/ComplexTextControllerJava.cpp +++ b/modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/ComplexTextControllerJava.cpp @@ -92,6 +92,10 @@ unsigned jGetEnd(jobject jRun) unsigned jGetCharOffset(jobject jRun, unsigned glyphIndex) { + if (!jGetGlyphCount(jRun)) { + return { }; + } + JNIEnv* env = WebCore_GetJavaEnv(); static jmethodID mID = env->GetMethodID( PG_GetTextRun(env),
02-01-2019
WebKit uses GlyphPage table to quickly decide whether any glyph is available for given character. For our JavaFX port, we get char to glyph mapping using CharToGlyphMapper. On macOS and Linux it is working because the LogicalFont had valid fallback for complex texts also, but it is missing for Windows. Refer PrismFontFactory.getLinkedFonts. 606.1 WebKit used GlyphTable only for simple texts, however 607.1 uses the same for complex texts also. I could think of the following workarounds., 1) Add "Arial Unicode MS Regular" as a fallback for Windows, it has almost all glyphs for the unicode range. 2) Call "TextLayout.getRuns" before invoking "CharToGlyphMapper", "TextLayout.getRuns" finds & adds the native fallback font. I prefer #2, because the changes are specific to WebKit platform layer.
27-12-2018
It works on JDK8 because it's jre/lib/fonts has LucidaSansRegular.ttf font. It is not present in JDK11.
26-12-2018
WCFontImpl.getGlyphCodes method returns 0 in Windows for all complex text characters. The method returns valid glyph index on other platform. It worked earlier because 606.1 WebKit didn't use the method for complex text paths, however 607.1 started using it even for complex texts.
24-12-2018
I see the following exception when enabling `prism.debugfonts` property. No such exceptions thrown with older WebKit. java.io.FileNotFoundException: Unable to create FontResource for file D:\Java\jdk-11\lib\fontsLucidaSansRegular.ttf at javafx.graphics/com.sun.javafx.font.PrismFontFile.init(PrismFontFile.java:466) at javafx.graphics/com.sun.javafx.font.PrismFontFile.<init>(PrismFontFile.java:103) at javafx.graphics/com.sun.javafx.font.directwrite.DWFontFile.<init>(DWFontFile.java:43) at javafx.graphics/com.sun.javafx.font.directwrite.DWFactory.createFontFile(DWFactory.java:65) at javafx.graphics/com.sun.javafx.font.PrismFontFactory.createFontResource(PrismFontFactory.java:248) at javafx.graphics/com.sun.javafx.font.PrismFontFactory.createFontResources(PrismFontFactory.java:285) at javafx.graphics/com.sun.javafx.font.PrismFontFactory.createFontResource(PrismFontFactory.java:265) at javafx.graphics/com.sun.javafx.font.PrismFontFactory.getFontResource(PrismFontFactory.java:628) at javafx.graphics/com.sun.javafx.font.PrismFontFactory.getFontResource(PrismFontFactory.java:701) at javafx.graphics/com.sun.javafx.font.LogicalFont.getSlotResource(LogicalFont.java:312) at javafx.graphics/com.sun.javafx.font.CompositeGlyphMapper.getSlotMapper(CompositeGlyphMapper.java:74) at javafx.graphics/com.sun.javafx.font.CompositeGlyphMapper.convertToGlyph(CompositeGlyphMapper.java:95) at javafx.graphics/com.sun.javafx.font.CompositeGlyphMapper.getGlyphCode(CompositeGlyphMapper.java:151) at javafx.graphics/com.sun.javafx.font.CharToGlyphMapper.charsToGlyphs(CharToGlyphMapper.java:84) at javafx.graphics/com.sun.javafx.font.CharToGlyphMapper.charsToGlyphs(CharToGlyphMapper.java:93) at javafx.web/com.sun.javafx.webkit.prism.WCFontImpl.getGlyphCodes(WCFontImpl.java:131) at javafx.web/com.sun.webkit.network.URLLoader.twkDidFinishLoading(Native Method) at javafx.web/com.sun.webkit.network.URLLoader.notifyDidFinishLoading(URLLoader.java:871) at javafx.web/com.sun.webkit.network.URLLoader.lambda$didFinishLoading$5(URLLoader.java:862) at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:428) at java.base/java.security.AccessController.doPrivileged(Native Method) at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$11(PlatformImpl.java:427) at javafx.graphics/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:96) at javafx.graphics/com.sun.glass.ui.win.WinApplication._runLoop(Native Method) at javafx.graphics/com.sun.glass.ui.win.WinApplication.lambda$runLoop$3(WinApplication.java:174) at java.base/java.lang.Thread.run(Thread.java:834)
21-12-2018
The below html snippet rendered as boxes. <!DOCTYPE html> <html> <meta charset="utf-8" /> <body> <p>���������������</p> <p><b>���������������</b></p> </body> </html>
21-12-2018