copy graphics state fully

The dash parameters where not copied; the copy
could point at random data and cause corpus
tests to hang when testing Skia.

PDFium measures text directly by calling FreeType.
Turn off hinting altogether in Skia so that drawn
text matches the metrics that PDFium measures.

Premultiply bits retrieved from images, and check
to see that the device bits are always
premultiplied.

Look for null graphics state and matrices.

R=thestig@chromium.org,dsinclair@chromium.org
BUG=pdfium:532

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

5 files changed, 111 insertions, 162 deletions

diff --git a/core/fxge/ge/fx_ge_device.cpp b/core/fxge/ge/fx_ge_device.cpp
index 00cc541c22..9bb950beed 100644
--- a/core/fxge/ge/fx_ge_device.cpp
+++ b/core/fxge/ge/fx_ge_device.cpp
@@ -7,6 +7,10 @@
 #include "core/fxge/include/fx_ge.h"
 #include "core/fxge/include/ifx_renderdevicedriver.h"
 
+#if defined _SKIA_SUPPORT_
+#include "third_party/skia/include/core/SkTypes.h"
+#endif
+
 CFX_RenderDevice::CFX_RenderDevice()
     : m_pBitmap(nullptr),
       m_Width(0),
@@ -20,6 +24,11 @@ CFX_RenderDevice::~CFX_RenderDevice() {
   delete m_pDeviceDriver;
 }
 
