Fixes #342 - Fixed text-justification and letter-spacing when fallbac…

…k fonts are used. With tests. Thanks for reporting @daliuss!
danfickle · Apr 29, 2019 · 957cd03 · 957cd03
1 parent 21d380b
commit 957cd03
Show file tree

Hide file tree

Showing 11 changed files with 63 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,7 @@
 ## CHANGELOG
 
 ### head - 0.0.1-RC21-SNAPSHOT
++ [#342](https://github.com/danfickle/openhtmltopdf/issues/342) Fixed text-justification/letter-spacing when fallback fonts are in use. Thanks @daliuss.
 + [#351](https://github.com/danfickle/openhtmltopdf/issues/351) Improved text-justification by removing spaces at ends of lines. Thanks @halcsi.
 
 

diff --git a/README.md b/README.md
@@ -70,6 +70,7 @@ from ````/openhtmltopdf-examples/src/main/java/com/openhtmltopdf/testcases/Testc
 ## CHANGELOG
 
 ### head - 0.0.1-RC21-SNAPSHOT
++ [#342](https://github.com/danfickle/openhtmltopdf/issues/342) Fixed text-justification/letter-spacing when fallback fonts are in use. Thanks @daliuss.
 + [#351](https://github.com/danfickle/openhtmltopdf/issues/351) Improved text-justification by removing spaces at ends of lines. Thanks @halcsi.
 
 

diff --git a/openhtmltopdf-examples/src/main/resources/visualtest/expected/text/letter-spacing-bidi.pdf b/openhtmltopdf-examples/src/main/resources/visualtest/expected/text/letter-spacing-bidi.pdf
diff --git a/...df-examples/src/main/resources/visualtest/expected/text/letter-spacing-fallback-fonts.pdf b/...df-examples/src/main/resources/visualtest/expected/text/letter-spacing-fallback-fonts.pdf
diff --git a/...opdf-examples/src/main/resources/visualtest/expected/text/text-justify-fallback-fonts.pdf b/...opdf-examples/src/main/resources/visualtest/expected/text/text-justify-fallback-fonts.pdf
diff --git a/...topdf-examples/src/main/resources/visualtest/html/text/letter-spacing-fallback-fonts.html b/...topdf-examples/src/main/resources/visualtest/html/text/letter-spacing-fallback-fonts.html
@@ -1,6 +1,9 @@
 <html>
 <head>
 <style>
+@page {
+  size: 400px 400px;
+}
 * {
   font-family: 'TestFont', 'ExtraFont';
   letter-spacing: 10px;
@@ -14,11 +17,11 @@
 </p>
 
 <p>
-ЏШЩЪЫЬЭЮЯабвгґдђеё
+ЏШ ЩЪЫЬ ЭЮЯаб вгґ дђ её
 </p>
 
 <p>
-ЏaШbЩcЪdЫeЬfЭgЮhЯiаkjбвkгlґmдnђoеё
+ЏaШbЩc ЪdЫeЬfЭgЮ hЯiаkj бвkг lґmдn ђoеё
 </p>
 </body>
 </html>
diff --git a/...mltopdf-examples/src/main/resources/visualtest/html/text/text-justify-fallback-fonts.html b/...mltopdf-examples/src/main/resources/visualtest/html/text/text-justify-fallback-fonts.html
@@ -0,0 +1,20 @@
+<html>
+<head>
+<style>
+@page {
+  size: 400px 400px;
+}
+* {
+  font-family: 'TestFont', 'ExtraFont';
+  font-size: 20px;
+}
+</style>
+</head>
+<body>
+<p style="text-align: justify;">
+The quick brown fox fell over the zoo.
+ABCČ ĆDĐEFG HIJKL MNOPQ RSŠTU VWXY ZŽabc čćdđe fghij klmnop qrsšt uvwx yzžА БВГҐД ЂЕЁЄ 
+ЖЗЅИІ ЇЙЈ КЛ ЉМНЊОП РСТЋ УЎФХ ЦЧЏШ ЩЪЫЬЭ ЮЯабв гґдђ еёє ж
+</p>
+</body>
+</html>
diff --git a/...mples/src/test/java/com/openhtmltopdf/visualregressiontests/TextVisualRegressionTest.java b/...mples/src/test/java/com/openhtmltopdf/visualregressiontests/TextVisualRegressionTest.java
@@ -584,7 +584,6 @@ public void testArabicBiDi() throws IOException {
      * Semi-related to issue 342.
      */
     @Test
-    @Ignore // It is a mess.
     public void testLetterSpacingBidi() throws IOException {
         assertTrue(vtester.runTest("letter-spacing-bidi", WITH_ARABIC));   
     }
@@ -594,11 +593,18 @@ public void testLetterSpacingBidi() throws IOException {
      * Issue 342.
      */
     @Test
-    @Ignore // Characters too close in mixed font text and some overlapping.
     public void testLetterSpacingFallbackFonts() throws IOException {
         assertTrue(vtester.runTest("letter-spacing-fallback-fonts", WITH_EXTRA_FONT));
     }
 
+    /**
+     * Tests that text-justification works when fallback fonts are being used.
+     */
+    @Test
+    public void testJustificationFallbackFonts() throws IOException {
+        assertTrue(vtester.runTest("text-justify-fallback-fonts", WITH_EXTRA_FONT));
+    }
+
     /**
      * Tests that justified text doesn't have space at the end of some lines.
      * Issue 351.

diff --git a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFastOutputDevice.java b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxFastOutputDevice.java
@@ -405,7 +405,13 @@ public void drawString(String s, float x, float y, JustificationInfo info) {
         for (FontRun run : fontRuns) {
             drawStringFast(run.str, x + xOffset, y, info, run.des, _font.getSize2D());
             try {
-                xOffset += (run.des.getFont().getStringWidth(run.str) / 1000f) * _font.getSize2D();
+                if (info == null) {
+                    xOffset += ((run.des.getFont().getStringWidth(run.str) / 1000f) * _font.getSize2D());
+                } else {
+                    xOffset += ((run.des.getFont().getStringWidth(run.str) / 1000f) * _font.getSize2D()) +
+                               (run.spaceCharacterCount * info.getSpaceAdjust()) +
+                               (run.otherCharacterCount * info.getNonSpaceAdjust());
+                }
             } catch (Exception e) {
                 XRLog.render(Level.WARNING, "BUG. Font didn't contain expected character.", e);
             }

diff --git a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxSlowOutputDevice.java b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxSlowOutputDevice.java
@@ -463,6 +463,8 @@ public void drawStringFast(String s, float x, float y, JustificationInfo info, F
     public static class FontRun {
         String str;
         FontDescription des;
+        int spaceCharacterCount;
+        int otherCharacterCount;
     }
 
     private Object[] makeJustificationArray(String s, JustificationInfo info) {

diff --git a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxTextRenderer.java b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxTextRenderer.java
@@ -133,6 +133,10 @@ private static class ReplacementChar {
         FontDescription fontDescription;
     }
 
+    public static boolean isJustificationSpace(int c) {
+        return c == ' ' || c == '\u00a0' || c == '\u3000';
+    }
+
     private static ReplacementChar getReplacementChar(FSFont font) {
         String replaceStr = ThreadCtx.get().sharedContext().getReplacementText();
         List<FontDescription> descriptions = ((PdfBoxFSFont) font).getFontDescription();
@@ -208,6 +212,12 @@ else if (des != current.des) {
                         sb = new StringBuilder();
                     }
 
+                    if (isJustificationSpace(unicode)) {
+                        current.spaceCharacterCount++;
+                    } else {
+                        current.otherCharacterCount++;
+                    }
+
                     sb.append(ch);
                     gotChar = true;
                     break;
@@ -231,6 +241,13 @@ else if (des != current.des) {
                                 current.des = des;
                                 sb = new StringBuilder();
                             }
+
+                            if (isJustificationSpace(unicode)) {
+                                current.spaceCharacterCount++;
+                            } else {
+                                current.otherCharacterCount++;
+                            }
+
                             sb.append(deshaped);
                             gotChar = true;
                             break FONT_LOOP;
@@ -258,12 +275,14 @@ else if (replace.fontDescription != current.des) {
                 }
 
                 if (Character.isSpaceChar(unicode) || Character.isWhitespace(unicode)) {
+                    current.spaceCharacterCount++;
                     sb.append(' ');
                 }
                 else if (!OpenUtil.isCodePointPrintable(unicode)) {
                     // Do nothing
                 }
                 else {
+                    current.otherCharacterCount++;
                     sb.append(replace.replacement);
                 }
             }