add local caching for skia draws

PDFium assumes the lowest common denominator
and draws many strings and paths that can
be accumulated.

Defer canvas->restore() calls until required
because the clip changed.

Defer text and path draws as long as subsequent
calls concatenate additional data.

Include debugging switch to allow disabling cache
at compile-time while bugs are shaken out.

Review-Url: https://codereview.chromium.org/2064753002

6 files changed, 633 insertions, 24 deletions

diff --git a/BUILD.gn b/BUILD.gn
index 10fe5df332..91348ce185 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -1490,6 +1490,10 @@ test("pdfium_unittests") {
       "xfa/fxfa/parser/xfa_utils_imp_unittest.cpp",
     ]
   }
+  if (pdf_use_skia) {
+    sources += [ "core/fxge/skia/fx_skia_device_unittest.cpp" ]
+    deps += [ "//skia" ]
+  }
   if (pdf_enable_v8) {
     sources += [ "fpdfsdk/javascript/public_methods_unittest.cpp" ]
     include_dirs += [
diff --git a/core/fxge/skia/DEPS b/core/fxge/skia/DEPS
index 6492756b7e..da24b1efec 100644
--- a/core/fxge/skia/DEPS
+++ b/core/fxge/skia/DEPS
@@ -1,3 +1,5 @@
 include_rules = [
-  '+third_party/skia/include'
+  '+fpdfsdk/include',
+  '+public',
+  '+third_party/skia/include',
 ]
diff --git a/core/fxge/skia/fx_skia_device.cpp b/core/fxge/skia/fx_skia_device.cpp
index 4b69d2fee6..c7f231a8ca 100644
--- a/core/fxge/skia/fx_skia_device.cpp
+++ b/core/fxge/skia/fx_skia_device.cpp
@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "core/fxge/include/fx_ge.h"
-
 #if defined(_SKIA_SUPPORT_)
 #include <algorithm>
 #include <vector>
@@ -29,6 +27,10 @@
 #include "third_party/skia/include/effects/SkGradientShader.h"
 #include "third_party/skia/include/pathops/SkPathOps.h"
 
+#ifdef SK_DEBUG
+#include "third_party/skia/include/core/SkClipStack.h"
+#endif
+
 namespace {
 
 #define SHOW_SKIA_PATH 0  // set to 1 to print the path contents
@@ -476,6 +478,401 @@ void ClipAngledGradient(const SkPoint pts[2],
 
 }  // namespace
 
+// Encapsulate the state used for successive text and path draws so that
+// they can be combined.
+class SkiaState {
+ public:
+  enum class Clip {
+    kSave,
+    kPath,
+  };
+
+  // mark all cached state as uninitialized
+  SkiaState()
+      : m_pFont(nullptr),
+        m_pCache(nullptr),
+        m_fontSize(0),
+        m_fillColor(0),
+        m_strokeColor(0),
+        m_blendType(0),
+        m_commandIndex(0),
+        m_drawText(false),
+        m_drawPath(false),
+        m_fillPath(false),
+        m_debugDisable(false) {}
+
+  bool DrawPath(const CFX_PathData* pPathData,
+                const CFX_Matrix* pMatrix,
+                const CFX_GraphStateData* pDrawState,
+                uint32_t fill_color,
+                uint32_t stroke_color,
+                int fill_mode,
+                int blend_type,
+                CFX_SkiaDeviceDriver* pDriver) {
+    if (m_debugDisable)
+      return false;
+    if (m_commandIndex < m_commands.count())
+      FlushCommands(pDriver);
+    if (m_drawText)
+      FlushText(pDriver);
+    if (m_drawPath && DrawChanged(pMatrix, pDrawState, fill_color, stroke_color,
+                                  fill_mode, blend_type)) {
+      FlushPath(pDriver);
+    }
+    if (!m_drawPath) {
+      m_skPath.reset();
+      m_fillPath = (fill_mode & 3) && fill_color;
+      m_skPath.setFillType((fill_mode & 3) == FXFILL_ALTERNATE
+                               ? SkPath::kEvenOdd_FillType
+                               : SkPath::kWinding_FillType);
+      m_drawState = *pDrawState;
+      m_fillColor = fill_color;
+      m_strokeColor = stroke_color;
+      m_blendType = blend_type;
+      m_drawMatrix = *pMatrix;
+    }
+    SkPath skPath = BuildPath(pPathData);
+    SkPoint delta;
+    if (MatrixOffset(pMatrix, &delta))
+      skPath.offset(delta.fX, delta.fY);
+    m_skPath.addPath(skPath);
+    m_drawPath = true;
+    return true;
+  }
+
+  void FlushPath(CFX_SkiaDeviceDriver* pDriver) {
+    SkMatrix skMatrix = ToSkMatrix(m_drawMatrix);
+    SkPaint skPaint;
+    skPaint.setAntiAlias(true);
+    int stroke_alpha = FXARGB_A(m_strokeColor);
+    if (stroke_alpha)
+      pDriver->PaintStroke(&skPaint, &m_drawState, skMatrix);
+    SkCanvas* skCanvas = pDriver->SkiaCanvas();
+    skCanvas->save();
+    skCanvas->concat(skMatrix);
+    if (m_fillPath) {
+      SkPath strokePath;
+      const SkPath* fillPath = &m_skPath;
+      if (stroke_alpha) {
+        SkAlpha fillA = SkColorGetA(m_fillColor);
+        SkAlpha strokeA = SkColorGetA(m_strokeColor);
+        if (fillA && fillA < 0xFF && strokeA && strokeA < 0xFF) {
+          skPaint.getFillPath(m_skPath, &strokePath);
+          if (Op(m_skPath, strokePath, SkPathOp::kDifference_SkPathOp,
+                 &strokePath)) {
+            fillPath = &strokePath;
+          }
+        }
+      }
+      skPaint.setStyle(SkPaint::kFill_Style);
+      skPaint.setColor(m_fillColor);
+      skCanvas->drawPath(*fillPath, skPaint);
+    }
+    if (stroke_alpha) {
+      DebugShowSkiaPath(m_skPath);
+      DebugShowCanvasMatrix(skCanvas);
+      skPaint.setStyle(SkPaint::kStroke_Style);
+      skPaint.setColor(m_strokeColor);
+      skCanvas->drawPath(m_skPath, skPaint);
+    }
+    skCanvas->restore();
+    m_drawPath = false;
+  }
+
+  bool DrawText(int nChars,
+                const FXTEXT_CHARPOS* pCharPos,
+                CFX_Font* pFont,
+                CFX_FontCache* pCache,
+                const CFX_Matrix* pMatrix,
+                FX_FLOAT font_size,
+                uint32_t color,
+                CFX_SkiaDeviceDriver* pDriver) {
+    if (m_debugDisable)
+      return false;
+    if (m_commandIndex < m_commands.count())
+      FlushCommands(pDriver);
+    if (m_drawPath)
+      FlushPath(pDriver);
+    if (m_drawText && FontChanged(pFont, pCache, pMatrix, font_size, color))
+      FlushText(pDriver);
+    if (!m_drawText) {
+      m_positions.setCount(0);
+      m_glyphs.setCount(0);
+      m_pFont = pFont;
+      m_pCache = pCache;
+      m_fontSize = font_size;
+      m_fillColor = color;
+      m_drawMatrix = *pMatrix;
+    }
+    int count = m_positions.count();
+    m_positions.setCount(nChars + count);
+    m_glyphs.setCount(nChars + count);
+    for (int index = 0; index < nChars; ++index) {
+      const FXTEXT_CHARPOS& cp = pCharPos[index];
+      m_positions[index + count] = {cp.m_OriginX, cp.m_OriginY};
+      m_glyphs[index + count] = (uint16_t)cp.m_GlyphIndex;
+    }
+    SkPoint delta;
+    if (MatrixOffset(pMatrix, &delta)) {
+      for (int index = 0; index < nChars; ++index)
+        m_positions[index + count].offset(delta.fX, -delta.fY);
+    }
+    m_drawText = true;
+    return true;
+  }
+
+  void FlushText(CFX_SkiaDeviceDriver* pDriver) {
+    SkMatrix skMatrix = ToFlippedSkMatrix(m_drawMatrix);
+    SkPaint skPaint;
+    skPaint.setAntiAlias(true);
+    skPaint.setColor(m_fillColor);
+    if (m_pFont->GetFace()) {  // exclude placeholder test fonts
+      sk_sp<SkTypeface> typeface(SkSafeRef(m_pCache->GetDeviceCache(m_pFont)));
+      skPaint.setTypeface(typeface);
+    }
+    skPaint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
+    skPaint.setTextSize(m_fontSize);
+    skPaint.setSubpixelText(true);
+    SkCanvas* skCanvas = pDriver->SkiaCanvas();
+    skCanvas->save();
+    skCanvas->concat(skMatrix);
+    skCanvas->drawPosText(m_glyphs.begin(), m_glyphs.count() * 2,
+                          m_positions.begin(), skPaint);
+    skCanvas->restore();
+    m_drawText = false;
+  }
+
+  bool SetClipFill(const CFX_PathData* pPathData,
+                   const CFX_Matrix* pMatrix,
+                   int fill_mode,
+                   CFX_SkiaDeviceDriver* pDriver) {
+    if (m_debugDisable)
+      return false;
+    SkPath skClipPath = BuildPath(pPathData);
+    skClipPath.setFillType((fill_mode & 3) == FXFILL_ALTERNATE
+                               ? SkPath::kEvenOdd_FillType
+                               : SkPath::kWinding_FillType);
+    SkMatrix skMatrix = ToSkMatrix(*pMatrix);
+    skClipPath.transform(skMatrix);
+    return SetClip(skClipPath, pDriver);
+  }
+
+  bool SetClip(const SkPath& skClipPath, CFX_SkiaDeviceDriver* pDriver) {
+    if (m_commandIndex < m_commands.count()) {
+      if (m_commands[m_commandIndex] == Clip::kPath &&
+          m_clips[m_commandIndex] == skClipPath) {
+        ++m_commandIndex;
+        return true;
+      }
+      FlushCommands(pDriver);
+    }
+    Flush(pDriver);
+    m_commands.push(Clip::kPath);
+    ++m_commandIndex;
+    m_clips.push_back(skClipPath);
+    return false;
+  }
+
+  bool SetClipStroke(const CFX_PathData* pPathData,
+                     const CFX_Matrix* pMatrix,
+                     const CFX_GraphStateData* pGraphState,
+                     CFX_SkiaDeviceDriver* pDriver) {
+    if (m_debugDisable)
+      return false;
+    SkPath skPath = BuildPath(pPathData);
+    SkMatrix skMatrix = ToSkMatrix(*pMatrix);
+    SkPaint skPaint;
+    pDriver->PaintStroke(&skPaint, pGraphState, skMatrix);
+    SkPath dst_path;
+    skPaint.getFillPath(skPath, &dst_path);
+    dst_path.transform(skMatrix);
+    return SetClip(dst_path, pDriver);
+  }
+
+  bool MatrixOffset(const CFX_Matrix* pMatrix, SkPoint* delta) {
+    delta->set(pMatrix->e - m_drawMatrix.e, pMatrix->f - m_drawMatrix.f);
+    if (!delta->fX && !delta->fY)
+      return true;
+    SkMatrix drawMatrix = ToSkMatrix(m_drawMatrix);
+    if (!(drawMatrix.getType() & ~SkMatrix::kTranslate_Mask))
+      return true;
+    SkMatrix invDrawMatrix;
+    if (!drawMatrix.invert(&invDrawMatrix))
+      return false;
+    SkMatrix invNewMatrix;
+    SkMatrix newMatrix = ToSkMatrix(*pMatrix);
+    if (!newMatrix.invert(&invNewMatrix))
+      return false;
+    delta->set(invDrawMatrix.getTranslateX() - invNewMatrix.getTranslateX(),
+               invDrawMatrix.getTranslateY() - invNewMatrix.getTranslateY());
+    return true;
+  }
+
+  void FlushCommands(CFX_SkiaDeviceDriver* pDriver) {
+    if (m_commandIndex == m_commands.count())
+      return;
+    if (m_commandIndex < m_commands.count())
+      pDriver->SkiaCanvas()->restore();
+    int index = m_commands.count() - 1;
+    if (m_commandIndex == index && m_commands[index] == Clip::kSave)
+      return;
+    for (; index > m_commandIndex; --index) {
+      if (m_commands[index] == Clip::kSave)
+        pDriver->SkiaCanvas()->restore();
+    }
+
+    if (m_commandIndex > 0)
+      pDriver->SkiaCanvas()->save();
+    while (index > 0 && m_commands[index] != Clip::kSave)
+      --index;
+    while (++index < m_commandIndex) {
+      SkASSERT(m_commands[index] == Clip::kPath);
+      pDriver->SkiaCanvas()->clipPath(m_clips[index], SkRegion::kIntersect_Op,
+                                      true);
+    }
+    m_commands.setCount(m_commandIndex);
+    m_clips.resize_back(m_commandIndex);
+  }
+
+  // returns true if caller should apply command to skia canvas
+  bool ClipSave(CFX_SkiaDeviceDriver* pDriver) {
+    if (m_debugDisable)
+      return false;
+    int count = m_commands.count();
+    if (m_commandIndex < count) {
+      if (m_commands[m_commandIndex] == Clip::kSave) {
+        ++m_commandIndex;
+        return true;
+      }
+      FlushCommands(pDriver);
+    }
+    Flush(pDriver);
+    m_commands.push(Clip::kSave);
+    ++m_commandIndex;
+    m_clips.push_back(m_skEmptyPath);
+    return false;
+  }
+
+  bool ClipRestore(CFX_SkiaDeviceDriver* pDriver) {
+    if (m_debugDisable)
+      return false;
+    while (m_commandIndex > 0) {
+      if (m_commands[--m_commandIndex] == Clip::kSave)
+        return true;
+    }
+    Flush(pDriver);
+    return false;
+  }
+
+  bool DrawChanged(const CFX_Matrix* pMatrix,
+                   const CFX_GraphStateData* pState,
+                   uint32_t fill_color,
+                   uint32_t stroke_color,
+                   int fill_mode,
+                   int blend_type) {
+    return MatrixChanged(pMatrix, m_drawMatrix) ||
+           StateChanged(pState, m_drawState) || fill_color != m_fillColor ||
+           stroke_color != m_strokeColor ||
+           ((fill_mode & 3) == FXFILL_ALTERNATE) !=
+               (m_skPath.getFillType() == SkPath::kEvenOdd_FillType) ||
+           blend_type != m_blendType;
+  }
+
+  bool FontChanged(CFX_Font* pFont,
+                   CFX_FontCache* pCache,
+                   const CFX_Matrix* pMatrix,
+                   FX_FLOAT font_size,
+                   uint32_t color) {
+    return pFont != m_pFont || pCache != m_pCache ||
+           MatrixChanged(pMatrix, m_drawMatrix) || font_size != m_fontSize ||
+           color != m_fillColor;
+  }
+
+  bool MatrixChanged(const CFX_Matrix* pMatrix, const CFX_Matrix& refMatrix) {
+    return pMatrix->a != refMatrix.a || pMatrix->b != refMatrix.b ||
+           pMatrix->c != refMatrix.c || pMatrix->d != refMatrix.d;
+  }
+
+  bool StateChanged(const CFX_GraphStateData* pState,
+                    const CFX_GraphStateData& refState) {
+    return pState->m_LineWidth != refState.m_LineWidth ||
+           pState->m_LineCap != refState.m_LineCap ||
+           pState->m_LineJoin != refState.m_LineJoin ||
+           pState->m_MiterLimit != refState.m_MiterLimit ||
+           DashChanged(pState, refState);
+  }
+
+  bool DashChanged(const CFX_GraphStateData* pState,
+                   const CFX_GraphStateData& refState) {
+    if (!pState->m_DashArray && !refState.m_DashArray)
+      return false;
+    if (!pState->m_DashArray || !refState.m_DashArray)
+      return true;
+    if (pState->m_DashPhase != refState.m_DashPhase ||
+        pState->m_DashCount != refState.m_DashCount) {
+      return true;
+    }
+    for (int index = 0; index < pState->m_DashCount; ++index) {
+      if (pState->m_DashArray[index] != refState.m_DashArray[index])
+        return false;
+    }
+    return true;
+  }
+
+  void Flush(CFX_SkiaDeviceDriver* pDriver) {
+    if (m_drawPath)
+      FlushPath(pDriver);
+    if (m_drawText)
+      FlushText(pDriver);
+  }
+
+#ifdef SK_DEBUG
+  void Dump(const CFX_SkiaDeviceDriver* pDriver) const {
+    SkDebugf("\n\nSkia Save Count %d:\n", pDriver->m_pCanvas->getSaveCount());
+    pDriver->m_pCanvas->getClipStack()->dump();
+    SkDebugf("Cache:\n");
+    for (int index = 0; index < m_commands.count(); ++index) {
+      SkDebugf("%s ", m_commandIndex == index ? "-->" : "   ");
+      switch (m_commands[index]) {
+        case Clip::kSave:
+          SkDebugf("Save\n");
+          break;
+        case Clip::kPath:
+          m_clips[index].dump();
+          break;
+        default:
+          SkDebugf("unknown\n");
+      }
+    }
+    if (m_commandIndex == m_commands.count())
+      SkDebugf("-->\n");
+  }
+#endif
+
+ private:
+  SkTArray<SkPath> m_clips;        // stack of clips that may be reused
+  SkTDArray<Clip> m_commands;      // stack of clip-related commands
+  SkTDArray<SkPoint> m_positions;  // accumulator for text positions
+  SkTDArray<uint16_t> m_glyphs;    // accumulator for text glyphs
+  SkPath m_skPath;                 // accumulator for path contours
+  SkPath m_skEmptyPath;            // used as placehold in the clips array
+  CFX_Matrix m_drawMatrix;
+  CFX_GraphStateData m_clipState;
+  CFX_GraphStateData m_drawState;
+  CFX_Matrix m_clipMatrix;
+  CFX_Font* m_pFont;
+  CFX_FontCache* m_pCache;
+  FX_FLOAT m_fontSize;
+  uint32_t m_fillColor;
+  uint32_t m_strokeColor;
+  int m_blendType;
+  int m_commandIndex;  // active position in clip command stack
+  bool m_drawText;
+  bool m_drawPath;
+  bool m_fillPath;
+  bool m_debugDisable;  // turn off cache for debugging
+};
+
 // convert a stroking path to scanlines
 void CFX_SkiaDeviceDriver::PaintStroke(SkPaint* spaint,
                                        const CFX_GraphStateData* pGraphState,
@@ -548,6 +945,7 @@ CFX_SkiaDeviceDriver::CFX_SkiaDeviceDriver(CFX_DIBitmap* pBitmap,
     : m_pBitmap(pBitmap),
       m_pOriDevice(pOriDevice),
       m_pRecorder(nullptr),
+      m_pCache(new SkiaState),
       m_bRgbByteOrder(bRgbByteOrder),
       m_bGroupKnockout(bGroupKnockout) {
   SkBitmap skBitmap;
@@ -568,6 +966,7 @@ CFX_SkiaDeviceDriver::CFX_SkiaDeviceDriver(int size_x, int size_y)
     : m_pBitmap(nullptr),
       m_pOriDevice(nullptr),
       m_pRecorder(new SkPictureRecorder),
+      m_pCache(new SkiaState),
       m_bRgbByteOrder(FALSE),
       m_bGroupKnockout(FALSE) {
   m_pRecorder->beginRecording(SkIntToScalar(size_x), SkIntToScalar(size_y));
@@ -578,16 +977,23 @@ CFX_SkiaDeviceDriver::CFX_SkiaDeviceDriver(SkPictureRecorder* recorder)
     : m_pBitmap(nullptr),
       m_pOriDevice(nullptr),
       m_pRecorder(recorder),
+      m_pCache(new SkiaState),
       m_bRgbByteOrder(FALSE),
       m_bGroupKnockout(FALSE) {
   m_pCanvas = m_pRecorder->getRecordingCanvas();
 }
 
 CFX_SkiaDeviceDriver::~CFX_SkiaDeviceDriver() {
+  Flush();
   if (!m_pRecorder)
     delete m_pCanvas;
 }
 
+void CFX_SkiaDeviceDriver::Flush() {
+  m_pCache->Flush(this);
+  m_pCache->FlushCommands(this);
+}
+
 FX_BOOL CFX_SkiaDeviceDriver::DrawDeviceText(int nChars,
                                              const FXTEXT_CHARPOS* pCharPos,
                                              CFX_Font* pFont,
@@ -595,6 +1001,10 @@ FX_BOOL CFX_SkiaDeviceDriver::DrawDeviceText(int nChars,
                                              const CFX_Matrix* pObject2Device,
                                              FX_FLOAT font_size,
                                              uint32_t color) {
+  if (m_pCache->DrawText(nChars, pCharPos, pFont, pCache, pObject2Device,
+                         font_size, color, this)) {
+    return TRUE;
+  }
   sk_sp<SkTypeface> typeface(SkSafeRef(pCache->GetDeviceCache(pFont)));
   SkPaint paint;
   paint.setAntiAlias(true);
@@ -642,13 +1052,15 @@ int CFX_SkiaDeviceDriver::GetDeviceCaps(int caps_id) const {
 }
 
 void CFX_SkiaDeviceDriver::SaveState() {
-  m_pCanvas->save();
+  if (!m_pCache->ClipSave(this))
+    m_pCanvas->save();
 }
 
 void CFX_SkiaDeviceDriver::RestoreState(bool bKeepSaved) {
-  m_pCanvas->restore();
+  if (!m_pCache->ClipRestore(this))
+    m_pCanvas->restore();
   if (bKeepSaved)
-    m_pCanvas->save();
+    SaveState();
 }
 
 FX_BOOL CFX_SkiaDeviceDriver::SetClip_PathFill(
@@ -656,9 +1068,13 @@ FX_BOOL CFX_SkiaDeviceDriver::SetClip_PathFill(
     const CFX_Matrix* pObject2Device,  // flips object's y-axis
     int fill_mode                      // fill mode, WINDING or ALTERNATE
     ) {
+  CFX_Matrix identity;
+  const CFX_Matrix* deviceMatrix = pObject2Device ? pObject2Device : &identity;
+  if (m_pCache->SetClipFill(pPathData, deviceMatrix, fill_mode, this))
+    return TRUE;
   if (pPathData->GetPointCount() == 5 || pPathData->GetPointCount() == 4) {
     CFX_FloatRect rectf;
-    if (pPathData->IsRect(pObject2Device, &rectf)) {
+    if (pPathData->IsRect(deviceMatrix, &rectf)) {
       rectf.Intersect(
           CFX_FloatRect(0, 0, (FX_FLOAT)GetDeviceCaps(FXDC_PIXEL_WIDTH),
                         (FX_FLOAT)GetDeviceCaps(FXDC_PIXEL_HEIGHT)));
@@ -666,19 +1082,19 @@ FX_BOOL CFX_SkiaDeviceDriver::SetClip_PathFill(
       SkRect skClipRect =
           SkRect::MakeLTRB(rectf.left, rectf.bottom, rectf.right, rectf.top);
       DebugDrawSkiaClipRect(m_pCanvas, skClipRect);
-      m_pCanvas->clipRect(skClipRect);
+      m_pCanvas->clipRect(skClipRect, SkRegion::kIntersect_Op, true);
       return TRUE;
     }
   }
   SkPath skClipPath = BuildPath(pPathData);
-  skClipPath.setFillType((fill_mode & 3) == FXFILL_WINDING
-                             ? SkPath::kWinding_FillType
-                             : SkPath::kEvenOdd_FillType);
-  SkMatrix skMatrix = ToSkMatrix(*pObject2Device);
+  skClipPath.setFillType((fill_mode & 3) == FXFILL_ALTERNATE
+                             ? SkPath::kEvenOdd_FillType
+                             : SkPath::kWinding_FillType);
+  SkMatrix skMatrix = ToSkMatrix(*deviceMatrix);
   skClipPath.transform(skMatrix);
   DebugShowSkiaPath(skClipPath);
   DebugDrawSkiaClipPath(m_pCanvas, skClipPath);
-  m_pCanvas->clipPath(skClipPath);
+  m_pCanvas->clipPath(skClipPath, SkRegion::kIntersect_Op, true);
 
   return TRUE;
 }
@@ -688,18 +1104,18 @@ FX_BOOL CFX_SkiaDeviceDriver::SetClip_PathStroke(
     const CFX_Matrix* pObject2Device,      // optional transformation
     const CFX_GraphStateData* pGraphState  // graphic state, for pen attributes
     ) {
+  if (m_pCache->SetClipStroke(pPathData, pObject2Device, pGraphState, this))
+    return TRUE;
   // build path data
   SkPath skPath = BuildPath(pPathData);
-  skPath.setFillType(SkPath::kWinding_FillType);
-
   SkMatrix skMatrix = ToSkMatrix(*pObject2Device);
-  SkPaint spaint;
-  PaintStroke(&spaint, pGraphState, skMatrix);
+  SkPaint skPaint;
+  PaintStroke(&skPaint, pGraphState, skMatrix);
   SkPath dst_path;
-  spaint.getFillPath(skPath, &dst_path);
+  skPaint.getFillPath(skPath, &dst_path);
   dst_path.transform(skMatrix);
   DebugDrawSkiaClipPath(m_pCanvas, dst_path);
-  m_pCanvas->clipPath(dst_path);
+  m_pCanvas->clipPath(dst_path, SkRegion::kIntersect_Op, true);
   return TRUE;
 }
 
@@ -711,6 +1127,10 @@ FX_BOOL CFX_SkiaDeviceDriver::DrawPath(
     uint32_t stroke_color,                  // stroke color
     int fill_mode,  // fill mode, WINDING or ALTERNATE. 0 for not filled
     int blend_type) {
+  if (m_pCache->DrawPath(pPathData, pObject2Device, pGraphState, fill_color,
+                         stroke_color, fill_mode, blend_type, this)) {
+    return TRUE;
+  }
   SkIRect rect;
   rect.set(0, 0, GetDeviceCaps(FXDC_PIXEL_WIDTH),
            GetDeviceCaps(FXDC_PIXEL_HEIGHT));
@@ -728,9 +1148,9 @@ FX_BOOL CFX_SkiaDeviceDriver::DrawPath(
   m_pCanvas->save();
   m_pCanvas->concat(skMatrix);
   if ((fill_mode & 3) && fill_color) {
-    skPath.setFillType((fill_mode & 3) == FXFILL_WINDING
-                           ? SkPath::kWinding_FillType
-                           : SkPath::kEvenOdd_FillType);
+    skPath.setFillType((fill_mode & 3) == FXFILL_ALTERNATE
+                           ? SkPath::kEvenOdd_FillType
+                           : SkPath::kWinding_FillType);
     SkPath strokePath;
     const SkPath* fillPath = &skPath;
     if (pGraphState && stroke_alpha) {
@@ -909,7 +1329,7 @@ FX_BOOL CFX_SkiaDeviceDriver::DrawShading(const CPDF_ShadingPattern* pPattern,
   }
   m_pCanvas->save();
   if (!skClip.isEmpty())
-    m_pCanvas->clipPath(skClip);
+    m_pCanvas->clipPath(skClip, SkRegion::kIntersect_Op, true);
   m_pCanvas->concat(skMatrix);
   m_pCanvas->drawPath(skPath, paint);
   m_pCanvas->restore();
@@ -1000,7 +1420,7 @@ FX_BOOL CFX_SkiaDeviceDriver::StretchDIBits(const CFX_DIBSource* pSource,
   m_pCanvas->save();
   SkRect skClipRect = SkRect::MakeLTRB(pClipRect->left, pClipRect->bottom,
                                        pClipRect->right, pClipRect->top);
-  m_pCanvas->clipRect(skClipRect);
+  m_pCanvas->clipRect(skClipRect, SkRegion::kIntersect_Op, true);
   void* dummy;
   FX_BOOL result = StartDIBits(pSource, 0xFF, argb, &m, 0, dummy, blend_type);
   m_pCanvas->restore();
@@ -1125,6 +1545,13 @@ void CFX_SkiaDeviceDriver::PreMultiply() {
   DebugVerifyBitmapIsPreMultiplied(buffer, width, height);
 }
 
+void CFX_SkiaDeviceDriver::Dump() const {
+#ifdef SK_DEBUG
+  if (m_pCache)
+    m_pCache->Dump(this);
+#endif
+}
+
 CFX_FxgeDevice::CFX_FxgeDevice() {
   m_bOwnedBitmap = FALSE;
 }
diff --git a/core/fxge/skia/fx_skia_device.h b/core/fxge/skia/fx_skia_device.h
index 4ea1ad1218..c21119b9a4 100644
--- a/core/fxge/skia/fx_skia_device.h
+++ b/core/fxge/skia/fx_skia_device.h
@@ -14,6 +14,7 @@ class SkMatrix;
 class SkPaint;
 class SkPath;
 class SkPictureRecorder;
+class SkiaState;
 struct SkIRect;
 
 class CFX_SkiaDeviceDriver : public IFX_RenderDeviceDriver {
@@ -122,14 +123,20 @@ class CFX_SkiaDeviceDriver : public IFX_RenderDeviceDriver {
   void PaintStroke(SkPaint* spaint,
                    const CFX_GraphStateData* pGraphState,
                    const SkMatrix& matrix);
+  void Flush();
   SkPictureRecorder* GetRecorder() const { return m_pRecorder; }
   void PreMultiply();
+  SkCanvas* SkiaCanvas() { return m_pCanvas; }
+  void Dump() const;
 
  private:
+  friend class SkiaState;
+
   CFX_DIBitmap* m_pBitmap;
   CFX_DIBitmap* m_pOriDevice;
   SkCanvas* m_pCanvas;
   SkPictureRecorder* const m_pRecorder;
+  std::unique_ptr<SkiaState> m_pCache;
   FX_BOOL m_bRgbByteOrder;
   FX_BOOL m_bGroupKnockout;
 };
diff --git a/core/fxge/skia/fx_skia_device_unittest.cpp b/core/fxge/skia/fx_skia_device_unittest.cpp
new file mode 100644
index 0000000000..eb7559b5e2
--- /dev/null
+++ b/core/fxge/skia/fx_skia_device_unittest.cpp
@@ -0,0 +1,164 @@
+// Copyright 2016 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/fxge/include/fx_ge.h"
+#include "core/fxge/skia/fx_skia_device.h"
+#include "fpdfsdk/include/fsdk_define.h"
+#include "public/fpdfview.h"
+#include "testing/fx_string_testhelpers.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SkPictureRecorder.h"
+
+namespace {
+
+struct State {
+  enum class Change { kNo, kYes };
+  enum class Save { kNo, kYes };
+  enum class Clip { kNo, kSame, kDifferentPath, kDifferentMatrix };
+  enum class Graphic { kNone, kPath, kText };
+
+  Change m_change;
+  Save m_save;
+  Clip m_clip;
+  Graphic m_graphic;
+  uint32_t m_pixel;
+};
+
+void EmptyTest(CFX_SkiaDeviceDriver* driver, const State&) {
+  driver->SaveState();
+  driver->RestoreState(true);
+  driver->RestoreState(false);
+}
+
+void CommonTest(CFX_SkiaDeviceDriver* driver, const State& state) {
+  FXTEXT_CHARPOS charPos[] = {1, 0, 1, 4, false, {0, 0, 0, 0}, false};
+  CFX_Font font;
+  FX_FLOAT fontSize = 1;
+  CFX_FontCache cache;
+  CFX_PathData clipPath, clipPath2;
+  clipPath.AppendRect(0, 0, 3, 1);
+  clipPath2.AppendRect(0, 0, 2, 1);
+  CFX_Matrix clipMatrix;
+  CFX_Matrix clipMatrix2(1, 0, 0, 1, 0, 1);
+  driver->SaveState();
+  CFX_PathData path1;
+  path1.AppendRect(0, 0, 1, 2);
+  CFX_Matrix matrix, matrix2;
+  matrix2.Translate(1, 0);
+  CFX_GraphStateData graphState;
+  if (state.m_save == State::Save::kYes)
+    driver->SaveState();
+  if (state.m_clip != State::Clip::kNo)
+    driver->SetClip_PathFill(&clipPath, &clipMatrix, 0);
+  if (state.m_graphic == State::Graphic::kPath) {
+    driver->DrawPath(&path1, &matrix, &graphState, 0xFF112233, 0,
+                     FXFILL_WINDING, 0);
+  } else if (state.m_graphic == State::Graphic::kText) {
+    driver->DrawDeviceText(SK_ARRAY_COUNT(charPos), charPos, &font, &cache,
+                           &matrix, fontSize, 0xFF445566);
+  }
+  if (state.m_save == State::Save::kYes)
+    driver->RestoreState(true);
+  CFX_PathData path2;
+  path2.AppendRect(0, 0, 2, 2);
+  if (state.m_change == State::Change::kYes) {
+    if (state.m_graphic == State::Graphic::kPath)
+      graphState.m_LineCap = CFX_GraphStateData::LineCapRound;
+    else if (state.m_graphic == State::Graphic::kText)
+      fontSize = 2;
+  }
+  if (state.m_clip == State::Clip::kSame)
+    driver->SetClip_PathFill(&clipPath, &clipMatrix, 0);
+  else if (state.m_clip == State::Clip::kDifferentPath)
+    driver->SetClip_PathFill(&clipPath2, &clipMatrix, 0);
+  else if (state.m_clip == State::Clip::kDifferentMatrix)
+    driver->SetClip_PathFill(&clipPath, &clipMatrix2, 0);
+  if (state.m_graphic == State::Graphic::kPath) {
+    driver->DrawPath(&path2, &matrix2, &graphState, 0xFF112233, 0,
+                     FXFILL_WINDING, 0);
+  } else if (state.m_graphic == State::Graphic::kText) {
+    driver->DrawDeviceText(SK_ARRAY_COUNT(charPos), charPos, &font, &cache,
+                           &matrix2, fontSize, 0xFF445566);
+  }
+  if (state.m_save == State::Save::kYes)
+    driver->RestoreState(false);
+  driver->RestoreState(false);
+}
+
+void OutOfSequenceClipTest(CFX_SkiaDeviceDriver* driver, const State&) {
+  CFX_PathData clipPath;
+  clipPath.AppendRect(1, 0, 3, 1);
+  CFX_Matrix clipMatrix;
+  driver->SaveState();
+  driver->SetClip_PathFill(&clipPath, &clipMatrix, 0);
+  driver->RestoreState(true);
+  driver->SaveState();
+  driver->SetClip_PathFill(&clipPath, &clipMatrix, 0);
+  driver->RestoreState(false);
+  driver->RestoreState(false);
+
+  driver->SaveState();
+  driver->SaveState();
+  driver->SetClip_PathFill(&clipPath, &clipMatrix, 0);
+  driver->RestoreState(true);
+  driver->SetClip_PathFill(&clipPath, &clipMatrix, 0);
+  driver->RestoreState(false);
+  driver->RestoreState(false);
+}
+
+void Harness(void (*Test)(CFX_SkiaDeviceDriver*, const State&),
+             const State& state) {
+  int h = 1;
+  int w = 4;
+  FPDF_BITMAP bitmap = FPDFBitmap_Create(w, h, 1);
+  EXPECT_NE(nullptr, bitmap);
+  if (!bitmap)
+    return;
+  FPDFBitmap_FillRect(bitmap, 0, 0, w, h, 0x00000000);
+  CFX_FxgeDevice geDevice;
+  CFX_DIBitmap* pBitmap = CFXBitmapFromFPDFBitmap(bitmap);
+  geDevice.Attach(pBitmap, false, nullptr, false);
+  CFX_SkiaDeviceDriver* driver =
+      static_cast<CFX_SkiaDeviceDriver*>(geDevice.GetDeviceDriver());
+  (*Test)(driver, state);
+  driver->Flush();
+  uint32_t pixel = pBitmap->GetPixel(0, 0);
+  EXPECT_EQ(state.m_pixel, pixel);
+#ifdef SK_DEBUG
+  if (!driver)  // force dump to be linked in so it can be called from debugger
+    driver->Dump();
+#endif
+}
+
+}  // namespace
+
+TEST(fxge, SkiaStateEmpty) {
+  Harness(&EmptyTest, {});
+}
+
+TEST(fxge, SkiaStatePath) {
+  Harness(&CommonTest, {State::Change::kNo, State::Save::kYes,
+                        State::Clip::kSame, State::Graphic::kPath, 0xFF112233});
+  Harness(&CommonTest,
+          {State::Change::kNo, State::Save::kYes, State::Clip::kDifferentPath,
+           State::Graphic::kPath, 0xFF112233});
+  Harness(&CommonTest, {State::Change::kNo, State::Save::kYes, State::Clip::kNo,
+                        State::Graphic::kPath, 0xFF112233});
+  Harness(&CommonTest, {State::Change::kYes, State::Save::kNo, State::Clip::kNo,
+                        State::Graphic::kPath, 0xFF112233});
+  Harness(&CommonTest, {State::Change::kNo, State::Save::kNo, State::Clip::kNo,
+                        State::Graphic::kPath, 0xFF112233});
+}
+
+TEST(fxge, SkiaStateText) {
+  Harness(&CommonTest,
+          {State::Change::kNo, State::Save::kYes, State::Clip::kDifferentMatrix,
+           State::Graphic::kText, 0xFF445566});
+  Harness(&CommonTest, {State::Change::kNo, State::Save::kYes,
+                        State::Clip::kSame, State::Graphic::kText, 0xFF445566});
+}
+
+TEST(fxge, SkiaStateOOSClip) {
+  Harness(&OutOfSequenceClipTest, {});
+}
diff --git a/pdfium.gyp b/pdfium.gyp
index a84c83df13..ca9f3f8aad 100644
--- a/pdfium.gyp
+++ b/pdfium.gyp
@@ -944,6 +944,11 @@
             'xfa/fxfa/parser/xfa_utils_imp_unittest.cpp',
           ],
         }],
+        ['pdf_use_skia==1', {
+          'sources': [
+            'core/fxge/skia/fx_skia_device_unittest.cpp',
+          ],
+        }],
         ['pdf_enable_v8==1', {
           'include_dirs': [
             '<(DEPTH)/v8',