#396 - Don't use an Area when there is border radii present

because creating such an Area is very very slow. This is a quick fix - we should really never use an Area but this breaks many tests with square borders by a pixel for some reason. With visual regression test for broder radius CSS.
danfickle · Nov 6, 2019 · 2134f57 · 2134f57
1 parent 919451d
commit 2134f57
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 3 deletions.
diff --git a/README.md b/README.md
@@ -67,6 +67,7 @@ from ````/openhtmltopdf-examples/src/main/java/com/openhtmltopdf/testcases/Testc
 ## CHANGELOG
 
 ### head - 1.0.1-SNAPSHOT
++ [#396](https://github.com/danfickle/openhtmltopdf/issues/396) Much faster rendering of boxes using border-radius properties. Thanks @mndzielski.
 + [#400](https://github.com/danfickle/openhtmltopdf/pull/400) Support for `lang` and `title` attrbiutes and `abbr` tag for accessible PDFs. Thanks @Ignaciort91.
 
 ### 1.0.0 (2019-July-23)

diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/AbstractOutputDevice.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/render/AbstractOutputDevice.java
@@ -247,7 +247,12 @@ private void paintBackground0(
             return;
         }
 
-        Area borderBounds = new Area(BorderPainter.generateBorderBounds(backgroundBounds, border, true));
+        Shape borderBoundsShape = BorderPainter.generateBorderBounds(backgroundBounds, border, true);
+
+        // FIXME for issue 396 - generating an Area for a shape with curves is very very slow and
+        // memory intensive. However, not generating an area for simple squares breaks many tests.
+        // Therefore, for now, we just don't use an area if there are border radii present.
+        Area borderBounds = border.hasBorderRadius() && c.isFastRenderer() ? null : new Area(borderBoundsShape);
 
         Shape oldclip = null;
 
@@ -259,12 +264,12 @@ private void paintBackground0(
             }
             setClip(borderBounds);
         } else if (backgroundImage != null) {
-        	pushClip(borderBounds);
+        	pushClip(borderBounds != null ? borderBounds : borderBoundsShape);
         }
 
         if (backgroundColor != null && backgroundColor != FSRGBColor.TRANSPARENT) {
             setColor(backgroundColor);
-            fill(borderBounds);
+            fill(borderBounds != null ? borderBounds : borderBoundsShape);
         }
 
         if (backgroundImage != null) {

diff --git a/openhtmltopdf-examples/src/main/resources/visualtest/expected/border-radius.pdf b/openhtmltopdf-examples/src/main/resources/visualtest/expected/border-radius.pdf
diff --git a/openhtmltopdf-examples/src/main/resources/visualtest/html/border-radius.html b/openhtmltopdf-examples/src/main/resources/visualtest/html/border-radius.html
@@ -0,0 +1,44 @@
+<html>
+<head>
+<style>
+@page {
+  size: 200px 300px;
+  margin: 0;
+  padding: 0;
+}
+body {
+  padding: 0;
+  margin: 0;
+}
+div {
+  margin: 10px;
+  width: 150px;
+  height: 40px;
+  background-color: blue;
+}
+.one {
+  /* no border */
+  border-radius: 10px;
+}
+.two {
+  /* with-border */
+  border-radius: 8px;
+  border: 2px solid red;
+}
+.three {
+  /* multiple value syntax */
+  border-radius: 20px 5px;
+}
+.four {
+  /* individual corner */
+  border-top-left-radius: 20px 10px;
+}
+</style>
+</head>
+<body>
+<div class="one"></div>
+<div class="two"></div>
+<div class="three"></div>
+<div class="four"></div>
+</body>
+</html>
diff --git a/...-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java b/...-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java
@@ -883,6 +883,14 @@ public void testFloatClearBoth() throws IOException {
         assertTrue(vt.runTest("float-clear-both"));
     }
 
+    /**
+     * Tests that border radii work. Issue 396.
+     */
+    @Test
+    public void testBorderRadius() throws IOException {
+        assertTrue(vt.runTest("border-radius"));
+    }
+
     // TODO:
     // + Elements that appear just on generated overflow pages.
     // + content property (page counters, etc)