Dynamic background color changes do not update until a layout is forced
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Oct 2015 19:59:12 +0000 (19:59 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Oct 2015 19:59:12 +0000 (19:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=131623

Source/WebCore:

Compute correct repaint rect for decorated RenderSVGRoots.

The current implementation of clippedOverflowRectForRepaint() uses the
generic repaint-rect calculations in SVGRenderSupport. Those in turn make
use of repaintRectInLocalCoordinates(), which for RenderSVGRoot is the
union of the painted children (w/ some expansion). If there're no children,
or they do not fill the entire content box, then a repaint would not
repaint the correct parts.
Fix by calculating the union of the border-box and the SVG content
when the SVG root is decorated (has background/border/etc.)

Adapted from a Chromium patch by fs@opera.com
https://src.chromium.org/viewvc/blink?revision=170890&view=revision

Patch by Antoine Quint <graouts@apple.com> on 2015-10-09
Reviewed by Darin Adler.

Tests: svg/repaint/add-background-property-on-root.html
       svg/repaint/add-border-property-on-root.html
       svg/repaint/add-outline-property-on-root.html
       svg/repaint/change-background-color.html
       svg/repaint/remove-background-property-on-root.html
       svg/repaint/remove-border-property-on-root.html
       svg/repaint/remove-outline-property-on-root.html

* rendering/svg/RenderSVGRoot.cpp:
(WebCore::RenderSVGRoot::layout):
(WebCore::RenderSVGRoot::styleDidChange):
(WebCore::RenderSVGRoot::clippedOverflowRectForRepaint):

LayoutTests:

Add some new tests checking that dynamically updating the "background",
"border" and "outline" CSS properties repaint correctly and rebase a few
existing tests that yield the same rendered results but slightly different
DRT output.

Patch by Antoine Quint <graouts@apple.com> on 2015-10-09
Reviewed by Darin Adler.

* platform/mac/fast/repaint/moving-shadow-on-container-expected.txt:
* platform/mac/svg/custom/simple-text-double-shadow-expected.txt:
* svg/css/composite-shadow-example-expected.txt:
* svg/css/composite-shadow-with-opacity-expected.txt:
* svg/repaint/add-background-property-on-root-expected.html: Added.
* svg/repaint/add-background-property-on-root.html: Added.
* svg/repaint/add-border-property-on-root-expected.html: Added.
* svg/repaint/add-border-property-on-root.html: Added.
* svg/repaint/add-outline-property-on-root-expected.html: Added.
* svg/repaint/add-outline-property-on-root.html: Added.
* svg/repaint/change-background-color-expected.html: Added.
* svg/repaint/change-background-color.html: Added.
* svg/repaint/remove-background-property-on-root-expected.html: Added.
* svg/repaint/remove-background-property-on-root.html: Added.
* svg/repaint/remove-border-property-on-root-expected.html: Added.
* svg/repaint/remove-border-property-on-root.html: Added.
* svg/repaint/remove-outline-property-on-root-expected.html: Added.
* svg/repaint/remove-outline-property-on-root.html: Added.
* svg/repaint/repaint-webkit-svg-shadow-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@190816 268f45cc-cd09-0410-ab3c-d52691b4dbfc

22 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/mac/fast/repaint/moving-shadow-on-container-expected.txt
LayoutTests/platform/mac/svg/custom/simple-text-double-shadow-expected.txt
LayoutTests/svg/css/composite-shadow-example-expected.txt
LayoutTests/svg/css/composite-shadow-with-opacity-expected.txt
LayoutTests/svg/repaint/add-background-property-on-root-expected.html [new file with mode: 0644]
LayoutTests/svg/repaint/add-background-property-on-root.html [new file with mode: 0644]
LayoutTests/svg/repaint/add-border-property-on-root-expected.html [new file with mode: 0644]
LayoutTests/svg/repaint/add-border-property-on-root.html [new file with mode: 0644]
LayoutTests/svg/repaint/add-outline-property-on-root-expected.html [new file with mode: 0644]
LayoutTests/svg/repaint/add-outline-property-on-root.html [new file with mode: 0644]
LayoutTests/svg/repaint/change-background-color-expected.html [new file with mode: 0644]
LayoutTests/svg/repaint/change-background-color.html [new file with mode: 0644]
LayoutTests/svg/repaint/remove-background-property-on-root-expected.html [new file with mode: 0644]
LayoutTests/svg/repaint/remove-background-property-on-root.html [new file with mode: 0644]
LayoutTests/svg/repaint/remove-border-property-on-root-expected.html [new file with mode: 0644]
LayoutTests/svg/repaint/remove-border-property-on-root.html [new file with mode: 0644]
LayoutTests/svg/repaint/remove-outline-property-on-root-expected.html [new file with mode: 0644]
LayoutTests/svg/repaint/remove-outline-property-on-root.html [new file with mode: 0644]
LayoutTests/svg/repaint/repaint-webkit-svg-shadow-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/rendering/svg/RenderSVGRoot.cpp

index b0b7705..acb4e11 100644 (file)
@@ -1,3 +1,35 @@
+2015-10-09  Antoine Quint  <graouts@apple.com>
+
+        Dynamic background color changes do not update until a layout is forced
+        https://bugs.webkit.org/show_bug.cgi?id=131623
+
+        Add some new tests checking that dynamically updating the "background",
+        "border" and "outline" CSS properties repaint correctly and rebase a few
+        existing tests that yield the same rendered results but slightly different
+        DRT output.
+
+        Reviewed by Darin Adler.
+
+        * platform/mac/fast/repaint/moving-shadow-on-container-expected.txt:
+        * platform/mac/svg/custom/simple-text-double-shadow-expected.txt:
+        * svg/css/composite-shadow-example-expected.txt:
+        * svg/css/composite-shadow-with-opacity-expected.txt:
+        * svg/repaint/add-background-property-on-root-expected.html: Added.
+        * svg/repaint/add-background-property-on-root.html: Added.
+        * svg/repaint/add-border-property-on-root-expected.html: Added.
+        * svg/repaint/add-border-property-on-root.html: Added.
+        * svg/repaint/add-outline-property-on-root-expected.html: Added.
+        * svg/repaint/add-outline-property-on-root.html: Added.
+        * svg/repaint/change-background-color-expected.html: Added.
+        * svg/repaint/change-background-color.html: Added.
+        * svg/repaint/remove-background-property-on-root-expected.html: Added.
+        * svg/repaint/remove-background-property-on-root.html: Added.
+        * svg/repaint/remove-border-property-on-root-expected.html: Added.
+        * svg/repaint/remove-border-property-on-root.html: Added.
+        * svg/repaint/remove-outline-property-on-root-expected.html: Added.
+        * svg/repaint/remove-outline-property-on-root.html: Added.
+        * svg/repaint/repaint-webkit-svg-shadow-expected.txt:
+
 2015-10-06  Simon Fraser  <simon.fraser@apple.com>
 
         Add some missing iOS results.
index d730508..e5e50ea 100644 (file)
@@ -3,7 +3,7 @@ layer at (0,0) size 785x616
 layer at (0,0) size 785x616
   RenderBlock {HTML} at (0,0) size 785x616
     RenderBody {BODY} at (8,8) size 769x600
-      RenderSVGRoot {svg} at (8,8) size 496x201
+      RenderSVGRoot {svg} at (8,8) size 472x177
         RenderSVGPath {path} at (8,8) size 80x70 [stroke={[type=SOLID] [color=#000000] [stroke width=10.00]}] [fill={[type=SOLID] [color=#999999]}] [data="M 1.83697e-15 30 L -35.2671 48.541 L -28.5317 9.27051 L -57.0634 -18.541 L -17.6336 -24.2705 L -1.10218e-14 -60 L 17.6336 -24.2705 L 57.0634 -18.541 L 28.5317 9.27051 L 35.2671 48.541 Z"]
         RenderSVGPath {path} at (198,44) size 132x129 [transform={m=((1.00,0.00)(0.00,1.00)) t=(250.00,100.00)}] [stroke={[type=SOLID] [color=#000000] [stroke width=10.00] [dash array={20.00}]}] [fill={[type=SOLID] [color=#999999]}] [data="M 1.83697e-15 30 L -35.2671 48.541 L -28.5317 9.27051 L -57.0634 -18.541 L -17.6336 -24.2705 L -1.10218e-14 -60 L 17.6336 -24.2705 L 57.0634 -18.541 L 28.5317 9.27051 L 35.2671 48.541 Z"]
         RenderSVGPath {path} at (347,44) size 121x126 [transform={m=((1.00,0.00)(0.00,1.00)) t=(400.00,100.00)}] [stroke={[type=SOLID] [color=#000000] [stroke width=10.00] [dash array={20.00}]}] [fill={[type=SOLID] [color=#999999]}] [data="M 1.53081e-15 25 L -29.3893 40.4509 L -23.7764 7.72542 L -47.5528 -15.4508 L -14.6946 -20.2254 L -9.18485e-15 -50 L 14.6946 -20.2254 L 47.5528 -15.4508 L 23.7764 7.72542 L 29.3893 40.4509 Z"]
index dbcd11b..c043fdc 100644 (file)
@@ -1,7 +1,7 @@
 layer at (0,0) size 800x600
   RenderView at (0,0) size 800x600
 layer at (0,0) size 800x600
-  RenderSVGRoot {svg} at (20,26) size 480x294
+  RenderSVGRoot {svg} at (20,26) size 380x194
     RenderSVGText {text} at (20,26) size 221x18 contains 1 chunk(s)
       RenderSVGInlineText {#text} at (0,0) size 221x18
         chunk 1 text run 1 at (20.00,40.00) startOffset 0 endOffset 34 width 220.84: "This text should appear only twice"
index 342021f..62724ae 100644 (file)
@@ -3,7 +3,7 @@ layer at (0,0) size 785x616
 layer at (0,0) size 785x616
   RenderBlock {HTML} at (0,0) size 785x616
     RenderBody {BODY} at (8,8) size 769x600
-      RenderSVGRoot {svg} at (32,28) size 472x186
+      RenderSVGRoot {svg} at (36,32) size 444x158
         RenderSVGContainer {g} at (36,32) size 164x158
           RenderSVGPath {path} at (38,34) size 150x144 [transform={m=((1.00,0.00)(0.00,1.00)) t=(100.00,100.00)}] [stroke={[type=SOLID] [color=#000000] [stroke width=10.00]}] [fill={[type=SOLID] [color=#999999]}] [data="M 1.83697e-15 30 L -35.2671 48.541 L -28.5317 9.27051 L -57.0634 -18.541 L -17.6336 -24.2705 L -1.10218e-14 -60 L 17.6336 -24.2705 L 57.0634 -18.541 L 28.5317 9.27051 L 35.2671 48.541 Z"]
         RenderSVGContainer {g} at (196,42) size 146x143
index 1ab885b..eba648b 100644 (file)
@@ -4,7 +4,7 @@ layer at (0,0) size 785x616
   RenderBlock {HTML} at (0,0) size 785x616
     RenderBody {BODY} at (8,8) size 769x600
 layer at (8,8) size 769x600
-  RenderSVGRoot {svg} at (32,28) size 472x186 [opacity=0.50]
+  RenderSVGRoot {svg} at (36,32) size 444x158 [opacity=0.50]
     RenderSVGContainer {g} at (36,32) size 164x158
       RenderSVGPath {path} at (38,34) size 150x144 [transform={m=((1.00,0.00)(0.00,1.00)) t=(100.00,100.00)}] [stroke={[type=SOLID] [color=#000000] [stroke width=10.00]}] [fill={[type=SOLID] [color=#999999]}] [data="M 1.83697e-15 30 L -35.2671 48.541 L -28.5317 9.27051 L -57.0634 -18.541 L -17.6336 -24.2705 L -1.10218e-14 -60 L 17.6336 -24.2705 L 57.0634 -18.541 L 28.5317 9.27051 L 35.2671 48.541 Z"]
     RenderSVGContainer {g} at (196,42) size 146x143
diff --git a/LayoutTests/svg/repaint/add-background-property-on-root-expected.html b/LayoutTests/svg/repaint/add-background-property-on-root-expected.html
new file mode 100644 (file)
index 0000000..a13175d
--- /dev/null
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<div style="width: 100px; height: 100px; background: red">
+    <svg width="100" height="100" style="background: green"></svg>
+</div>
diff --git a/LayoutTests/svg/repaint/add-background-property-on-root.html b/LayoutTests/svg/repaint/add-background-property-on-root.html
new file mode 100644 (file)
index 0000000..e9e630d
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+window.onload = function() {
+    requestAnimationFrame(function() {
+        document.querySelector("svg").style.background = "green";
+        if (window.testRunner)
+            window.testRunner.notifyDone();
+    });
+};
+</script>
+<div style="width: 100px; height: 100px; background: red">
+    <svg width="100" height="100"></svg>
+</div>
diff --git a/LayoutTests/svg/repaint/add-border-property-on-root-expected.html b/LayoutTests/svg/repaint/add-border-property-on-root-expected.html
new file mode 100644 (file)
index 0000000..cab012f
--- /dev/null
@@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<svg width="100" height="100" style="position: absolute; border: 10px solid green"></svg>
diff --git a/LayoutTests/svg/repaint/add-border-property-on-root.html b/LayoutTests/svg/repaint/add-border-property-on-root.html
new file mode 100644 (file)
index 0000000..699a0bf
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+window.onload = function() {
+    requestAnimationFrame(function() {
+        document.querySelector("svg").style.border = "10px solid green";
+        if (window.testRunner)
+            window.testRunner.notifyDone();
+    });
+};
+</script>
+<svg width="100" height="100" style="position: absolute"></svg>
diff --git a/LayoutTests/svg/repaint/add-outline-property-on-root-expected.html b/LayoutTests/svg/repaint/add-outline-property-on-root-expected.html
new file mode 100644 (file)
index 0000000..ba621b1
--- /dev/null
@@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<svg width="100" height="100" style="outline: 10px solid green"><rect width="100" height="100" fill="blue"/></svg>
diff --git a/LayoutTests/svg/repaint/add-outline-property-on-root.html b/LayoutTests/svg/repaint/add-outline-property-on-root.html
new file mode 100644 (file)
index 0000000..766de71
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+window.onload = function() {
+    requestAnimationFrame(function() {
+        document.querySelector("svg").style.outline = "10px solid green";
+        if (window.testRunner)
+            window.testRunner.notifyDone();
+    });
+};
+</script>
+<svg width="100" height="100">
+    <rect width="100" height="100" fill="blue"/>
+</svg>
diff --git a/LayoutTests/svg/repaint/change-background-color-expected.html b/LayoutTests/svg/repaint/change-background-color-expected.html
new file mode 100644 (file)
index 0000000..02456bc
--- /dev/null
@@ -0,0 +1,2 @@
+<!DOCTYPE HTML>
+<svg width="200px" height="100px" viewBox="0 0 100 100" style="background-color: green"></svg>
diff --git a/LayoutTests/svg/repaint/change-background-color.html b/LayoutTests/svg/repaint/change-background-color.html
new file mode 100644 (file)
index 0000000..0355dee
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE HTML>
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+window.onload = function() {
+    requestAnimationFrame(function() {
+        document.querySelector("svg").style.backgroundColor = "green";
+        if (window.testRunner)
+            window.testRunner.notifyDone();
+    });
+};
+</script>
+<svg width="200px" height="100px" viewBox="0 0 100 100" style="background-color: red"></svg>
diff --git a/LayoutTests/svg/repaint/remove-background-property-on-root-expected.html b/LayoutTests/svg/repaint/remove-background-property-on-root-expected.html
new file mode 100644 (file)
index 0000000..ebd2d86
--- /dev/null
@@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<div style="width: 100px; height: 100px; background: green"></div>
diff --git a/LayoutTests/svg/repaint/remove-background-property-on-root.html b/LayoutTests/svg/repaint/remove-background-property-on-root.html
new file mode 100644 (file)
index 0000000..bfdefb8
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+window.onload = function() {
+    requestAnimationFrame(function() {
+        document.querySelector("svg").style.removeProperty("background");
+        if (window.testRunner)
+            window.testRunner.notifyDone();
+    });
+};
+</script>
+<div style="width: 100px; height: 100px; background: green">
+    <svg style="background: red" width="100" height="100"></svg>
+</div>
diff --git a/LayoutTests/svg/repaint/remove-border-property-on-root-expected.html b/LayoutTests/svg/repaint/remove-border-property-on-root-expected.html
new file mode 100644 (file)
index 0000000..aa7a8e8
--- /dev/null
@@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<svg style="position: absolute" width="100" height="100"></svg>
diff --git a/LayoutTests/svg/repaint/remove-border-property-on-root.html b/LayoutTests/svg/repaint/remove-border-property-on-root.html
new file mode 100644 (file)
index 0000000..cd7a511
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+window.onload = function() {
+    requestAnimationFrame(function() {
+        document.querySelector("svg").style.removeProperty("border");
+        if (window.testRunner)
+            window.testRunner.notifyDone();
+    });
+};
+</script>
+<svg style="border: 10px solid red; position: absolute" width="100" height="100"></svg>
diff --git a/LayoutTests/svg/repaint/remove-outline-property-on-root-expected.html b/LayoutTests/svg/repaint/remove-outline-property-on-root-expected.html
new file mode 100644 (file)
index 0000000..faa9033
--- /dev/null
@@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<svg width="100" height="100"><rect width="100" height="100" fill="green"/></svg>
diff --git a/LayoutTests/svg/repaint/remove-outline-property-on-root.html b/LayoutTests/svg/repaint/remove-outline-property-on-root.html
new file mode 100644 (file)
index 0000000..40e56f3
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<script>
+if (window.testRunner)
+    testRunner.waitUntilDone();
+
+window.onload = function() {
+    requestAnimationFrame(function() {
+        document.querySelector("svg").style.removeProperty("outline");
+        if (window.testRunner)
+            window.testRunner.notifyDone();
+    });
+};
+</script>
+<svg style="outline: 10px solid red" width="100" height="100">
+    <rect width="100" height="100" fill="green"/>
+</svg>
index f99975d..52671e7 100644 (file)
@@ -1,6 +1,6 @@
 layer at (0,0) size 800x600
   RenderView at (0,0) size 800x600
 layer at (0,0) size 800x600
-  RenderSVGRoot {svg} at (120,0) size 450x520
+  RenderSVGRoot {svg} at (120,0) size 250x320
     RenderSVGRect {rect} at (150,100) size 120x120 [transform={m=((1.00,0.00)(0.00,1.00)) t=(150.00,50.00)}] [fill={[type=SOLID] [color=#000000]}] [x=0.00] [y=50.00] [width=20.00] [height=20.00]
     RenderSVGRect {rect} at (120,0) size 120x120 [fill={[type=SOLID] [color=#000000]}] [x=120.00] [y=0.00] [width=20.00] [height=20.00]
index c4a327d..6fa7a57 100644 (file)
@@ -1,3 +1,37 @@
+2015-10-09  Antoine Quint  <graouts@apple.com>
+
+        Dynamic background color changes do not update until a layout is forced
+        https://bugs.webkit.org/show_bug.cgi?id=131623
+
+        Compute correct repaint rect for decorated RenderSVGRoots.
+
+        The current implementation of clippedOverflowRectForRepaint() uses the
+        generic repaint-rect calculations in SVGRenderSupport. Those in turn make
+        use of repaintRectInLocalCoordinates(), which for RenderSVGRoot is the
+        union of the painted children (w/ some expansion). If there're no children,
+        or they do not fill the entire content box, then a repaint would not
+        repaint the correct parts.
+        Fix by calculating the union of the border-box and the SVG content
+        when the SVG root is decorated (has background/border/etc.)
+
+        Adapted from a Chromium patch by fs@opera.com
+        https://src.chromium.org/viewvc/blink?revision=170890&view=revision
+
+        Reviewed by Darin Adler.
+
+        Tests: svg/repaint/add-background-property-on-root.html
+               svg/repaint/add-border-property-on-root.html
+               svg/repaint/add-outline-property-on-root.html
+               svg/repaint/change-background-color.html
+               svg/repaint/remove-background-property-on-root.html
+               svg/repaint/remove-border-property-on-root.html
+               svg/repaint/remove-outline-property-on-root.html
+
+        * rendering/svg/RenderSVGRoot.cpp:
+        (WebCore::RenderSVGRoot::layout):
+        (WebCore::RenderSVGRoot::styleDidChange):
+        (WebCore::RenderSVGRoot::clippedOverflowRectForRepaint):
+
 2015-10-09  Simon Fraser  <simon.fraser@apple.com>
 
         [iOS WK2] Fix assertion in ViewportConfiguration::setDefaultConfiguration seen in testing
index 2a862db..e42f010 100644 (file)
@@ -300,6 +300,11 @@ void RenderSVGRoot::styleDidChange(StyleDifference diff, const RenderStyle* oldS
 {
     if (diff == StyleDifferenceLayout)
         setNeedsBoundariesUpdate();
+
+    // Box decorations may have appeared/disappeared - recompute status.
+    if (diff == StyleDifferenceRepaint)
+        m_hasBoxDecorations = hasBoxDecorationStyle();
+
     RenderReplaced::styleDidChange(diff, oldStyle);
     SVGResourcesCache::clientStyleChanged(*this, diff, style());
 }
@@ -342,7 +347,17 @@ const AffineTransform& RenderSVGRoot::localToParentTransform() const
 
 LayoutRect RenderSVGRoot::clippedOverflowRectForRepaint(const RenderLayerModelObject* repaintContainer) const
 {
-    return SVGRenderSupport::clippedOverflowRectForRepaint(*this, repaintContainer);
+    if (style().visibility() != VISIBLE && !enclosingLayer()->hasVisibleContent())
+        return LayoutRect();
+
+    FloatRect contentRepaintRect = m_localToBorderBoxTransform.mapRect(repaintRectInLocalCoordinates());
+    contentRepaintRect.intersect(snappedIntRect(borderBoxRect()));
+
+    LayoutRect repaintRect = enclosingLayoutRect(contentRepaintRect);
+    if (m_hasBoxDecorations || hasRenderOverflow())
+        repaintRect.unite(unionRect(localSelectionRect(false), visualOverflowRect()));
+
+    return RenderReplaced::computeRectForRepaint(enclosingIntRect(repaintRect), repaintContainer);
 }
 
 FloatRect RenderSVGRoot::computeFloatRectForRepaint(const FloatRect& repaintRect, const RenderLayerModelObject* repaintContainer, bool fixed) const