Bug 728028. CG: Avoid double transforming filled gradients. r=mwoodrow
authorJeff Muizelaar <jmuizelaar@mozilla.com>
Sat, 25 Feb 2012 00:35:39 -0500
changeset 90593 0de785d6345a9af7c6dd7262c3accf0882f4a237
parent 90592 c1c6cd6c52b7dfbc9be40b377b1259256a464f08
child 90594 a6589012675086b98bf5591bfef5c62590b6064f
push idunknown
push userunknown
push dateunknown
reviewersmwoodrow
bugs728028
milestone13.0a1
Bug 728028. CG: Avoid double transforming filled gradients. r=mwoodrow When we fixed transformed clips it caused us to double transform gradients. We fix this by avoiding ::PushClip when drawing gradients. This has the advantage of saving a save and restore pair and it makes Fill() more closely match Stroke()
gfx/2d/DrawTargetCG.cpp
layout/reftests/canvas/reftest.list
layout/reftests/canvas/transformed-gradient-ref.html
layout/reftests/canvas/transformed-gradient.html
--- a/gfx/2d/DrawTargetCG.cpp
+++ b/gfx/2d/DrawTargetCG.cpp
@@ -658,25 +658,36 @@ DrawTargetCG::Fill(const Path *aPath, co
 
   CGContextSetBlendMode(mCg, ToBlendMode(aDrawOptions.mCompositionOp));
   UnboundnessFixer fixer;
   CGContextRef cg = fixer.Check(mCg, aDrawOptions.mCompositionOp);
   CGContextSetAlpha(cg, aDrawOptions.mAlpha);
 
   CGContextConcatCTM(cg, GfxMatrixToCGAffineTransform(mTransform));
 
+  CGContextBeginPath(cg);
+  // XXX: we could put fill mode into the path fill rule if we wanted
+  const PathCG *cgPath = static_cast<const PathCG*>(aPath);
+
   if (isGradient(aPattern)) {
-    // XXX: we should be able to avoid the extra SaveState that PushClip does
-    PushClip(aPath);
+    // setup a clip to draw the gradient through
+    if (CGPathIsEmpty(cgPath->GetPath())) {
+      // Adding an empty path will cause us not to clip
+      // so clip everything explicitly
+      CGContextClipToRect(mCg, CGRectZero);
+    } else {
+      CGContextAddPath(cg, cgPath->GetPath());
+      if (cgPath->GetFillRule() == FILL_EVEN_ODD)
+        CGContextEOClip(mCg);
+      else
+        CGContextClip(mCg);
+    }
+
     DrawGradient(cg, aPattern);
-    PopClip();
   } else {
-    CGContextBeginPath(cg);
-    // XXX: we could put fill mode into the path fill rule if we wanted
-    const PathCG *cgPath = static_cast<const PathCG*>(aPath);
     CGContextAddPath(cg, cgPath->GetPath());
 
     SetFillFromPattern(cg, mColorSpace, aPattern);
 
     if (cgPath->GetFillRule() == FILL_EVEN_ODD)
       CGContextEOFillPath(cg);
     else
       CGContextFillPath(cg);
--- a/layout/reftests/canvas/reftest.list
+++ b/layout/reftests/canvas/reftest.list
@@ -73,8 +73,9 @@ fails == ctm-singular-sanity.html data:t
 fails-if(/Mac\x20OS\x20X\x2010\.[56]/.test(http.oscpu)) == 672646-alpha-radial-gradient.html 672646-alpha-radial-gradient-ref.html # Bug 673333
 == 674003-alpha-radial-gradient-superlum.html 674003-alpha-radial-gradient-superlum-ref.html
 
 != 693610-1.html 693610-1-notref.html # bug 693610: multiple glyph runs should not be overprinted
 
 == 726951-shadow-clips.html 726951-shadow-clips-ref.html
 
 == transformed-clip.html transformed-clip-ref.html
+== transformed-gradient.html transformed-gradient-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/canvas/transformed-gradient-ref.html
@@ -0,0 +1,17 @@
+<html>
+<body>
+  <canvas width="500" height="500"></canvas>
+<script>
+  var canvas = document.getElementsByTagName('canvas')[0];
+  var ctx = canvas.getContext('2d');
+
+  var lineargradient = ctx.createLinearGradient(000,000,500,500);
+  lineargradient.addColorStop(0,'red');
+  lineargradient.addColorStop(1,'black');
+  ctx.fillStyle = lineargradient;
+  ctx.beginPath();
+  ctx.rect(00, 00, 250, 250);
+  ctx.fill();
+</script>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/canvas/transformed-gradient.html
@@ -0,0 +1,18 @@
+<html>
+<body>
+  <canvas width="500" height="500"></canvas>
+<script>
+  var canvas = document.getElementsByTagName('canvas')[0];
+  var ctx = canvas.getContext('2d');
+
+  ctx.translate(500, 500);
+  var lineargradient = ctx.createLinearGradient(-500,-500,0,0);
+  lineargradient.addColorStop(0,'red');
+  lineargradient.addColorStop(1,'black');
+  ctx.fillStyle = lineargradient;
+  ctx.beginPath();
+  ctx.rect(-500, -500, 250, 250);
+  ctx.fill();
+</script>
+</body>
+</html>