2010-06-11 James Robinson <jamesr@chromium.org>
authorjamesr@google.com <jamesr@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Jun 2010 21:08:05 +0000 (21:08 +0000)
committerjamesr@google.com <jamesr@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Jun 2010 21:08:05 +0000 (21:08 +0000)
        Reviewed by Dimitri Glazkov.

        [chromium] Skia mispaints pages with border-radius

        Skia mispaints pages that have border radius set in some cases. The bug is in the anti aliased
        clip path logic used to implement anti aliased curves in Skia.  Since Skia internally only supports
        1-bit clips, anti aliased clipping is emulated by creating a new alpha layer, storing a set of
        clip paths on the side, and then 'erasing' the regions outside the clip.  See r49641.
        PlatformContextSkia maintains a stack of PlatformContextSkia::State objects that preserve information
        like fill color, drawing mode, etc that is manipulated by GraphicsContext::save() /
        GraphicsContext::restore() calls as well some internal functions.  Whenever a new State object is pushed
        a new copy of the current State object is pushed onto the top of this stack using the copy c'tor.  The
        set of anti alias clip paths is also stored on the State object, but not copied when new entries are
        added as the paths only apply to that entry on the stack.

        The bug is that the state stack is stored in a WTF::Vector.  When this vector exceeds its capacity
        (by default at 16 elements) all of the existing State entries are copied into the new buffer using
        State's copy constructor.  This does not preserve the anti alias clip paths, so when the State entries
        are popped the anti aliasing info is lost.  This corrupts all further paint operations since it results
        in inbalanced save/restore calls to the underlying SkCanvas.

        The fix is to make the PlatformContextSkia::State copy constructor copy all fields and to add a new
        function PlatformContextSkia::State::cloneInheritedProperties to use when pushing new State entries
        that copies everything except for the anti aliased clip paths.

        Test: fast/css/nested-rounded-corners.html

        * platform/graphics/skia/PlatformContextSkia.cpp:
2010-06-11  James Robinson  <jamesr@chromium.org>

        Reviewed by Dimitri Glazkov.

        [chromium] Skia mispaints pages with border-radius

        Tests deeply nested divs with border-radius.

        * fast/css/nested-rounded-corners-expected.txt: Added.
        * fast/css/nested-rounded-corners.html: Added.
        * platform/chromium/test_expectations.txt
        * platform/chromium-linux/fast/css/nested-rounded-corners-expected.checksum: Added.
        * platform/chromium-linux/fast/css/nested-rounded-corners-expected.png: Added.

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

LayoutTests/fast/css/nested-rounded-corners-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/nested-rounded-corners.html [new file with mode: 0644]
LayoutTests/platform/chromium-linux/fast/css/nested-rounded-corners-expected.checksum [new file with mode: 0644]
LayoutTests/platform/chromium-linux/fast/css/nested-rounded-corners-expected.png [new file with mode: 0644]

index 46e24a0..a4f5892 100644 (file)
@@ -1,3 +1,18 @@
+2010-06-11  James Robinson  <jamesr@chromium.org>
+        Reviewed by Dimitri Glazkov.
+        [chromium] Skia mispaints pages with border-radius
+        https://bugs.webkit.org/show_bug.cgi?id=40456
+        Tests deeply nested divs with border-radius.
+        * fast/css/nested-rounded-corners-expected.txt: Added.
+        * fast/css/nested-rounded-corners.html: Added.
+        * platform/chromium/test_expectations.txt
+        * platform/chromium-linux/fast/css/nested-rounded-corners-expected.checksum: Added.
+        * platform/chromium-linux/fast/css/nested-rounded-corners-expected.png: Added.
 2010-06-11  Eric Seidel  <eric@webkit.org>
         Reviewed by Adam Barth.
