Title: [153701] trunk
Revision
153701
Author
za...@apple.com
Date
2013-08-04 14:29:00 -0700 (Sun, 04 Aug 2013)

Log Message

Background doesn't fully repaint when body has margins.
https://bugs.webkit.org/show_bug.cgi?id=119033

Reviewed by Simon Fraser.

Ensure that background-color changes do not leave unpainted areas when
body has margins.

Both <body> and <html> background-color get propagated up to the viewport.
If <body> has background-color attribute set, while <html> doesn't, the color is
applied not only on the <body> but on both the <html> and the viewport. However,
it's not enough to mark the RenderView dirty because with tiles backing on,
there could be areas outside of the viewport that need repaint. By marking
the RenderView's graphics layer dirty instead, we ensure that all the related
tiles get marked dirty too and the new background color covers all areas.

Manual test added. When forcing top-level composition on (even with embedded iframe to
make sure we don't do paintsIntoWindow rendering), the test case execution changes so much,
that the repaint rects don't reflect the functionality difference anymore.

.:

Reviewed by Simon Fraser.

* ManualTests/compositing/background-color-change-on-body-with-margin.html: Added.

Source/WebCore:

* page/FrameView.cpp:
(WebCore::FrameView::reset):
(WebCore::FrameView::layout):
* page/FrameView.h:
(WebCore::FrameView::needsFullRepaint):
* rendering/RenderBox.cpp:
(WebCore::RenderBox::styleWillChange):
* rendering/RenderObjectChildList.cpp:
(WebCore::RenderObjectChildList::removeChildNode):
* rendering/RenderView.cpp:
(WebCore::RenderView::repaintRootContents):
(WebCore::RenderView::repaintViewAndCompositedLayers):
* rendering/RenderView.h:

Modified Paths

Added Paths

Diff

Modified: trunk/ChangeLog (153700 => 153701)


--- trunk/ChangeLog	2013-08-04 16:17:32 UTC (rev 153700)
+++ trunk/ChangeLog	2013-08-04 21:29:00 UTC (rev 153701)
@@ -1,3 +1,29 @@
+2013-08-04  Zalan Bujtas  <za...@apple.com>
+
+        Background doesn't fully repaint when body has margins.
+        https://bugs.webkit.org/show_bug.cgi?id=119033
+
+        Reviewed by Simon Fraser.
+
+        Ensure that background-color changes do not leave unpainted areas when
+        body has margins.
+
+        Both <body> and <html> background-color get propagated up to the viewport.
+        If <body> has background-color attribute set, while <html> doesn't, the color is
+        applied not only on the <body> but on both the <html> and the viewport. However,
+        it's not enough to mark the RenderView dirty because with tiles backing on,
+        there could be areas outside of the viewport that need repaint. By marking
+        the RenderView's graphics layer dirty instead, we ensure that all the related
+        tiles get marked dirty too and the new background color covers all areas.
+
+        Manual test added. When forcing top-level composition on (even with embedded iframe to
+        make sure we don't do paintsIntoWindow rendering), the test case execution changes so much,
+        that the repaint rects don't reflect the functionality difference anymore.
+
+        Reviewed by Simon Fraser.
+
+        * ManualTests/compositing/background-color-change-on-body-with-margin.html: Added.
+
 2013-07-30  Ádám Kallai  <ka...@inf.u-szeged.hu>
 
         [Qt] Workaround to make syncqt run and generate forwarding headers in SVN repositories too.

Added: trunk/ManualTests/compositing/background-color-change-on-body-with-margin.html (0 => 153701)


--- trunk/ManualTests/compositing/background-color-change-on-body-with-margin.html	                        (rev 0)
+++ trunk/ManualTests/compositing/background-color-change-on-body-with-margin.html	2013-08-04 21:29:00 UTC (rev 153701)
@@ -0,0 +1,116 @@
+<html>
+<head>
+
+<style>
+body {
+  margin: 100px 100px 100px 100px;
+  background-color: red;
+}
+</style>
+
+<script>
+  function start() {
+    window.setTimeout("document.getElementsByTagName('body')[0].style.backgroundColor='white';", 500); 
+  }
+</script>
+</head>
+<body _onload_='start();'>
+  <p>This test verifies that we don't do partial repaint when background-color changes.<br>
+     Scroll to check if the background is white all the way down.</p>
+  <div id='content'>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+    Lorem ipsum dolor sit amet<br>
+  </div>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (153700 => 153701)


--- trunk/Source/WebCore/ChangeLog	2013-08-04 16:17:32 UTC (rev 153700)
+++ trunk/Source/WebCore/ChangeLog	2013-08-04 21:29:00 UTC (rev 153701)
@@ -1,3 +1,39 @@
+2013-08-04  Zalan Bujtas  <za...@apple.com>
+
+        Background doesn't fully repaint when body has margins.
+        https://bugs.webkit.org/show_bug.cgi?id=119033
+
+        Reviewed by Simon Fraser.
+
+        Ensure that background-color changes do not leave unpainted areas when
+        body has margins.
+
+        Both <body> and <html> background-color get propagated up to the viewport.
+        If <body> has background-color attribute set, while <html> doesn't, the color is
+        applied not only on the <body> but on both the <html> and the viewport. However,
+        it's not enough to mark the RenderView dirty because with tiles backing on,
+        there could be areas outside of the viewport that need repaint. By marking
+        the RenderView's graphics layer dirty instead, we ensure that all the related
+        tiles get marked dirty too and the new background color covers all areas.
+
+        Manual test added. When forcing top-level composition on (even with embedded iframe to
+        make sure we don't do paintsIntoWindow rendering), the test case execution changes so much,
+        that the repaint rects don't reflect the functionality difference anymore.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::reset):
+        (WebCore::FrameView::layout):
+        * page/FrameView.h:
+        (WebCore::FrameView::needsFullRepaint):
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::styleWillChange):
+        * rendering/RenderObjectChildList.cpp:
+        (WebCore::RenderObjectChildList::removeChildNode):
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::repaintRootContents):
+        (WebCore::RenderView::repaintViewAndCompositedLayers):
+        * rendering/RenderView.h:
+
 2013-08-04  Andreas Kling  <akl...@apple.com>
 
         Document needn't expose its active element.

Modified: trunk/Source/WebCore/page/FrameView.cpp (153700 => 153701)


--- trunk/Source/WebCore/page/FrameView.cpp	2013-08-04 16:17:32 UTC (rev 153700)
+++ trunk/Source/WebCore/page/FrameView.cpp	2013-08-04 21:29:00 UTC (rev 153701)
@@ -274,7 +274,7 @@
     m_layoutTimer.stop();
     m_layoutRoot = 0;
     m_delayedLayout = false;
-    m_doFullRepaint = true;
+    m_needsFullRepaint = true;
     m_layoutSchedulingEnabled = true;
     m_inLayout = false;
     m_doingPreLayoutStyleUpdate = false;
@@ -1259,7 +1259,7 @@
         ScrollbarMode vMode;    
         calculateScrollbarModesForLayout(hMode, vMode);
 
-        m_doFullRepaint = !subtree && (m_firstLayout || toRenderView(root)->printing());
+        m_needsFullRepaint = !subtree && (m_firstLayout || toRenderView(root)->printing());
 
         if (!subtree) {
             // Now set our scrollbar state for the layout.
@@ -1296,7 +1296,7 @@
             m_size = LayoutSize(layoutWidth(), layoutHeight());
 
             if (oldSize != m_size) {
-                m_doFullRepaint = true;
+                m_needsFullRepaint = true;
                 if (!m_firstLayout) {
                     RenderBox* rootRenderer = document->documentElement() ? document->documentElement()->renderBox() : 0;
                     RenderBox* bodyRenderer = rootRenderer && document->body() ? document->body()->renderBox() : 0;
@@ -1339,20 +1339,19 @@
         m_layoutRoot = 0;
     } // Reset m_layoutSchedulingEnabled to its previous value.
 
-    bool neededFullRepaint = m_doFullRepaint;
+    bool neededFullRepaint = m_needsFullRepaint;
 
     if (!subtree && !toRenderView(root)->printing())
         adjustViewSize();
 
-    m_doFullRepaint = neededFullRepaint;
+    m_needsFullRepaint = neededFullRepaint;
 
     // Now update the positions of all layers.
     beginDeferredRepaints();
-    if (m_doFullRepaint)
-        root->view()->repaint(); // FIXME: This isn't really right, since the RenderView doesn't fully encompass the visibleContentRect(). It just happens
-                                 // to work out most of the time, since first layouts and printing don't have you scrolled anywhere.
+    if (m_needsFullRepaint)
+        root->view()->repaintRootContents();
 
-    layer->updateLayerPositionsAfterLayout(renderView()->layer(), updateLayerPositionFlags(layer, subtree, m_doFullRepaint));
+    layer->updateLayerPositionsAfterLayout(renderView()->layer(), updateLayerPositionFlags(layer, subtree, m_needsFullRepaint));
 
     endDeferredRepaints();
 

Modified: trunk/Source/WebCore/page/FrameView.h (153700 => 153701)


--- trunk/Source/WebCore/page/FrameView.h	2013-08-04 16:17:32 UTC (rev 153700)
+++ trunk/Source/WebCore/page/FrameView.h	2013-08-04 21:29:00 UTC (rev 153701)
@@ -115,7 +115,7 @@
     void setNeedsLayout();
     void setViewportConstrainedObjectsNeedLayout();
 
-    bool needsFullRepaint() const { return m_doFullRepaint; }
+    bool needsFullRepaint() const { return m_needsFullRepaint; }
 
 #if ENABLE(REQUEST_ANIMATION_FRAME)
     void serviceScriptedAnimations(double monotonicAnimationStartTime);
@@ -555,7 +555,7 @@
 
     OwnPtr<RenderObjectSet> m_slowRepaintObjects;
 
-    bool m_doFullRepaint;
+    bool m_needsFullRepaint;
     
     bool m_canHaveScrollbars;
     bool m_cannotBlitToWindow;

Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (153700 => 153701)


--- trunk/Source/WebCore/rendering/RenderBox.cpp	2013-08-04 16:17:32 UTC (rev 153700)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp	2013-08-04 21:29:00 UTC (rev 153701)
@@ -206,11 +206,9 @@
     RenderStyle* oldStyle = style();
     if (oldStyle) {
         // The background of the root element or the body element could propagate up to
-        // the canvas.  Just dirty the entire canvas when our style changes substantially.
-        if (diff >= StyleDifferenceRepaint && node() &&
-            (node()->hasTagName(htmlTag) || node()->hasTagName(bodyTag))) {
-            view()->repaint();
-            
+        // the canvas. Issue full repaint, when our style changes substantially.
+        if (diff >= StyleDifferenceRepaint && (isRoot() || isBody())) {
+            view()->repaintRootContents();
 #if USE(ACCELERATED_COMPOSITING)
             if (oldStyle->hasEntirelyFixedBackground() != newStyle->hasEntirelyFixedBackground())
                 view()->compositor()->rootFixedBackgroundsChanged();
@@ -229,7 +227,7 @@
                 removeFloatingOrPositionedChildFromBlockLists();
         }
     } else if (newStyle && isBody())
-        view()->repaint();
+        view()->repaintRootContents();
 
     RenderBoxModelObject::styleWillChange(diff, newStyle);
 }

Modified: trunk/Source/WebCore/rendering/RenderObjectChildList.cpp (153700 => 153701)


--- trunk/Source/WebCore/rendering/RenderObjectChildList.cpp	2013-08-04 16:17:32 UTC (rev 153700)
+++ trunk/Source/WebCore/rendering/RenderObjectChildList.cpp	2013-08-04 21:29:00 UTC (rev 153701)
@@ -67,7 +67,7 @@
         oldChild->setNeedsLayoutAndPrefWidthsRecalc();
         // We only repaint |oldChild| if we have a RenderLayer as its visual overflow may not be tracked by its parent.
         if (oldChild->isBody())
-            owner->view()->repaint();
+            owner->view()->repaintRootContents();
         else
             oldChild->repaint();
     }

Modified: trunk/Source/WebCore/rendering/RenderView.cpp (153700 => 153701)


--- trunk/Source/WebCore/rendering/RenderView.cpp	2013-08-04 16:17:32 UTC (rev 153700)
+++ trunk/Source/WebCore/rendering/RenderView.cpp	2013-08-04 21:29:00 UTC (rev 153701)
@@ -526,6 +526,17 @@
     return true;
 }
 
+void RenderView::repaintRootContents()
+{
+#if USE(ACCELERATED_COMPOSITING)
+    if (layer()->isComposited()) {
+        layer()->setBackingNeedsRepaint();
+        return;
+    }
+#endif
+    repaint();
+}
+
 void RenderView::repaintViewRectangle(const LayoutRect& ur, bool immediate) const
 {
     if (!shouldRepaint(ur))
@@ -567,8 +578,7 @@
 
 void RenderView::repaintViewAndCompositedLayers()
 {
-    repaint();
-    
+    repaintRootContents();
 #if USE(ACCELERATED_COMPOSITING)
     if (compositor()->inCompositingMode())
         compositor()->repaintCompositedLayers();

Modified: trunk/Source/WebCore/rendering/RenderView.h (153700 => 153701)


--- trunk/Source/WebCore/rendering/RenderView.h	2013-08-04 16:17:32 UTC (rev 153700)
+++ trunk/Source/WebCore/rendering/RenderView.h	2013-08-04 21:29:00 UTC (rev 153701)
@@ -76,6 +76,7 @@
 
     virtual LayoutRect visualOverflowRect() const OVERRIDE;
     virtual void computeRectForRepaint(const RenderLayerModelObject* repaintContainer, LayoutRect&, bool fixed = false) const OVERRIDE;
+    void repaintRootContents();
     void repaintViewRectangle(const LayoutRect&, bool immediate = false) const;
     // Repaint the view, and all composited layers that intersect the given absolute rectangle.
     // FIXME: ideally we'd never have to do this, if all repaints are container-relative.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to