fix/improve Skia debug messages

CopyBits() wasn't reporting 'src' if it was different from 'this'.
Put the 'O' for offscreen after 'G' or 'R', so that it doesn't look
like 0 being part of the size.
Add pointer value to the Idle instances debug name.

Change-Id: I001f4265696ff2b15e0273b3ae0c3857b39e2a0a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/91835
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

2 files changed, 31 insertions, 4 deletions

diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx
index a145e001850d..a99bb9ae4ce1 100644
--- a/vcl/inc/skia/gdiimpl.hxx
+++ b/vcl/inc/skia/gdiimpl.hxx
@@ -269,7 +269,7 @@ protected:
     { // O - offscreen, G - GPU-based, R - raster
         return stream << static_cast<const void*>(graphics) << " "
                       << Size(graphics->GetWidth(), graphics->GetHeight())
-                      << (graphics->isOffscreen() ? "O" : "") << (graphics->isGPU() ? "G" : "R");
+                      << (graphics->isGPU() ? "G" : "R") << (graphics->isOffscreen() ? "O" : "");
     }
 
     SalGraphics& mParent;
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index 8b484ea124c7..b9d6d3575c85 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -156,15 +156,31 @@ bool checkInvalidSourceOrDestination(SalTwoRect const& rPosAry)
 class SkiaFlushIdle : public Idle
 {
     SkiaSalGraphicsImpl* mpGraphics;
+#ifndef NDEBUG
+    char* debugname;
+#endif
 
 public:
     explicit SkiaFlushIdle(SkiaSalGraphicsImpl* pGraphics)
-        : Idle("skia idle swap")
+        : Idle(get_debug_name(pGraphics))
         , mpGraphics(pGraphics)
     {
         // We don't want to be swapping before we've painted.
         SetPriority(TaskPriority::POST_PAINT);
     }
+#ifndef NDEBUG
+    virtual ~SkiaFlushIdle() { free(debugname); }
+    const char* get_debug_name(SkiaSalGraphicsImpl* pGraphics)
+    {
+        // Idle keeps just a pointer, so we need to store the string
+        debugname = strdup(
+            OString("skia idle 0x" + OString::number(reinterpret_cast<sal_uIntPtr>(pGraphics), 16))
+                .getStr());
+        return debugname;
+    }
+#else
+    const char* get_debug_name(SkiaSalGraphicsImpl*) { return "skia idle"; }
+#endif
 
     virtual void Invoke() override
     {
@@ -895,13 +911,24 @@ void SkiaSalGraphicsImpl::copyBits(const SalTwoRect& rPosAry, SalGraphics* pSrcG
         src = this;
     if (rPosAry.mnSrcWidth == rPosAry.mnDestWidth && rPosAry.mnSrcHeight == rPosAry.mnDestHeight)
     {
-        SAL_INFO("vcl.skia.trace", "copybits(" << this << "): copy area:" << rPosAry);
+        auto srcDebug = [&]() -> std::string {
+            if (src == this)
+                return "(self)";
+            else
+            {
+                std::ostringstream stream;
+                stream << "(" << src << ")";
+                return stream.str();
+            }
+        };
+        SAL_INFO("vcl.skia.trace",
+                 "copybits(" << this << "): " << srcDebug() << " copy area: " << rPosAry);
         ::copyArea(getDrawCanvas(), src->mSurface, rPosAry.mnDestX, rPosAry.mnDestY, rPosAry.mnSrcX,
                    rPosAry.mnSrcY, rPosAry.mnDestWidth, rPosAry.mnDestHeight);
     }
     else
     {
-        SAL_INFO("vcl.skia.trace", "copybits(" << this << "): (" << src << "):" << rPosAry);
+        SAL_INFO("vcl.skia.trace", "copybits(" << this << "): (" << src << "): " << rPosAry);
         // Do not use makeImageSnapshot(rect), as that one may make a needless data copy.
         sk_sp<SkImage> image = src->mSurface->makeImageSnapshot();
         SkPaint paint;