diff --git a/LayoutTests/fast/css/nested-rounded-corners-expected.txt b/LayoutTests/fast/css/nested-rounded-corners-expected.txt
new file mode 100644 (file)
index 0000000..d444708
--- /dev/null
@@ -0,0 +1,32 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+layer at (8,8) size 784x495 clip at (8,8) size 769x480
+  RenderBlock {DIV} at (0,0) size 784x495
+layer at (8,8) size 769x480 clip at (28,28) size 729x440
+  RenderBlock {DIV} at (0,0) size 769x480 [border: (20px solid #008000)]
+layer at (28,28) size 729x440 clip at (48,48) size 689x400
+  RenderBlock {DIV} at (20,20) size 729x440 [border: (20px solid #0000FF)]
+layer at (48,48) size 689x400 clip at (68,68) size 649x360
+  RenderBlock {DIV} at (20,20) size 689x400 [border: (20px solid #008000)]
+layer at (68,68) size 649x360 clip at (88,88) size 609x320
+  RenderBlock {DIV} at (20,20) size 649x360 [border: (20px solid #0000FF)]
+layer at (88,88) size 609x320 clip at (108,108) size 569x280
+  RenderBlock {DIV} at (20,20) size 609x320 [border: (20px solid #008000)]
+layer at (108,108) size 569x280 clip at (128,128) size 529x240
+  RenderBlock {DIV} at (20,20) size 569x280 [border: (20px solid #0000FF)]
+layer at (128,128) size 529x240 clip at (148,148) size 489x200
+  RenderBlock {DIV} at (20,20) size 529x240 [border: (20px solid #008000)]
+layer at (148,148) size 489x200 clip at (168,168) size 449x160
+  RenderBlock {DIV} at (20,20) size 489x200 [border: (20px solid #0000FF)]
+layer at (168,168) size 449x160 clip at (188,188) size 409x120
+  RenderBlock {DIV} at (20,20) size 449x160 [border: (20px solid #008000)]
+layer at (188,188) size 409x120 clip at (208,208) size 369x80
+  RenderBlock {DIV} at (20,20) size 409x120 [border: (20px solid #0000FF)]
+layer at (208,208) size 369x80 clip at (228,228) size 329x40
+  RenderBlock {DIV} at (20,20) size 369x80 [border: (20px solid #008000)]
+layer at (228,228) size 329x40 clip at (0,0) size 0x0
+  RenderBlock {DIV} at (20,20) size 329x40 [border: (20px solid #0000FF)]
+    RenderBlock {DIV} at (20,20) size 289x0
diff --git a/LayoutTests/fast/css/nested-rounded-corners.html b/LayoutTests/fast/css/nested-rounded-corners.html
new file mode 100644 (file)
index 0000000..533b409
--- /dev/null
@@ -0,0 +1,57 @@
+<html xmlns="http://www.w3.org/1999/xhtml"> 
+<meta content="text/html; charset=UTF-8" http-equiv="content-type"> 
+.oauto {
+  overflow: scroll;
+.ohidden {
+  overflow: hidden;
+.border1 {
+  border: 20px solid green;
+  -webkit-border-radius: 50px;
+.border2 {
+  border: 20px solid blue;
+  -webkit-border-radius: 50px;
+<div class="oauto"> 
+  <div class="ohidden border1"> 
+    <div class="ohidden border2"> 
+      <div class="ohidden border1"> 
+        <div class="ohidden border2"> 
+          <div class="ohidden border1"> 
+            <div class="ohidden border2"> 
+              <div class="ohidden border1"> 
+                <div class="ohidden border2"> 
+                  <div class="ohidden border1"> 
+                    <div class="ohidden border2"> 
+                      <div class="ohidden border1"> 
+                        <div class="ohidden border2">
+                          <div> </div> 
+                        </div> 
+                      </div> 
+                    </div> 
+                  </div> 
+                </div> 
+              </div> 
+            </div> 
+          </div> 
+        </div> 
+      </div> 
+    </div> 
+  </div> 
diff --git a/LayoutTests/platform/chromium-linux/fast/css/nested-rounded-corners-expected.checksum b/LayoutTests/platform/chromium-linux/fast/css/nested-rounded-corners-expected.checksum
new file mode 100644 (file)
index 0000000..5a15989
--- /dev/null
@@ -0,0 +1 @@
\ No newline at end of file
diff --git a/LayoutTests/platform/chromium-linux/fast/css/nested-rounded-corners-expected.png b/LayoutTests/platform/chromium-linux/fast/css/nested-rounded-corners-expected.png
new file mode 100644 (file)
index 0000000..404216e
Binary files /dev/null and b/LayoutTests/platform/chromium-linux/fast/css/nested-rounded-corners-expected.png differ
index abc5dff..80b627d 100644 (file)
@@ -2790,6 +2790,8 @@ BUGWK38705 : http/tests/security/sandbox-inherit-to-initial-document-2.html = TE
 BUG43495 LINUX : svg/W3C-SVG-1.1/render-groups-01-b.svg = IMAGE
 BUG43495 LINUX : svg/W3C-SVG-1.1/render-groups-03-t.svg = IMAGE
+// Needs new baselines
+BUGJAMESR WIN MAC : fast/css/nested-rounded-corners.html = IMAGE
 // WebKit roll 58791:58807
 BUG43319 MAC : transforms/2d/zoom-menulist.html = IMAGE PASS
index 20f494c..4c912c2 100644 (file)
@@ -1,3 +1,38 @@
+2010-06-11  James Robinson  <jamesr@chromium.org>
+        Reviewed by Dimitri Glazkov.
+        [chromium] Skia mispaints pages with border-radius
+        https://bugs.webkit.org/show_bug.cgi?id=40456
+        Skia mispaints pages that have border radius set in some cases. The bug is in the anti aliased
+        clip path logic used to implement anti aliased curves in Skia.  Since Skia internally only supports
+        1-bit clips, anti aliased clipping is emulated by creating a new alpha layer, storing a set of
+        clip paths on the side, and then 'erasing' the regions outside the clip.  See r49641.
+        PlatformContextSkia maintains a stack of PlatformContextSkia::State objects that preserve information
+        like fill color, drawing mode, etc that is manipulated by GraphicsContext::save() / 
+        GraphicsContext::restore() calls as well some internal functions.  Whenever a new State object is pushed
+        a new copy of the current State object is pushed onto the top of this stack using the copy c'tor.  The
+        set of anti alias clip paths is also stored on the State object, but not copied when new entries are
+        added as the paths only apply to that entry on the stack.
+        The bug is that the state stack is stored in a WTF::Vector.  When this vector exceeds its capacity
+        (by default at 16 elements) all of the existing State entries are copied into the new buffer using
+        State's copy constructor.  This does not preserve the anti alias clip paths, so when the State entries
+        are popped the anti aliasing info is lost.  This corrupts all further paint operations since it results
+        in inbalanced save/restore calls to the underlying SkCanvas.
+        The fix is to make the PlatformContextSkia::State copy constructor copy all fields and to add a new
+        function PlatformContextSkia::State::cloneInheritedProperties to use when pushing new State entries
+        that copies everything except for the anti aliased clip paths.
+        Test: fast/css/nested-rounded-corners.html
+        * platform/graphics/skia/PlatformContextSkia.cpp:
+        (PlatformContextSkia::State::State):
+        (PlatformContextSkia::State::cloneInheritedProperties):
+        (PlatformContextSkia::save):
 2010-06-11  Jeremy Orlow  <jorlow@chromium.org>
         Reviewed by Darin Fisher.
index 1d441d9..15bd9b4 100644 (file)
@@ -101,6 +101,7 @@ struct PlatformContextSkia::State {
     WTF::Vector<SkPath> m_antiAliasClipPaths;
     WebCore::InterpolationQuality m_interpolationQuality;
+    PlatformContextSkia::State cloneInheritedProperties();
     // Not supported.
     void operator=(const State&);
@@ -149,6 +150,7 @@ PlatformContextSkia::State::State(const State& other)
     , m_imageBufferClip(other.m_imageBufferClip)
     , m_clip(other.m_clip)
+    , m_antiAliasClipPaths(other.m_antiAliasClipPaths)
     , m_interpolationQuality(other.m_interpolationQuality)
     // Up the ref count of these. saveRef does nothing if 'this' is NULL.
@@ -166,6 +168,17 @@ PlatformContextSkia::State::~State()
+// Returns a new State with all of this object's inherited properties copied.
+PlatformContextSkia::State PlatformContextSkia::State::cloneInheritedProperties()
+    PlatformContextSkia::State state(*this);
+    // Everything is inherited except for the clip paths.
+    state.m_antiAliasClipPaths.clear();  
+    return state;
 SkColor PlatformContextSkia::State::applyAlpha(SkColor c) const
     int s = roundf(m_alpha * 256);
@@ -216,7 +229,7 @@ void PlatformContextSkia::save()
-    m_stateStack.append(*m_state);
+    m_stateStack.append(m_state->cloneInheritedProperties());
     m_state = &m_stateStack.last();