Clarify GraphicsContext::beginLayer()/endLayer() have unexpected behaviors.

Source/platform/graphics/GraphicsContext.cpp

 /*
  * Copyright (C) 2003, 2004, 2005, 2006, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 70 matching lines @@
     RegionTrackingMode m_regionTrackingMode;
 };
 
 GraphicsContext::GraphicsContext(SkCanvas* canvas, DisabledMode disableContextOrPainting)
     : m_canvas(canvas)
     , m_paintStateStack()
     , m_paintStateIndex(0)
     , m_annotationMode(0)
 #if ENABLE(ASSERT)
     , m_annotationCount(0)
-    , m_layerCount(0)
     , m_disableDestructionChecks(false)
 #endif
     , m_disabledState(disableContextOrPainting)
     , m_deviceScaleFactor(1.0f)
     , m_regionTrackingMode(RegionTrackingDisabled)
     , m_trackTextRegion(false)
     , m_accelerated(false)
     , m_isCertainlyOpaque(true)
     , m_printing(false)
     , m_antialiasHairlineImages(false)
     , m_shouldSmoothFonts(true)
 {
     // FIXME: Do some tests to determine how many states are typically used, and allocate
     // several here.
     m_paintStateStack.append(GraphicsContextState::create());
     m_paintState = m_paintStateStack.last().get();
 }
 
 GraphicsContext::~GraphicsContext()
 {
 #if ENABLE(ASSERT)
     if (!m_disableDestructionChecks) {
         ASSERT(!m_paintStateIndex);
         ASSERT(!m_paintState->saveCount());
         ASSERT(!m_annotationCount);
-        ASSERT(!m_layerCount);
+        ASSERT(m_clipStackPointersOfLayerStack.isEmpty());
         ASSERT(m_recordingStateStack.isEmpty());
         ASSERT(!saveCount());
     }
 #endif
 }
 
 void GraphicsContext::resetCanvas(SkCanvas* canvas)
 {
     m_canvas = canvas;
     m_trackedRegion.reset();
@@ skipping 47 matching lines @@
     // (on top of its own saveCount), except for the first frame.
     unsigned count = m_paintStateIndex;
     ASSERT(m_paintStateStack.size() > m_paintStateIndex);
     for (unsigned i = 0; i <= m_paintStateIndex; ++i)
         count += m_paintStateStack[i]->saveCount();
 
     return count;
 }
 #endif
 
+static unsigned getClipStackPointer(const SkCanvas* canvas)
+{
+    SkClipStack::B2TIter iter(*canvas->getClipStack());
+    const SkClipStack::Element* element = iter.next();
+    unsigned stackPointer = 0;
+    while (element) {
+        stackPointer++;
+        element = iter.next();
+    }
+    return stackPointer;
+}
+
 void GraphicsContext::saveLayer(const SkRect* bounds, const SkPaint* paint)
 {
     if (contextDisabled())
         return;
 
     ASSERT(m_canvas);
 
     m_canvas->saveLayer(bounds, paint);
     if (regionTrackingEnabled())
         m_trackedRegion.pushCanvasLayer(paint);
+    m_clipStackPointersOfLayerStack.append(getClipStackPointer(m_canvas));
 }
 
 void GraphicsContext::restoreLayer()
 {
     if (contextDisabled())
         return;
 
     ASSERT(m_canvas);
+    ASSERT(m_clipStackPointersOfLayerStack.size() > 0);
+
+    // beginLayer()/endLayer() must not expose states stacking mechanism.
+    // Currently SkCanvas::save()/restore() in GraphicsContext affected only ctm and clip.
+    SkMatrix currentCTM = m_canvas->getTotalMatrix();
+    SkClipStack currentSkClipStack(*m_canvas->getClipStack());

[Justin Novosad, 2014/10/16 17:51:58]

Seems unfortunate to duplicate the entire clip stack when all you care about is
elements from m_clipStackPointersOfLayerStack.last() to the end.

 
     m_canvas->restore();
     if (regionTrackingEnabled())
         m_trackedRegion.popCanvasLayer(this);
+
+    // SkCanvas::clipXXX() transforms a rect/path by CTM.
+    m_canvas->resetMatrix();
+
+    static const SkRect kEmptyRect = { 0, 0, 0, 0 };
+    SkClipStack::B2TIter iter(currentSkClipStack);
+    const SkClipStack::Element* element = nullptr;
+    unsigned stackPointer = m_clipStackPointersOfLayerStack.last();
+    m_clipStackPointersOfLayerStack.removeLast();
+    for (unsigned i = 0; i < stackPointer; i++) {
+        iter.next();
+    }
+    element = iter.next();
+    while (element) {
+        switch (element->getType()) {
+        case SkClipStack::Element::kPath_Type:
+            m_canvas->clipPath(element->getPath(), element->getOp(), element->isAA());
+            break;
+        case SkClipStack::Element::kRRect_Type:
+            m_canvas->clipRRect(element->getRRect(), element->getOp(), element->isAA());
+            break;
+        case SkClipStack::Element::kRect_Type:
+            m_canvas->clipRect(element->getRect(), element->getOp(), element->isAA());
+            break;
+        case SkClipStack::Element::kEmpty_Type:
+            m_canvas->clipRect(kEmptyRect, SkRegion::kIntersect_Op, false);
+            break;
+        }
+        element = iter.next();
+    }
+
+    m_canvas->setMatrix(currentCTM);
 }

[dshwang, 2014/10/16 15:58:20]

Feel like using implementation detail of skia, but I cannot find a better way.
I hope new unittests in GraphicsContextTest is to catch it if clip
implementation of SkCanvas is updated incompatible to this CL.

[Justin Novosad, 2014/10/16 17:51:58]

I agree. reverse engineering the state seems unfortunate. Have you considered
making a change to skia? Perhaps adding a flavor of saveLayer that does not save
the matrix and clip?

 
 void GraphicsContext::beginAnnotation(const AnnotationList& annotations)
 {
     if (contextDisabled())
         return;
 
     ASSERT(m_canvas);
 
     canvas()->beginCommentGroup("GraphicsContextAnnotation");
 
@@ skipping 237 matching lines @@
     layerPaint.setXfermodeMode(WebCoreCompositeToSkiaComposite(op, m_paintState->blendMode()));
     layerPaint.setColorFilter(WebCoreColorFilterToSkiaColorFilter(colorFilter).get());
     layerPaint.setImageFilter(imageFilter);
 
     if (bounds) {
         SkRect skBounds = WebCoreFloatRectToSKRect(*bounds);
         saveLayer(&skBounds, &layerPaint);
     } else {
         saveLayer(0, &layerPaint);
     }
-
-#if ENABLE(ASSERT)
-    ++m_layerCount;
-#endif
 }
 
 void GraphicsContext::endLayer()
 {
     if (contextDisabled())
         return;
 
     restoreLayer();
-
-    ASSERT(m_layerCount > 0);
-#if ENABLE(ASSERT)
-    --m_layerCount;
-#endif
 }
 
 void GraphicsContext::beginRecording(const FloatRect& bounds, uint32_t recordFlags)
 {
     RefPtr<DisplayList> displayList = DisplayList::create(bounds);
 
     SkCanvas* savedCanvas = m_canvas;
     SkMatrix savedMatrix = getTotalMatrix();
     SkPictureRecorder* recorder = 0;
 
@@ skipping 1498 matching lines @@
         // FIXME: This is to not break tests (it results in the filter bitmap flag
         // being set to true). We need to decide if we respect InterpolationNone
         // being returned from computeInterpolationQuality.
         resampling = InterpolationLow;
     }
     resampling = limitInterpolationQuality(this, resampling);
     paint->setFilterLevel(static_cast<SkPaint::FilterLevel>(resampling));
 }
 
 } // namespace blink