+void CFX_RenderDevice::Flush() {
+  delete m_pDeviceDriver;
+  m_pDeviceDriver = nullptr;
+}
+
 void CFX_RenderDevice::SetDeviceDriver(IFX_RenderDeviceDriver* pDriver) {
   delete m_pDeviceDriver;
   m_pDeviceDriver = pDriver;
@@ -480,3 +489,10 @@ FX_BOOL CFX_RenderDevice::ContinueDIBits(void* handle, IFX_Pause* pPause) {
 void CFX_RenderDevice::CancelDIBits(void* handle) {
   m_pDeviceDriver->CancelDIBits(handle);
 }
+
+#ifdef _SKIA_SUPPORT_
+
+void CFX_RenderDevice::DebugVerifyBitmapIsPreMultiplied() const {
+  SkASSERT(0);
+}
+#endif
diff --git a/core/fxge/include/fx_dib.h b/core/fxge/include/fx_dib.h
index 5a31722a82..8d9f27e3f7 100644
--- a/core/fxge/include/fx_dib.h
+++ b/core/fxge/include/fx_dib.h
@@ -247,6 +247,10 @@ class CFX_DIBSource {
                       int& src_top,
                       const CFX_ClipRgn* pClipRgn);
 
+#if defined _SKIA_SUPPORT_
+  void DebugVerifyBitmapIsPreMultiplied(void* buffer = nullptr) const;
+#endif
+
   CFX_DIBitmap* m_pAlphaMask;
 
  protected:
diff --git a/core/fxge/include/fx_ge.h b/core/fxge/include/fx_ge.h
index 5599a75f59..ea2adec1dc 100644
--- a/core/fxge/include/fx_ge.h
+++ b/core/fxge/include/fx_ge.h
@@ -215,6 +215,7 @@ class CFX_RenderDevice {
   CFX_RenderDevice();
   virtual ~CFX_RenderDevice();
 
+  void Flush();
   void SetDeviceDriver(IFX_RenderDeviceDriver* pDriver);
   IFX_RenderDeviceDriver* GetDeviceDriver() const { return m_pDeviceDriver; }
 
@@ -378,6 +379,10 @@ class CFX_RenderDevice {
                                 CFX_PathData* pClippingPath,
                                 int nFlag);
 
+#ifdef _SKIA_SUPPORT_
+  virtual void DebugVerifyBitmapIsPreMultiplied() const;
+#endif
+
  private:
   void InitDeviceInfo();
   void UpdateClipBox();
@@ -416,7 +421,7 @@ class CFX_FxgeDevice : public CFX_RenderDevice {
 #ifdef _SKIA_SUPPORT_
   bool AttachRecorder(SkPictureRecorder* recorder);
   SkPictureRecorder* CreateRecorder(int size_x, int size_y);
-  void PreMultiply();
+  void DebugVerifyBitmapIsPreMultiplied() const override;
 #endif
 
  protected:
diff --git a/core/fxge/skia/fx_skia_device.cpp b/core/fxge/skia/fx_skia_device.cpp
index 148f623124..222c8466e0 100644
--- a/core/fxge/skia/fx_skia_device.cpp
+++ b/core/fxge/skia/fx_skia_device.cpp
@@ -86,41 +86,18 @@ void DebugDrawSkiaClipPath(SkCanvas* canvas, const SkPath& path) {}
 #undef SHOW_SKIA_PATH
 #undef DRAW_SKIA_CLIP
 
-static void DebugVerifyBitmapIsPreMultiplied(void* buffer,
-                                             int width,
-                                             int height) {
-#ifdef SK_DEBUG
-  // verify that input is really premultiplied
-  for (int y = 0; y < height; ++y) {
-    const uint32_t* srcRow = static_cast<const uint32_t*>(buffer) + y * width;
-    for (int x = 0; x < width; ++x) {
-      uint8_t a = SkGetPackedA32(srcRow[x]);
-      uint8_t r = SkGetPackedR32(srcRow[x]);
-      uint8_t g = SkGetPackedG32(srcRow[x]);
-      uint8_t b = SkGetPackedB32(srcRow[x]);
-      SkA32Assert(a);
-      SkASSERT(r <= a);
-      SkASSERT(g <= a);
-      SkASSERT(b <= a);
-    }
-  }
-#endif
-}
-
 static void DebugValidate(const CFX_DIBitmap* bitmap,
                           const CFX_DIBitmap* device) {
   if (bitmap) {
     SkASSERT(bitmap->GetBPP() == 8 || bitmap->GetBPP() == 32);
     if (bitmap->GetBPP() == 32) {
-      DebugVerifyBitmapIsPreMultiplied(bitmap->GetBuffer(), bitmap->GetWidth(),
-                                       bitmap->GetHeight());
+      bitmap->DebugVerifyBitmapIsPreMultiplied();
     }
   }
   if (device) {
     SkASSERT(device->GetBPP() == 8 || device->GetBPP() == 32);
     if (device->GetBPP() == 32) {
-      DebugVerifyBitmapIsPreMultiplied(device->GetBuffer(), device->GetWidth(),
-                                       device->GetHeight());
+      device->DebugVerifyBitmapIsPreMultiplied();
     }
   }
 }
@@ -291,98 +268,6 @@ bool AddStitching(const CPDF_StitchFunc* pFunc,
   return true;
 }
 
-void RgbByteOrderTransferBitmap(CFX_DIBitmap* pBitmap,
-                                int dest_left,
-                                int dest_top,
-                                int width,
-                                int height,
-                                const CFX_DIBSource* pSrcBitmap,
-                                int src_left,
-                                int src_top) {
-  if (!pBitmap)
-    return;
-  pBitmap->GetOverlapRect(dest_left, dest_top, width, height,
-                          pSrcBitmap->GetWidth(), pSrcBitmap->GetHeight(),
-                          src_left, src_top, nullptr);
-  if (width == 0 || height == 0)
-    return;
-  int Bpp = pBitmap->GetBPP() / 8;
-  FXDIB_Format dest_format = pBitmap->GetFormat();
-  FXDIB_Format src_format = pSrcBitmap->GetFormat();
-  int pitch = pBitmap->GetPitch();
-  uint8_t* buffer = pBitmap->GetBuffer();
-  if (dest_format == src_format) {
-    for (int row = 0; row < height; row++) {
-      uint8_t* dest_scan = buffer + (dest_top + row) * pitch + dest_left * Bpp;
-      uint8_t* src_scan =
-          (uint8_t*)pSrcBitmap->GetScanline(src_top + row) + src_left * Bpp;
-      if (Bpp == 4) {
-        for (int col = 0; col < width; col++) {
-          FXARGB_SETDIB(dest_scan, FXARGB_MAKE(src_scan[3], src_scan[0],
-                                               src_scan[1], src_scan[2]));
-          dest_scan += 4;
-          src_scan += 4;
-        }
-      } else {
-        for (int col = 0; col < width; col++) {
-          *dest_scan++ = src_scan[2];
-          *dest_scan++ = src_scan[1];
-          *dest_scan++ = src_scan[0];
-          src_scan += 3;
-        }
-      }
-    }
-    return;
-  }
-  uint8_t* dest_buf = buffer + dest_top * pitch + dest_left * Bpp;
-  if (dest_format == FXDIB_Rgb) {
-    if (src_format == FXDIB_Rgb32) {
-      for (int row = 0; row < height; row++) {
-        uint8_t* dest_scan = dest_buf + row * pitch;
-        uint8_t* src_scan =
-            (uint8_t*)pSrcBitmap->GetScanline(src_top + row) + src_left * 4;
-        for (int col = 0; col < width; col++) {
-          *dest_scan++ = src_scan[2];
-          *dest_scan++ = src_scan[1];
-          *dest_scan++ = src_scan[0];
-          src_scan += 4;
-        }
-      }
-    } else {
-      ASSERT(FALSE);
-    }
-  } else if (dest_format == FXDIB_Argb || dest_format == FXDIB_Rgb32) {
-    if (src_format == FXDIB_Rgb) {
-      for (int row = 0; row < height; row++) {
-        uint8_t* dest_scan = (uint8_t*)(dest_buf + row * pitch);
-        uint8_t* src_scan =
-            (uint8_t*)pSrcBitmap->GetScanline(src_top + row) + src_left * 3;
-        for (int col = 0; col < width; col++) {
-          FXARGB_SETDIB(dest_scan, FXARGB_MAKE(0xff, src_scan[0], src_scan[1],
-                                               src_scan[2]));
-          dest_scan += 4;
-          src_scan += 3;
-        }
-      }
-    } else if (src_format == FXDIB_Rgb32) {
-      ASSERT(dest_format == FXDIB_Argb);
-      for (int row = 0; row < height; row++) {
-        uint8_t* dest_scan = dest_buf + row * pitch;
-        uint8_t* src_scan =
-            (uint8_t*)(pSrcBitmap->GetScanline(src_top + row) + src_left * 4);
-        for (int col = 0; col < width; col++) {
-          FXARGB_SETDIB(dest_scan, FXARGB_MAKE(0xff, src_scan[0], src_scan[1],
-                                               src_scan[2]));
-          src_scan += 4;
-          dest_scan += 4;
-        }
-      }
-    }
-  } else {
-    ASSERT(FALSE);
-  }
-}
-
 // see https://en.wikipedia.org/wiki/Distance_from_a_point_to_a_line
 SkScalar LineSide(const SkPoint line[2], const SkPoint& pt) {
   return (line[1].fY - line[0].fY) * pt.fX - (line[1].fX - line[0].fX) * pt.fY +
@@ -525,11 +410,13 @@ class SkiaState {
       m_skPath.setFillType((fill_mode & 3) == FXFILL_ALTERNATE
                                ? SkPath::kEvenOdd_FillType
                                : SkPath::kWinding_FillType);
-      m_drawState = *pDrawState;
+      if (pDrawState)
+        m_drawState.Copy(*pDrawState);
       m_fillColor = fill_color;
       m_strokeColor = stroke_color;
       m_blendType = blend_type;
-      m_drawMatrix = *pMatrix;
+      if (pMatrix)
+        m_drawMatrix = *pMatrix;
     }
     SkPath skPath = BuildPath(pPathData);
     SkPoint delta;
@@ -633,6 +520,7 @@ class SkiaState {
     skPaint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
     skPaint.setTextSize(m_fontSize);
     skPaint.setSubpixelText(true);
+    skPaint.setHinting(SkPaint::kNo_Hinting);
     SkCanvas* skCanvas = pDriver->SkiaCanvas();
     skCanvas->save();
     skCanvas->concat(skMatrix);
@@ -690,6 +578,9 @@ class SkiaState {
   }
 
   bool MatrixOffset(const CFX_Matrix* pMatrix, SkPoint* delta) {
+    CFX_Matrix identityMatrix;
+    if (!pMatrix)
+      pMatrix = &identityMatrix;
     delta->set(pMatrix->e - m_drawMatrix.e, pMatrix->f - m_drawMatrix.f);
     if (!delta->fX && !delta->fY)
       return true;
@@ -789,12 +680,18 @@ class SkiaState {
   }
 
   bool MatrixChanged(const CFX_Matrix* pMatrix, const CFX_Matrix& refMatrix) {
+    CFX_Matrix identityMatrix;
+    if (!pMatrix)
+      pMatrix = &identityMatrix;
     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) {
+    CFX_GraphStateData identityState;
+    if (!pState)
+      pState = &identityState;
     return pState->m_LineWidth != refState.m_LineWidth ||
            pState->m_LineCap != refState.m_LineCap ||
            pState->m_LineJoin != refState.m_LineJoin ||
@@ -804,9 +701,10 @@ class SkiaState {
 
   bool DashChanged(const CFX_GraphStateData* pState,
                    const CFX_GraphStateData& refState) {
-    if (!pState->m_DashArray && !refState.m_DashArray)
+    bool dashArray = pState && pState->m_DashArray;
+    if (!dashArray && !refState.m_DashArray)
       return false;
-    if (!pState->m_DashArray || !refState.m_DashArray)
+    if (!dashArray || !refState.m_DashArray)
       return true;
     if (pState->m_DashPhase != refState.m_DashPhase ||
         pState->m_DashCount != refState.m_DashCount) {
@@ -946,7 +844,6 @@ CFX_SkiaDeviceDriver::CFX_SkiaDeviceDriver(CFX_DIBitmap* pBitmap,
       m_pOriDevice(pOriDevice),
       m_pRecorder(nullptr),
       m_pCache(new SkiaState),
-      m_bRgbByteOrder(bRgbByteOrder),
       m_bGroupKnockout(bGroupKnockout) {
   SkBitmap skBitmap;
   SkASSERT(pBitmap->GetBPP() == 8 || pBitmap->GetBPP() == 32);
@@ -967,7 +864,6 @@ CFX_SkiaDeviceDriver::CFX_SkiaDeviceDriver(int size_x, int size_y)
       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));
   m_pCanvas = m_pRecorder->getRecordingCanvas();
@@ -978,7 +874,6 @@ CFX_SkiaDeviceDriver::CFX_SkiaDeviceDriver(SkPictureRecorder* recorder)
       m_pOriDevice(nullptr),
       m_pRecorder(recorder),
       m_pCache(new SkiaState),
-      m_bRgbByteOrder(FALSE),
       m_bGroupKnockout(FALSE) {
   m_pCanvas = m_pRecorder->getRecordingCanvas();
 }
@@ -1012,6 +907,7 @@ FX_BOOL CFX_SkiaDeviceDriver::DrawDeviceText(int nChars,
   paint.setColor(color);
   paint.setTypeface(typeface);
   paint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
+  paint.setHinting(SkPaint::kNo_Hinting);
   paint.setTextSize(font_size);
   paint.setSubpixelText(true);
   m_pCanvas->save();
@@ -1354,35 +1250,33 @@ FX_BOOL CFX_SkiaDeviceDriver::GetClipBox(FX_RECT* pRect) {
 FX_BOOL CFX_SkiaDeviceDriver::GetDIBits(CFX_DIBitmap* pBitmap,
                                         int left,
                                         int top) {
-  if (!m_pBitmap || !m_pBitmap->GetBuffer())
+  if (!m_pBitmap)
     return TRUE;
-
-  FX_RECT rect(left, top, left + pBitmap->GetWidth(),
-               top + pBitmap->GetHeight());
-  std::unique_ptr<CFX_DIBitmap> pBack;
-  if (m_pOriDevice) {
-    pBack.reset(m_pOriDevice->Clone(&rect));
-    if (!pBack)
-      return TRUE;
-
-    pBack->CompositeBitmap(0, 0, pBack->GetWidth(), pBack->GetHeight(),
-                           m_pBitmap, 0, 0);
-  } else {
-    pBack.reset(m_pBitmap->Clone(&rect));
-    if (!pBack)
-      return TRUE;
-  }
-
-  left = std::min(left, 0);
-  top = std::min(top, 0);
-  if (m_bRgbByteOrder) {
-    RgbByteOrderTransferBitmap(pBitmap, 0, 0, rect.Width(), rect.Height(),
-                               pBack.get(), left, top);
+  uint8_t* srcBuffer = m_pBitmap->GetBuffer();
+  if (!srcBuffer)
     return TRUE;
-  }
-
-  return pBitmap->TransferBitmap(0, 0, rect.Width(), rect.Height(), pBack.get(),
-                                 left, top);
+  int srcWidth = m_pBitmap->GetWidth();
+  int srcHeight = m_pBitmap->GetHeight();
+  int srcRowBytes = srcWidth * sizeof(uint32_t);
+  SkImageInfo srcImageInfo = SkImageInfo::Make(
+      srcWidth, srcHeight, SkColorType::kN32_SkColorType, kPremul_SkAlphaType);
+  SkBitmap skSrcBitmap;
+  skSrcBitmap.installPixels(srcImageInfo, srcBuffer, srcRowBytes, nullptr,
+                            nullptr, nullptr);
+  SkASSERT(pBitmap);
+  uint8_t* dstBuffer = pBitmap->GetBuffer();
+  SkASSERT(dstBuffer);
+  int dstWidth = pBitmap->GetWidth();
+  int dstHeight = pBitmap->GetHeight();
+  int dstRowBytes = dstWidth * sizeof(uint32_t);
+  SkImageInfo dstImageInfo = SkImageInfo::Make(
+      dstWidth, dstHeight, SkColorType::kN32_SkColorType, kPremul_SkAlphaType);
+  SkBitmap skDstBitmap;
+  skDstBitmap.installPixels(dstImageInfo, dstBuffer, dstRowBytes, nullptr,
+                            nullptr, nullptr);
+  SkCanvas canvas(skDstBitmap);
+  canvas.drawBitmap(skSrcBitmap, left, top, nullptr);
+  return TRUE;
 }
 
 CFX_DIBitmap* CFX_SkiaDeviceDriver::GetBackDrop() {
@@ -1491,7 +1385,7 @@ FX_BOOL CFX_SkiaDeviceDriver::StartDIBits(const CFX_DIBSource* pSource,
     case 32:
       colorType = SkColorType::kN32_SkColorType;
       alphaType = kPremul_SkAlphaType;
-      DebugVerifyBitmapIsPreMultiplied(buffer, width, height);
+      pSource->DebugVerifyBitmapIsPreMultiplied(buffer);
       break;
     default:
       SkASSERT(0);  // TODO(caryclark) ensure that all cases are covered
@@ -1528,10 +1422,6 @@ FX_BOOL CFX_SkiaDeviceDriver::ContinueDIBits(void* handle, IFX_Pause* pPause) {
   return FALSE;
 }
 
-void CFX_SkiaDeviceDriver::PreMultiply() {
-  PreMultiply(m_pBitmap);
-}
-
 void CFX_SkiaDeviceDriver::PreMultiply(CFX_DIBitmap* pDIBitmap) {
   void* buffer = pDIBitmap->GetBuffer();
   if (!buffer)
@@ -1549,7 +1439,7 @@ void CFX_SkiaDeviceDriver::PreMultiply(CFX_DIBitmap* pDIBitmap) {
       SkImageInfo::Make(width, height, kN32_SkColorType, kPremul_SkAlphaType);
   SkPixmap premultiplied(premultipliedInfo, buffer, rowBytes);
   unpremultiplied.readPixels(premultiplied);
-  DebugVerifyBitmapIsPreMultiplied(buffer, width, height);
+  pDIBitmap->DebugVerifyBitmapIsPreMultiplied();
 }
 
 void CFX_SkiaDeviceDriver::Dump() const {
@@ -1559,6 +1449,11 @@ void CFX_SkiaDeviceDriver::Dump() const {
 #endif
 }
 
+void CFX_SkiaDeviceDriver::DebugVerifyBitmapIsPreMultiplied() const {
+  if (m_pOriDevice)
+    m_pOriDevice->DebugVerifyBitmapIsPreMultiplied();
+}
+
 CFX_FxgeDevice::CFX_FxgeDevice() {
   m_bOwnedBitmap = FALSE;
 }
@@ -1606,12 +1501,42 @@ bool CFX_FxgeDevice::Create(int width,
 }
 
 CFX_FxgeDevice::~CFX_FxgeDevice() {
+  Flush();
+  // call destructor of CFX_RenderDevice / CFX_SkiaDeviceDriver immediately
   if (m_bOwnedBitmap && GetBitmap())
     delete GetBitmap();
 }
 
-void CFX_FxgeDevice::PreMultiply() {
-  (static_cast<CFX_SkiaDeviceDriver*>(GetDeviceDriver()))->PreMultiply();
+void CFX_FxgeDevice::DebugVerifyBitmapIsPreMultiplied() const {
+#ifdef SK_DEBUG
+  CFX_SkiaDeviceDriver* skDriver =
+      static_cast<CFX_SkiaDeviceDriver*>(GetDeviceDriver());
+  if (skDriver)
+    skDriver->DebugVerifyBitmapIsPreMultiplied();
+#endif
+}
+
+void CFX_DIBSource::DebugVerifyBitmapIsPreMultiplied(void* opt) const {
+#ifdef SK_DEBUG
+  SkASSERT(32 == GetBPP());
+  const uint32_t* buffer = (const uint32_t*)(opt ? opt : GetBuffer());
+  int width = GetWidth();
+  int height = GetHeight();
+  // verify that input is really premultiplied
+  for (int y = 0; y < height; ++y) {
+    const uint32_t* srcRow = buffer + y * width;
+    for (int x = 0; x < width; ++x) {
+      uint8_t a = SkGetPackedA32(srcRow[x]);
+      uint8_t r = SkGetPackedR32(srcRow[x]);
+      uint8_t g = SkGetPackedG32(srcRow[x]);
+      uint8_t b = SkGetPackedB32(srcRow[x]);
+      SkA32Assert(a);
+      SkASSERT(r <= a);
+      SkASSERT(g <= a);
+      SkASSERT(b <= a);
+    }
+  }
+#endif
 }
 
 #endif
diff --git a/core/fxge/skia/fx_skia_device.h b/core/fxge/skia/fx_skia_device.h
index f7e5306ab5..a7a5f85e3e 100644
--- a/core/fxge/skia/fx_skia_device.h
+++ b/core/fxge/skia/fx_skia_device.h
@@ -125,9 +125,9 @@ class CFX_SkiaDeviceDriver : public IFX_RenderDeviceDriver {
                    const SkMatrix& matrix);
   void Flush();
   SkPictureRecorder* GetRecorder() const { return m_pRecorder; }
-  void PreMultiply();
   static void PreMultiply(CFX_DIBitmap* pDIBitmap);
   SkCanvas* SkiaCanvas() { return m_pCanvas; }
+  void DebugVerifyBitmapIsPreMultiplied() const;
   void Dump() const;
 
  private:
@@ -138,7 +138,6 @@ class CFX_SkiaDeviceDriver : public IFX_RenderDeviceDriver {
   SkCanvas* m_pCanvas;
   SkPictureRecorder* const m_pRecorder;
   std::unique_ptr<SkiaState> m_pCache;
-  FX_BOOL m_bRgbByteOrder;
   FX_BOOL m_bGroupKnockout;
 };
 #endif  // defined(_SKIA_SUPPORT_)