Move overdraw tracking code from GraphicsContext to CanvasRenderingContext2D

Source/core/html/canvas/CanvasRenderingContext2D.cpp

Index: Source/core/html/canvas/CanvasRenderingContext2D.cpp
diff --git a/Source/core/html/canvas/CanvasRenderingContext2D.cpp b/Source/core/html/canvas/CanvasRenderingContext2D.cpp
index 9f3346a862e6e8964aa072a165c91ade2dd36a4f..aebebe6b318d1c7eeb3121069b61758d5b2d09ec 100644
--- a/Source/core/html/canvas/CanvasRenderingContext2D.cpp
+++ b/Source/core/html/canvas/CanvasRenderingContext2D.cpp
@@ -283,6 +283,7 @@ CanvasRenderingContext2D::State::State()
     , m_unparsedFont(defaultFont)
     , m_realizedFont(false)
     , m_hasClip(false)
+    , m_hasComplexClip(false)
 {
 }
 
@@ -313,6 +314,7 @@ CanvasRenderingContext2D::State::State(const State& other, ClipListCopyMode mode
     , m_font(other.m_font)
     , m_realizedFont(other.m_realizedFont)
     , m_hasClip(other.m_hasClip)
+    , m_hasComplexClip(other.m_hasComplexClip)
 {
     if (mode == CopyClipList) {
         m_clipList = other.m_clipList;
@@ -355,6 +357,7 @@ CanvasRenderingContext2D::State& CanvasRenderingContext2D::State::operator=(cons
     m_font = other.m_font;
     m_realizedFont = other.m_realizedFont;
     m_hasClip = other.m_hasClip;
+    m_hasComplexClip = other.m_hasComplexClip;
     m_clipList = other.m_clipList;
 
     if (m_realizedFont)
@@ -1122,7 +1125,7 @@ void CanvasRenderingContext2D::clipInternal(const Path& path, const String& wind
 
     c->clipPath(skPath, SkRegion::kIntersect_Op, m_clipAntialiasing == AntiAliased);
     if (!skPath.isRect(0))
-        drawingContext()->setHasComplexClip();
+        modifiableState().m_hasComplexClip = true;
     modifiableState().m_hasClip = true;
 }
 
@@ -1297,6 +1300,7 @@ void CanvasRenderingContext2D::fillRect(float x, float y, float width, float hei
 
     FloatRect rect(x, y, width, height);
     if (rectContainsTransformedRect(rect, clipBounds)) {
+        checkOverdraw(rect, &c->fillPaint(), NoImage, ClipFill);
         c->fillRect(rect);
         didDraw(clipBounds);
     } else if (isFullCanvasCompositeMode(state().m_globalComposite)) {
@@ -1304,7 +1308,7 @@ void CanvasRenderingContext2D::fillRect(float x, float y, float width, float hei
         didDraw(clipBounds);
     } else if (state().m_globalComposite == SkXfermode::kSrc_Mode) {
         clearCanvas();
-        c->clearShadow();
+        c->clearShadow(); // Takes care of signaling the overdraw
         c->fillRect(rect);
         applyShadow(DrawShadowAndForeground);
         didDraw(clipBounds);
@@ -1518,13 +1522,14 @@ void CanvasRenderingContext2D::drawImageInternal(CanvasImageSource* imageSource,
         canvas()->buffer()->willAccessPixels();
 
     if (rectContainsTransformedRect(dstRect, clipBounds)) {
+        checkOverdraw(dstRect, &c->fillPaint(), imageSource->isOpaque() ? OpaqueImage : NonOpaqueImage, NormalFill);

[dshwang, 2015/02/06 19:10:37]

Why drawImage uses NormalFill while fillRect use ClipFill? what's different?
IMO this code is also already known to cover the current clip.

[Justin Novosad, 2015/02/06 19:37:31]

You are right. And this was the only place NormalFill was used so we can get rid
of it, which will simplify checkOverdraw.

         drawImageOnContext(drawingCanvas(), c, imageSource, image.get(), srcRect, dstRect);
         didDraw(clipBounds);
     } else if (isFullCanvasCompositeMode(state().m_globalComposite)) {
         fullCanvasCompositedDraw(bind(&drawImageOnContext, drawingCanvas(), c, imageSource, image.get(), srcRect, dstRect));
         didDraw(clipBounds);
     } else if (state().m_globalComposite == SkXfermode::kSrc_Mode) {
-        clearCanvas();
+        clearCanvas(); // takes care of signaling an overdraw
         drawImageOnContext(drawingCanvas(), c, imageSource, image.get(), srcRect, dstRect);
         didDraw(clipBounds);
     } else {
@@ -1551,6 +1556,7 @@ void CanvasRenderingContext2D::clearCanvas()
     if (!c)
         return;
 
+    checkOverdraw(canvasRect, 0, NoImage, ClipFill);
     c->clear(m_hasAlpha ? SK_ColorTRANSPARENT : SK_ColorBLACK);
 }
 
@@ -1791,6 +1797,8 @@ void CanvasRenderingContext2D::putImageData(ImageData* data, float dx, float dy,
     IntRect sourceRect(destRect);
     sourceRect.move(-destOffset);
 
+    checkOverdraw(destRect, 0, NoImage, UntransformedUnclippedFill);
+
     buffer->putByteArray(Unmultiplied, data->data()->data(), IntSize(data->width(), data->height()), sourceRect, IntPoint(destOffset));
 
     didDraw(destRect);
@@ -2382,4 +2390,92 @@ unsigned CanvasRenderingContext2D::hitRegionsCount() const
     return 0;
 }
 
+void CanvasRenderingContext2D::checkOverdraw(const SkRect& rect, const SkPaint* paint, ImageType imageType, DrawType drawType)
+{
+    SkCanvas* c = drawingCanvas();
+    if (!c || !canvas()->buffer()->isRecording())
+        return;
+
+    SkRect deviceRect;
+    if (drawType == UntransformedUnclippedFill) {
+        deviceRect = rect;
+    } else if (drawType == ClipFill) {
+        if (state().m_hasComplexClip)
+            return;
+
+        SkIRect skIBounds;
+        if (!c->getClipDeviceBounds(&skIBounds))
+            return;
+        deviceRect = SkRect::Make(skIBounds);
+    } else {
+        // FIXME: Early out if rotate/skew. To be thorough, we could compute
+        // an enclsed rect, but that would probably be overkill.

[dshwang, 2015/02/06 19:10:37]

s/enclsed/enclosed/

[Justin Novosad, 2015/02/06 19:37:31]

Acknowledged.

+        const SkMatrix& ctm = c->getTotalMatrix();
+        if (!ctm.rectStaysRect())
+            return;
+
+        if (state().m_hasComplexClip)
+            return;
+
+        ctm.mapRect(&deviceRect, rect);
+        SkIRect skIBounds;
+        if (!c->getClipDeviceBounds(&skIBounds))
+            return;
+        SkRect skBounds = SkRect::Make(skIBounds);
+        if (!deviceRect.intersect(skBounds))
+            return;
+    }
+
+    const SkImageInfo& imageInfo = c->imageInfo();
+    if (!deviceRect.contains(SkRect::MakeWH(imageInfo.width(), imageInfo.height())))
+        return;
+
+    bool isSourceOver = true;
+    unsigned alpha = 0xFF;
+    if (paint) {
+        if (paint->getLooper() || paint->getImageFilter() || paint->getMaskFilter())
+            return;
+
+        SkXfermode* xfermode = paint->getXfermode();
+        if (xfermode) {
+            SkXfermode::Mode mode;
+            if (xfermode->asMode(&mode)) {
+                isSourceOver = mode == SkXfermode::kSrcOver_Mode;
+                if (!isSourceOver && mode != SkXfermode::kSrc_Mode && mode != SkXfermode::kClear_Mode)
+                    return; // The code below only knows how to handle Src, SrcOver, and Clear
+            } else {
+                // unknown xfermode

[dshwang, 2015/02/06 19:10:37]

ASSERT_NOT_REACHED?

[Justin Novosad, 2015/02/06 19:37:31]

Acknowledged.

+                return;
+            }
+        }
+
+        if (isSourceOver) {
+            // With source over, we need to certify that alpha == 0xFF for all pixels
+            SkColorFilter* colorFilter = paint->getColorFilter();
+            if (colorFilter && !(colorFilter->getFlags() & SkColorFilter::kAlphaUnchanged_Flag))
+                return;
+
+            SkShader* shader = paint->getShader();
+            if (shader) {
+                // Shader overrides bitmap and paint color, so we can end here
+                if (shader->isOpaque())

[dshwang, 2015/02/06 19:10:37]

don't we need to consider alpha even if shader is opaque?

[Justin Novosad, 2015/02/06 19:37:31]

Did some digging in the code. In skia-land, the shader overrides the SkPaint's
color, except for the alpha, which modulates the shader's output.  This means
there was a bug in the old code too, that there is an error in the old unit
tests, and that we have no layout test coverage that exercises that specific
case.  Thanks for spotting this. I will take care of it and I will add adequate
testing.

+                    canvas()->buffer()->willOverwriteCanvas();
+                return;
+            }
+        }
+
+        alpha = paint->getAlpha();
+    }
+
+    if (isSourceOver) {
+        // With source over, we need to certify that alpha == 0xFF for all pixels
+        if (imageType == NonOpaqueImage)
+            return;
+        if (imageType == NoImage && alpha < 0xFF)
+            return;

[dshwang, 2015/02/06 19:10:37]

Can we overdraw when imageType == OpaqueImage && alpha < 0xFF ?

[Justin Novosad, 2015/02/06 19:37:31]

Yep, Made the same mistake here as with shaders above.

+    }
+
+    canvas()->buffer()->willOverwriteCanvas();
+}
+
 } // namespace blink