Merge pull request #653 from danfickle/fix_642_inline_block_layer_dou…

…ble_paint Fixes #642 Solution for double ups of inline-blocks
danfickle · Mar 1, 2021 · 44787f7 · 44787f7
2 parents 76b86d9 + 84803bc
commit 44787f7
Show file tree

Hide file tree

Showing 11 changed files with 415 additions and 8 deletions.
diff --git a/...htmltopdf-core/src/main/java/com/openhtmltopdf/render/displaylist/DisplayListPainter.java b/...htmltopdf-core/src/main/java/com/openhtmltopdf/render/displaylist/DisplayListPainter.java
@@ -120,13 +120,15 @@ private void paintInlineContent(RenderingContext c, List<DisplayListItem> inline
 				OperatorSetClip setClip = (OperatorSetClip) dli;
 				setClip(c, setClip);
 			} else if (dli instanceof BlockBox) {
-			    // Inline blocks need to be painted as a layer.
-			    BlockBox bb = (BlockBox) dli;
-			    List<PageBox> pageBoxes = bb.getContainingLayer().getPages();
-			    DisplayListCollector dlCollector = new DisplayListCollector(pageBoxes);
-			    DisplayListPageContainer pageInstructions = dlCollector.collectInlineBlock(c, bb, EnumSet.noneOf(CollectFlags.class), c.getShadowPageNumber());
-
-			    paint(c, pageInstructions);
+                // Inline blocks need to be painted as a layer, if not already done so.
+                BlockBox bb = (BlockBox) dli;
+                if (!bb.getStyle().requiresLayer()) {
+                    List<PageBox> pageBoxes = bb.getContainingLayer().getPages();
+                    DisplayListCollector dlCollector = new DisplayListCollector(pageBoxes);
+                    DisplayListPageContainer pageInstructions = dlCollector.collectInlineBlock(c, bb, EnumSet.noneOf(CollectFlags.class), c.getShadowPageNumber());
+
+                    paint(c, pageInstructions);
+               }
 			} else {
                 InlinePaintable paintable = (InlinePaintable) dli;
                 Object token = c.getOutputDevice().startStructure(StructureType.INLINE, (Box) dli);

diff --git a/openhtmltopdf-examples/src/main/resources/visualtest/expected/issue-642-inline-blocks.pdf b/openhtmltopdf-examples/src/main/resources/visualtest/expected/issue-642-inline-blocks.pdf
diff --git a/openhtmltopdf-examples/src/main/resources/visualtest/expected/issue-642-transform-inline.pdf b/openhtmltopdf-examples/src/main/resources/visualtest/expected/issue-642-transform-inline.pdf
diff --git a/openhtmltopdf-examples/src/main/resources/visualtest/html/issue-642-inline-blocks.html b/openhtmltopdf-examples/src/main/resources/visualtest/html/issue-642-inline-blocks.html
@@ -0,0 +1,56 @@
+<html>
+<head>
+<style>
+@page {
+  size: 150px 200px;
+  margin: 0;
+  margin-top: 20px;
+
+  @top-left {
+    content: element(runner);
+  }
+}
+@page:first {
+  size: 150px 500px;
+  @top-left {
+    content: none;
+  }
+  margin-top: 0;
+}
+body {
+  margin: 10px;
+  max-width: 130px;
+}
+</style>
+</head>
+<body>
+<!-- inline block running element -->
+<div style="display: inline-block; position: running(runner);">
+SIX<span style="display: inline-block; transform: rotate(-20deg);">TEEN</span>
+</div>
+<!-- fixed inline block -->
+<div style="position:fixed;top:0;left:0;display:inline-block;">ONE</div>
+<!-- inline block inside fixed -->
+<div style="position:fixed;top:50px;left:0;">TWO <span style="display:inline-block;">THREE</span></div>
+<!-- absolute inline block -->
+<div style="position:absolute;top:100px;left:0;display:inline-block;">FOUR</div>
+<!-- inline block inside absolute -->
+<div style="position:absolute;top:150px;left:0;">FIVE <span style="display:inline-block;">SIX</span></div>
+<div></div>
+<div style="margin-top: 200px;">SPACER</div>
+<!-- inline block inside table -->
+<table><tr><td><span style="display: inline-block;">SEVEN</span></td></tr></table>
+<!-- floating inline block -->
+<span style="display: inline-block;float: left;">EIGHT</span>
+<!-- cleared inline block -->
+<span style="display: block; clear: both;"><span style="display: inline-block;">NINE</span></span>
+<!-- absolute inside inline block -->
+<span style="display: inline-block; position: relative; width: 90%;">TEN <div style="position: absolute; top: 0; right: 0;">ELEVEN</div></span>
+<!-- transformed inline block inside transformed inline block -->
+<span style="display: inline-block; transform: scale(2, 1); margin-left: 30px;">TWELVE<span style="display: inline-block; transform: rotate(180deg);">THIRTEEN</span></span>
+<div style="page-break-before: always; margin-top: 100px;">NEW PAGE</div>
+<!-- inline block on new page -->
+<span style="display: inline-block; transform: scale(2, 1); margin-left: 30px;">FOURTEEN<span style="display: inline-block; transform: rotate(180deg);">FIFTEEN</span></span>
+
+</body>
+</html>
diff --git a/openhtmltopdf-examples/src/main/resources/visualtest/html/issue-642-transform-inline.html b/openhtmltopdf-examples/src/main/resources/visualtest/html/issue-642-transform-inline.html
@@ -9,5 +9,8 @@
 </head>
 <body>
 <div><span style="transform: scale(2, 2); display: inline-block;">O N E</span></div>
+<div><span style="display: inline-block;">T W O</span></div>
+<div> <span style="display: inline-block;">T H R E E</span> </div>
+<div> <span style="transform: scale(2, 2); display: inline-block;">F O U R</span> </div>
 </body>
 </html>
diff --git a/...src/test/java/com/openhtmltopdf/nonvisualregressiontests/RepeatContentRegressionTest.java b/...src/test/java/com/openhtmltopdf/nonvisualregressiontests/RepeatContentRegressionTest.java
@@ -0,0 +1,203 @@
+package com.openhtmltopdf.nonvisualregressiontests;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.pdfbox.io.IOUtils;
+import org.apache.pdfbox.pdmodel.PDDocument;
+import org.apache.pdfbox.text.PDFTextStripper;
+import org.apache.pdfbox.util.Charsets;
+import org.junit.Test;
+
+import com.openhtmltopdf.pdfboxout.PdfRendererBuilder;
+import com.openhtmltopdf.testcases.TestcaseRunner;
+import com.openhtmltopdf.visualtest.VisualTester.BuilderConfig;
+
+public class RepeatContentRegressionTest {
+    private static final String RES_PATH = "/repeated-content-tests/";
+    private static final String OUT_PATH = "target/test/visual-tests/test-output/";
+
+    interface StringMatcher {
+        List<String> problems(String expected, String actual);
+        void doAssert(String expected, String actual, String fileName, List<String> problems);
+    }
+
+    /**
+     * Simple tests with content all in one layer
+     * (ie. no z-index, absolute, relative, fixed, transform)
+     * should generally use an ordered matcher.
+     */
+    private static class OrderedMatcher implements StringMatcher {
+        public List<String> problems(String expected, String actual) {
+            if (!expected.equals(actual)) {
+                return Collections.singletonList("Mismatched ordered");
+            }
+
+            return Collections.emptyList();
+        }
+
+        public void doAssert(String expected, String actual, String fileName, List<String> problems) {
+            assertEquals("Mismatched: " + fileName, expected, actual);
+        }
+    }
+
+    /**
+     * Layers have to follow a specific painting order so they may be out of order.
+     */
+    private static class UnOrderedMatcher implements StringMatcher {
+        public List<String> problems(String expected, String actual) {
+            String[] expWords = expected.split("\\s");
+            String[] actWords = actual.split("\\s");
+
+            Predicate<String> filter = w -> !w.trim().isEmpty();
+
+            Set<String> exp = Arrays.stream(expWords)
+                                    .filter(filter)
+                                    .collect(Collectors.toSet());
+
+            List<String> act = Arrays.stream(actWords)
+                                     .filter(filter)
+                                     .collect(Collectors.toList());
+
+            Set<String> seen = new HashSet<>();
+
+            List<String> problems = new ArrayList<>();
+
+            for (String word : act) {
+                if (seen.contains(word)) {
+                    problems.add("Repeat content: " + word);
+                }
+
+                seen.add(word);
+
+                if (!exp.contains(word)) {
+                    problems.add("Unexpected content: " + word);
+                }
+            }
+
+            for (String word : exp) {
+                if (!act.contains(word)) {
+                    problems.add("Missing: " + word);
+                }
+            }
+
+            return problems;
+        }
+
+        public void doAssert(String expected, String actual, String fileName, List<String> problems) {
+            fail(problems.stream().collect(Collectors.joining("\n")));
+        }
+    }
+
+    private static final StringMatcher ORDERED = new OrderedMatcher();
+    private static final StringMatcher UNORDERED = new UnOrderedMatcher();
+
+    private static void render(String fileName, String html, BuilderConfig config, String proof, StringMatcher matcher) throws IOException {
+        ByteArrayOutputStream actual = new ByteArrayOutputStream();
+
+        PdfRendererBuilder builder = new PdfRendererBuilder();
+        builder.withHtmlContent(html, NonVisualRegressionTest.class.getResource(RES_PATH).toString());
+        builder.toStream(actual);
+        builder.useFastMode();
+        builder.testMode(true);
+        config.configure(builder);
+
+        try {
+            builder.run();
+        } catch (IOException e) {
+            System.err.println("Failed to render resource (" + fileName + ")");
+            e.printStackTrace();
+            throw e;
+        }
+
+        byte[] pdfBytes = actual.toByteArray();
+
+        try (PDDocument doc = PDDocument.load(pdfBytes)) {
+            PDFTextStripper stripper = new PDFTextStripper();
+            stripper.setSuppressDuplicateOverlappingText(false);
+            stripper.setLineSeparator("\n");
+
+            String text = stripper.getText(doc).trim();
+            String expected = proof.trim().replace("\r\n", "\n");
+
+            List<String> problems = matcher.problems(expected, text);
+
+            if (!problems.isEmpty()) {
+                FileUtils.writeByteArrayToFile(new File(OUT_PATH, fileName + ".pdf"), pdfBytes);
+
+                matcher.doAssert(expected, text, fileName, problems);
+            }
+        }
+    }
+
+    private static void run(String fileName, BuilderConfig config, StringMatcher matcher) throws IOException {
+        String absResPath = RES_PATH + fileName + ".html";
+
+        try (InputStream is = TestcaseRunner.class.getResourceAsStream(absResPath)) {
+            byte[] htmlBytes = IOUtils.toByteArray(is);
+            String htmlWithProof = new String(htmlBytes, Charsets.UTF_8);
+
+            String[] parts = htmlWithProof.split(Pattern.quote("======="));
+            String html = parts[0];
+            String proof = parts[1];
+
+            render(fileName, html, config, proof, matcher);
+        }
+    }
+
+    private static void runOrdered(String fileName) throws IOException {
+        run(fileName, builder -> { }, ORDERED);
+    }
+
+    private static void runUnOrdered(String fileName) throws IOException {
+        run(fileName, builder -> { }, UNORDERED);
+    }
+
+    /**
+     * inline-blocks/relative/absolute with z-index.
+     */
+    @Test
+    public void testInlineBlockZIndex() throws IOException {
+        runOrdered("inline-block-z-index");
+    }
+
+    /**
+     * Multiple in-flow inline-blocks on the one page. 
+     */
+    @Test
+    public void testInlineBlockMultiple() throws IOException {
+        runUnOrdered("inline-block-multiple");
+    }
+
+    /**
+     * Multiple in-flow inline-blocks across pages with large page margin.
+     */
+    @Test
+    public void testInlineBlockPages() throws IOException {
+        runUnOrdered("inline-block-pages");
+    }
+
+    /**
+     * Float that goes over two pages.
+     */
+    @Test
+    public void testFloatsPages() throws IOException {
+        runUnOrdered("floats-pages");
+    }
+
+}
diff --git a/...-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java b/...-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java
@@ -1367,14 +1367,23 @@ public void testExternalAccessControl() throws IOException {
     /**
      * Transforms on inline-block or inline elements output twice,
      * once as the non-transformed output and once as transformed.
+     * Also affected inline-blocks with a z-index.
      * https://github.com/danfickle/openhtmltopdf/issues/642
      */
     @Test
-    @Ignore // Not debugged yet.
     public void testIssue642TransformInline() throws IOException {
         assertTrue(vt.runTest("issue-642-transform-inline"));
     }
 
+    /**
+     * Combinations of inline-blocks with other display and positioned
+     * content.
+     */
+    @Test
+    public void testIssue642InlineBlocks() throws IOException {
+        assertTrue(vt.runTest("issue-642-inline-blocks"));
+    }
+
     /**
      * Tests that the background-image property allows multiple values.
      */

diff --git a/openhtmltopdf-examples/src/test/resources/repeated-content-tests/floats-pages.html b/openhtmltopdf-examples/src/test/resources/repeated-content-tests/floats-pages.html
@@ -0,0 +1,50 @@
+<html>
+<head>
+<style>
+@page {
+  size: 180px 100px;
+  margin: 20px;
+}
+body {
+  font-size: 14px;
+  margin: 0;
+}
+.flt-l {
+  float: left;
+  border: 1px solid red;
+  margin: 3px;
+}
+.flt-r {
+  float: right;
+  border: 1px solid green;
+  width: 60%;
+}
+</style>
+</head>
+<body>
+<div class="flt-l">one two three</div>
+four five six seven eleven
+<div class="flt-r">eight nine ten sixteen seventeen eighteen</div>
+ twelve thirteen fourteen
+fifteen
+</body>
+</html>
+=======
+one
+two
+three
+four
+five
+six
+seven
+eight
+nine
+ten
+eleven
+twelve
+thirteen
+fourteen
+fifteen
+sixteen
+seventeen
+eighteen
diff --git a/openhtmltopdf-examples/src/test/resources/repeated-content-tests/inline-block-multiple.html b/openhtmltopdf-examples/src/test/resources/repeated-content-tests/inline-block-multiple.html
@@ -0,0 +1,28 @@
+<html>
+<head>
+<style>
+span {
+  display: inline-block;
+}
+</style>
+</head>
+<body>
+<div><span>one</span></div>
+<div> <span>two</span> </div>
+<span>three</span>
+<span>four</span>
+<div><span>five</span> <span>six</span></div>
+<div> seven <span>eight</span> nine <span>ten</span></div>
+</body>
+</html>
+=======
+one
+two
+three
+four
+five
+six
+seven
+eight
+nine
+ten