From b333c9ec415c132451f4a10487b84f91124d08e8 Mon Sep 17 00:00:00 2001 From: Cary Clark Date: Wed, 12 Apr 2017 11:53:32 -0400 Subject: Clip stack management was off by one. To help find future bugs, add printf and checking to see that the skia clip stack and the pdfium clip stack are in sync. Bug: 705783,795821 Change-Id: I3ed39cbe2514ab18e5bee6eb363cc2d0f042bff5 Reviewed-on: https://pdfium-review.googlesource.com/4090 Reviewed-by: dsinclair Commit-Queue: dsinclair --- core/fxge/skia/fx_skia_device.cpp | 179 +++++++++++++++++++++++++++++++++++--- 1 file changed, 169 insertions(+), 10 deletions(-) diff --git a/core/fxge/skia/fx_skia_device.cpp b/core/fxge/skia/fx_skia_device.cpp index c9bc6f472f..eacfd69c93 100644 --- a/core/fxge/skia/fx_skia_device.cpp +++ b/core/fxge/skia/fx_skia_device.cpp @@ -153,6 +153,9 @@ void RgbByteOrderTransferBitmap(const CFX_RetainPtr& pBitmap, #endif // _SKIA_SUPPORT_PATHS_ #define SHOW_SKIA_PATH 0 // set to 1 to print the path contents +#if SHOW_SKIA_PATH +#define SHOW_SKIA_PATH_SHORTHAND 0 // set to 1 for abbreviated path contents +#endif #define DRAW_SKIA_CLIP 0 // set to 1 to draw a green rectangle around the clip #if SHOW_SKIA_PATH @@ -176,12 +179,16 @@ void DebugShowCanvasMatrix(const SkCanvas* canvas) { void DebugShowSkiaPath(const SkPath& path) { #if SHOW_SKIA_PATH +#if SHOW_SKIA_PATH_SHORTHAND + printf(" **\n"); +#else SkDynamicMemoryWStream stream; path.dump(&stream, false, false); std::unique_ptr storage; storage.reset(FX_Alloc(char, stream.bytesWritten())); stream.copyTo(storage.get()); - printf("%s", storage.get()); + printf("%.*s", (int)stream.bytesWritten(), storage.get()); +#endif // SHOW_SKIA_PATH_SHORTHAND #endif // SHOW_SKIA_PATH } @@ -923,20 +930,20 @@ class SkiaState { Dump(__func__); int count = m_commands.count(); if (m_commandIndex < count - 1) { - if (Clip::kSave == m_commands[m_commandIndex + 1]) { + if (Clip::kSave == m_commands[m_commandIndex]) { ++m_commandIndex; return true; } Flush(); AdjustClip(m_commandIndex); - m_commands[++m_commandIndex] = Clip::kSave; + m_commands[m_commandIndex] = Clip::kSave; m_clips[m_commandIndex] = m_skEmptyPath; } else { AdjustClip(m_commandIndex); m_commands.push(Clip::kSave); m_clips.push_back(m_skEmptyPath); - ++m_commandIndex; } + ++m_commandIndex; return true; } @@ -1071,34 +1078,177 @@ class SkiaState { void Dump(const char* where) const { #if SHOW_SKIA_PATH - printf("\n%s\nSkia Save Count %d:\n", where, - m_pDriver->m_pCanvas->getSaveCount()); + if (m_debugDisable) + return; + printf( + "\n%s\nSkia Save Count %d Agg Save Count %d" + " Cache Save Count %d\n", + where, m_pDriver->m_pCanvas->getSaveCount(), + (int)m_pDriver->m_StateStack.size(), m_commandIndex); printf("Cache:\n"); +#if SHOW_SKIA_PATH_SHORTHAND + bool dumpedPath = false; +#endif for (int index = 0; index < m_commands.count(); ++index) { +#if SHOW_SKIA_PATH_SHORTHAND + if (Clip::kSave == m_commands[index] && dumpedPath) { + printf("\n"); + dumpedPath = false; + } +#endif DumpPrefix(index); switch (m_commands[index]) { case Clip::kSave: printf("Save %d\n", ++m_debugSaveCounter); break; case Clip::kPath: +#if SHOW_SKIA_PATH_SHORTHAND + printf("*"); + dumpedPath = true; +#else m_clips[index].dump(); +#endif break; default: printf("unknown\n"); } } +#if SHOW_SKIA_PATH_SHORTHAND + if (dumpedPath) + printf("\n"); +#endif DumpEndPrefix(); -#endif // SHOW_SKIA_PATH -#ifdef SK_DEBUG int skCanvasSaveCount = m_pDriver->m_pCanvas->getSaveCount(); int cacheSaveCount = 1; SkASSERT(m_clipIndex <= m_commands.count()); for (int index = 0; index < m_clipIndex; ++index) cacheSaveCount += Clip::kSave == m_commands[index]; SkASSERT(skCanvasSaveCount == cacheSaveCount); -#endif +#endif // SHOW_SKIA_PATH + } + + void DebugCheckClip() { +#if SHOW_SKIA_PATH + if (m_debugDisable) + return; + int aggSaveCount = 0; + FX_RECT last; + bool foundLast = false; + for (int index = 0; index < (int)m_pDriver->m_StateStack.size(); ++index) { + if (!m_pDriver->m_StateStack[index]) { + continue; + } + if (m_pDriver->m_StateStack[index]->GetType() != CFX_ClipRgn::RectI) { + aggSaveCount += 1; + foundLast = false; + continue; + } + if (!foundLast || memcmp(&last, &m_pDriver->m_StateStack[index]->GetBox(), + sizeof(FX_RECT))) { + aggSaveCount += 1; + foundLast = true; + last = m_pDriver->m_StateStack[index]->GetBox(); + } + } + if (m_pDriver->m_pClipRgn) { + CFX_ClipRgn::ClipType clipType = m_pDriver->m_pClipRgn->GetType(); + if (clipType != CFX_ClipRgn::RectI || !foundLast || + memcmp(&last, &m_pDriver->m_pClipRgn->GetBox(), sizeof(FX_RECT))) { + aggSaveCount += 1; + } + } + int cacheSaveCount = 0; + SkASSERT(m_clipIndex <= m_commands.count()); + bool newPath = false; + for (int index = 0; index < m_commandIndex; ++index) { + if (Clip::kSave == m_commands[index]) { + newPath = true; + } else if (newPath) { + ++cacheSaveCount; + newPath = false; + } + } + if (aggSaveCount != cacheSaveCount) { + DumpClipStacks(); + SkASSERT(0); + return; + } + for (int aggIndex = 0; aggIndex < (int)m_pDriver->m_StateStack.size(); + ++aggIndex) { + if (!m_pDriver->m_StateStack[aggIndex]) + continue; + if (m_pDriver->m_StateStack[aggIndex]->GetType() != CFX_ClipRgn::RectI) + continue; + const FX_RECT& aggRect = m_pDriver->m_StateStack[aggIndex]->GetBox(); + SkRect skRect = SkRect::MakeLTRB(aggRect.left, aggRect.top, aggRect.right, + aggRect.bottom); + bool foundMatch = false; + for (int skIndex = 0; skIndex < m_commandIndex; ++skIndex) { + if (Clip::kPath != m_commands[skIndex]) + continue; + const SkPath& clip = m_clips[skIndex]; + SkRect bounds; + if (!clip.isRect(&bounds)) + continue; + bounds.roundOut(&bounds); + if (skRect == bounds) { + foundMatch = true; + break; + } + } + if (!foundMatch) { + DumpClipStacks(); + SkASSERT(0); + } + } +#endif // SHOW_SKIA_PATH } +#if SHOW_SKIA_PATH + void DumpClipStacks() const { + if (m_debugDisable) + return; + printf("\ncache\n"); + for (int index = 0; index < m_commandIndex; ++index) { + DumpPrefix(index); + switch (m_commands[index]) { + case Clip::kSave: + printf("Save\n"); + break; + case Clip::kPath: + m_clips[index].dump(); + break; + default: + printf("unknown\n"); + } + } + printf("\nagg\n"); + for (int index = 0; index < (int)m_pDriver->m_StateStack.size(); ++index) { + if (!m_pDriver->m_StateStack[index]) { + printf("null\n"); + continue; + } + CFX_ClipRgn::ClipType clipType = + m_pDriver->m_StateStack[index]->GetType(); + const FX_RECT& box = m_pDriver->m_StateStack[index]->GetBox(); + printf("stack rect: %d,%d,%d,%d mask=%s\n", box.left, box.top, box.right, + box.bottom, + CFX_ClipRgn::MaskF == clipType + ? "1" + : CFX_ClipRgn::RectI == clipType ? "0" : "?"); + } + if (m_pDriver->m_pClipRgn) { + const FX_RECT& box = m_pDriver->m_pClipRgn->GetBox(); + CFX_ClipRgn::ClipType clipType = m_pDriver->m_pClipRgn->GetType(); + printf("clip rect: %d,%d,%d,%d mask=%s\n", box.left, box.top, box.right, + box.bottom, + CFX_ClipRgn::MaskF == clipType + ? "1" + : CFX_ClipRgn::RectI == clipType ? "0" : "?"); + } + } +#endif // SHOW_SKIA_PATH + private: SkTArray m_clips; // stack of clips that may be reused SkTDArray m_commands; // stack of clip-related commands @@ -1125,6 +1275,7 @@ class SkiaState { bool m_groupKnockout; bool m_debugDisable; // turn off cache for debugging #if SHOW_SKIA_PATH + public: mutable int m_debugSaveCounter; static int m_debugInitCounter; #endif @@ -1355,10 +1506,14 @@ int CFX_SkiaDeviceDriver::GetDeviceCaps(int caps_id) const { } void CFX_SkiaDeviceDriver::SaveState() { + m_pCache->DebugCheckClip(); if (!m_pCache->ClipSave()) m_pCanvas->save(); #ifdef _SKIA_SUPPORT_PATHS_ +#if SHOW_SKIA_PATH + printf("SaveState %zd\n", m_StateStack.size()); +#endif std::unique_ptr pClip; if (m_pClipRgn) pClip = pdfium::MakeUnique(*m_pClipRgn); @@ -1376,12 +1531,15 @@ void CFX_SkiaDeviceDriver::RestoreState(bool bKeepSaved) { if (m_pCache->IsEmpty()) return; #endif - if (!m_pCache->ClipRestore()) m_pCanvas->restore(); if (bKeepSaved && !m_pCache->ClipSave()) m_pCanvas->save(); #ifdef _SKIA_SUPPORT_PATHS_ +#if SHOW_SKIA_PATH + printf("RestoreState %zd %s\n", m_StateStack.size(), + bKeepSaved ? "bKeepSaved" : ""); +#endif if (bKeepSaved) { if (m_StateStack.back()) m_pClipRgn = pdfium::MakeUnique(*m_StateStack.back()); @@ -1389,6 +1547,7 @@ void CFX_SkiaDeviceDriver::RestoreState(bool bKeepSaved) { m_pClipRgn = std::move(m_StateStack.back()); m_StateStack.pop_back(); } + m_pCache->DebugCheckClip(); #endif // _SKIA_SUPPORT_PATHS_ } -- cgit v1.2.3