track dirty areas for Skia drawing

Updates to the screen in raster mode aren't _that_ slow, in fact
it seems using SkRegion can make things slower because of manipulating
the region, but with SkIRect this could sometimes help a bit.
It also appears that StretchDIBits() that is used by the Windows
raster code doesn't work correctly if only a subset of the y-axis
range is specified, which reduces the usefulness.

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

6 files changed, 153 insertions, 24 deletions

diff --git a/external/skia/UnpackedTarball_skia.mk b/external/skia/UnpackedTarball_skia.mk
index 0e486a916388..dfafc00d66ec 100644
--- a/external/skia/UnpackedTarball_skia.mk
+++ b/external/skia/UnpackedTarball_skia.mk
@@ -38,6 +38,7 @@ skia_patches := \
     public-make-from-backend-texture.patch.1 \
     c++20.patch.0 \
     constexpr-debug-std-max.patch.1 \
+    swap-buffers-rect.patch.1
 
 $(eval $(call gb_UnpackedTarball_set_patchlevel,skia,1))
 
diff --git a/external/skia/swap-buffers-rect.patch.1 b/external/skia/swap-buffers-rect.patch.1
new file mode 100644
index 000000000000..d04ea91c0bc9
--- /dev/null
+++ b/external/skia/swap-buffers-rect.patch.1
@@ -0,0 +1,114 @@
+diff --git a/tools/sk_app/VulkanWindowContext.cpp b/tools/sk_app/VulkanWindowContext.cpp
+index 66670c892e..3a6804166f 100644
+--- a/tools/sk_app/VulkanWindowContext.cpp
++++ b/tools/sk_app/VulkanWindowContext.cpp
+@@ -553,7 +553,7 @@ sk_sp<SkSurface> VulkanWindowContext::getBackbufferSurface() {
+     return sk_ref_sp(surface);
+ }
+ 
+-void VulkanWindowContext::swapBuffers() {
++void VulkanWindowContext::swapBuffers(const SkIRect*) {
+ 
+     BackbufferInfo* backbuffer = fBackbuffers + fCurrentBackbufferIndex;
+     SkSurface* surface = fSurfaces[backbuffer->fImageIndex].get();
+diff --git a/tools/sk_app/VulkanWindowContext.h b/tools/sk_app/VulkanWindowContext.h
+index 07a18a46a9..aa6f95e2a1 100644
+--- a/tools/sk_app/VulkanWindowContext.h
++++ b/tools/sk_app/VulkanWindowContext.h
+@@ -48,7 +48,7 @@ public:
+     static SharedGrDirectContext getSharedGrDirectContext() { return SharedGrDirectContext( fGlobalShared ); }
+ 
+     sk_sp<SkSurface> getBackbufferSurface() override;
+-    void swapBuffers() override;
++    void swapBuffers(const SkIRect* rect = nullptr) override;
+ 
+     bool isValid() override { return fSurface != VK_NULL_HANDLE; }
+ 
+diff --git a/tools/sk_app/WindowContext.h b/tools/sk_app/WindowContext.h
+index 6920e5ba89..219330a61b 100644
+--- a/tools/sk_app/WindowContext.h
++++ b/tools/sk_app/WindowContext.h
+@@ -8,6 +8,7 @@
+ #define WindowContext_DEFINED
+ 
+ #include "include/core/SkRefCnt.h"
++#include "include/core/SkRect.h"
+ #include "include/core/SkSurfaceProps.h"
+ #include "include/gpu/GrTypes.h"
+ #include "include/gpu/GrDirectContext.h"
+@@ -29,7 +30,7 @@ public:
+ 
+     virtual sk_sp<SkSurface> getBackbufferSurface() = 0;
+ 
+-    virtual void swapBuffers() = 0;
++    virtual void swapBuffers(const SkIRect* rect = nullptr) = 0;
+ 
+     virtual bool isValid() = 0;
+ 
+diff --git a/tools/sk_app/unix/RasterWindowContext_unix.cpp b/tools/sk_app/unix/RasterWindowContext_unix.cpp
+index e8ae942308..fc06b40069 100644
+--- a/tools/sk_app/unix/RasterWindowContext_unix.cpp
++++ b/tools/sk_app/unix/RasterWindowContext_unix.cpp
+@@ -19,7 +19,7 @@ public:
+     RasterWindowContext_xlib(Display*, XWindow, int width, int height, const DisplayParams&);
+ 
+     sk_sp<SkSurface> getBackbufferSurface() override;
+-    void swapBuffers() override;
++    void swapBuffers(const SkIRect* rect) override;
+     bool isValid() override { return SkToBool(fWindow); }
+     void resize(int  w, int h) override;
+     void setDisplayParams(const DisplayParams& params) override;
+@@ -60,7 +60,7 @@ void RasterWindowContext_xlib::resize(int  w, int h) {
+ 
+ sk_sp<SkSurface> RasterWindowContext_xlib::getBackbufferSurface() { return fBackbufferSurface; }
+ 
+-void RasterWindowContext_xlib::swapBuffers() {
++void RasterWindowContext_xlib::swapBuffers(const SkIRect* rect) {
+     SkPixmap pm;
+     if (!fBackbufferSurface->peekPixels(&pm)) {
+         return;
+@@ -82,7 +82,9 @@ void RasterWindowContext_xlib::swapBuffers() {
+     if (!XInitImage(&image)) {
+         return;
+     }
+-    XPutImage(fDisplay, fWindow, fGC, &image, 0, 0, 0, 0, pm.width(), pm.height());
++    SkIRect update = rect ? *rect : SkIRect::MakeWH( pm.width(), pm.height());
++    XPutImage(fDisplay, fWindow, fGC, &image, update.x(), update.y(),
++              update.x(), update.y(), update.width(), update.height());
+ }
+ 
+ }  // anonymous namespace
+diff --git a/tools/sk_app/win/RasterWindowContext_win.cpp b/tools/sk_app/win/RasterWindowContext_win.cpp
+index 49f1f9ed17..f0db1f6f06 100644
+--- a/tools/sk_app/win/RasterWindowContext_win.cpp
++++ b/tools/sk_app/win/RasterWindowContext_win.cpp
+@@ -22,7 +22,7 @@ public:
+     RasterWindowContext_win(HWND, const DisplayParams&);
+ 
+     sk_sp<SkSurface> getBackbufferSurface() override;
+-    void swapBuffers() override;
++    void swapBuffers(const SkIRect* rect) override;
+     bool isValid() override { return SkToBool(fWnd); }
+     void resize(int w, int h) override;
+     void setDisplayParams(const DisplayParams& params) override;
+@@ -75,13 +75,17 @@ void RasterWindowContext_win::resize(int w, int h) {
+ 
+ sk_sp<SkSurface> RasterWindowContext_win::getBackbufferSurface() { return fBackbufferSurface; }
+ 
+-void RasterWindowContext_win::swapBuffers() {
++void RasterWindowContext_win::swapBuffers(const SkIRect* rect) {
+     BITMAPINFO* bmpInfo = reinterpret_cast<BITMAPINFO*>(fSurfaceMemory.get());
+     HDC dc = GetDC(fWnd);
+     SkPixmap pixmap;
+     fBackbufferSurface->peekPixels(&pixmap);
+-    StretchDIBits(dc, 0, 0, fWidth, fHeight, 0, 0, fWidth, fHeight, pixmap.addr(), bmpInfo,
+-                  DIB_RGB_COLORS, SRCCOPY);
++    SkIRect update = rect ? *rect : SkIRect::MakeWH( fWidth, fHeight );
++    // It appears that y-axis handling is broken if it doesn't match the window size.
++    update = SkIRect::MakeXYWH( update.x(), 0, update.width(), fHeight );
++    StretchDIBits(dc, update.x(), update.y(), update.width(), update.height(),
++                  update.x(), update.y(), update.width(), update.height(),
++                  pixmap.addr(), bmpInfo, DIB_RGB_COLORS, SRCCOPY);
+     ReleaseDC(fWnd, dc);
+ }
+ 
diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx
index 21aaf880ae8c..3cc577300128 100644
--- a/vcl/inc/skia/gdiimpl.hxx
+++ b/vcl/inc/skia/gdiimpl.hxx
@@ -264,12 +264,12 @@ protected:
     SkCanvas* getXorCanvas();
     void applyXor();
     // NOTE: This must be called before the operation does any drawing.
-    void addXorRegion(const SkRect& rect)
+    void addUpdateRegion(const SkRect& rect)
     {
+        // Make slightly larger, just in case (rounding, antialiasing,...).
+        SkIRect addedRect = rect.makeOutset(2, 2).round();
         if (mXorMode)
         {
-            // Make slightly larger, just in case (rounding, antialiasing,...).
-            SkIRect addedRect = rect.makeOutset(2, 2).round();
             // Two xor operations should cancel each other out. We batch xor operations,
             // but if they can overlap, apply xor now, since applyXor() does the operation
             // just once.
@@ -277,6 +277,9 @@ protected:
                 applyXor();
             mXorRegion.op(addedRect, SkRegion::kUnion_Op);
         }
+        // Using SkIRect should be enough, SkRegion would be too slow with many operations
+        // and swapping to the screen is not _that_slow.
+        mDirtyRect.join(addedRect);
     }
     static void setCanvasClipRegion(SkCanvas* canvas, const vcl::Region& region);
     sk_sp<SkImage> mergeCacheBitmaps(const SkiaSalBitmap& bitmap, const SkiaSalBitmap* alphaBitmap,
@@ -318,6 +321,7 @@ protected:
     // The Skia surface that is target of all the rendering.
     sk_sp<SkSurface> mSurface;
     bool mIsGPU; // whether the surface is GPU-backed
+    SkIRect mDirtyRect; // the area that has been changed since the last performFlush()
     vcl::Region mClipRegion;
     Color mLineColor;
     Color mFillColor;
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index fa29ab32d0ac..0ed499ca771d 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -291,6 +291,7 @@ void SkiaSalGraphicsImpl::createSurface()
         createWindowSurface();
     mSurface->getCanvas()->save(); // see SetClipRegion()
     mClipRegion = vcl::Region(tools::Rectangle(0, 0, GetWidth(), GetHeight()));
+    mDirtyRect = SkIRect::MakeWH(GetWidth(), GetHeight());
 
     // We don't want to be swapping before we've painted.
     mFlush->Stop();
@@ -695,7 +696,7 @@ void SkiaSalGraphicsImpl::drawPixel(long nX, long nY, Color nColor)
         return;
     preDraw();
     SAL_INFO("vcl.skia.trace", "drawpixel(" << this << "): " << Point(nX, nY) << ":" << nColor);
-    addXorRegion(SkRect::MakeXYWH(nX, nY, 1, 1));
+    addUpdateRegion(SkRect::MakeXYWH(nX, nY, 1, 1));
     SkPaint paint;
     paint.setColor(toSkColor(nColor));
     // Apparently drawPixel() is actually expected to set the pixel and not draw it.
@@ -711,7 +712,7 @@ void SkiaSalGraphicsImpl::drawLine(long nX1, long nY1, long nX2, long nY2)
     preDraw();
     SAL_INFO("vcl.skia.trace", "drawline(" << this << "): " << Point(nX1, nY1) << "->"
                                            << Point(nX2, nY2) << ":" << mLineColor);
-    addXorRegion(SkRect::MakeLTRB(nX1, nY1, nX2, nY2).makeSorted());
+    addUpdateRegion(SkRect::MakeLTRB(nX1, nY1, nX2, nY2).makeSorted());
     SkPaint paint;
     paint.setColor(toSkColor(mLineColor));
     paint.setAntiAlias(mParent.getAntiAlias());
@@ -726,7 +727,7 @@ void SkiaSalGraphicsImpl::privateDrawAlphaRect(long nX, long nY, long nWidth, lo
     SAL_INFO("vcl.skia.trace",
              "privatedrawrect(" << this << "): " << SkIRect::MakeXYWH(nX, nY, nWidth, nHeight)
                                 << ":" << mLineColor << ":" << mFillColor << ":" << fTransparency);
-    addXorRegion(SkRect::MakeXYWH(nX, nY, nWidth, nHeight));
+    addUpdateRegion(SkRect::MakeXYWH(nX, nY, nWidth, nHeight));
     SkCanvas* canvas = getDrawCanvas();
     SkPaint paint;
     paint.setAntiAlias(!blockAA && mParent.getAntiAlias());
@@ -837,7 +838,7 @@ void SkiaSalGraphicsImpl::performDrawPolyPolygon(const basegfx::B2DPolyPolygon&
     SkPath polygonPath;
     addPolyPolygonToPath(aPolyPolygon, polygonPath);
     polygonPath.setFillType(SkPathFillType::kEvenOdd);
-    addXorRegion(polygonPath.getBounds());
+    addUpdateRegion(polygonPath.getBounds());
 
     SkPaint aPaint;
     aPaint.setAntiAlias(useAA);
@@ -1075,7 +1076,7 @@ bool SkiaSalGraphicsImpl::drawPolyLine(const basegfx::B2DHomMatrix& rObjectToDev
         for (sal_uInt32 a(0); a < aPolyPolygonLine.count(); a++)
             addPolygonToPath(aPolyPolygonLine.getB2DPolygon(a), aPath);
         aPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, nullptr);
-        addXorRegion(aPath.getBounds());
+        addUpdateRegion(aPath.getBounds());
         getDrawCanvas()->drawPath(aPath, aPaint);
     }
     else
@@ -1096,7 +1097,7 @@ bool SkiaSalGraphicsImpl::drawPolyLine(const basegfx::B2DHomMatrix& rObjectToDev
                              rPolygon.getB2DPoint(index2).getY());
 
                 aPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, nullptr);
-                addXorRegion(aPath.getBounds());
+                addUpdateRegion(aPath.getBounds());
                 getDrawCanvas()->drawPath(aPath, aPaint);
             }
         }
@@ -1162,6 +1163,7 @@ void SkiaSalGraphicsImpl::copyArea(long nDestX, long nDestY, long nSrcX, long nS
                                    << this << "): " << Point(nSrcX, nSrcY) << "->"
                                    << SkIRect::MakeXYWH(nDestX, nDestY, nSrcWidth, nSrcHeight));
     assert(!mXorMode);
+    addUpdateRegion(SkRect::MakeXYWH(nDestX, nDestY, nSrcWidth, nSrcHeight));
     ::copyArea(getDrawCanvas(), mSurface, nDestX, nDestY, nSrcX, nSrcY, nSrcWidth, nSrcHeight,
                !isGPU(), !isGPU());
     postDraw();
@@ -1183,6 +1185,9 @@ void SkiaSalGraphicsImpl::copyBits(const SalTwoRect& rPosAry, SalGraphics* pSrcG
         src = this;
         assert(!mXorMode);
     }
+    assert(!mXorMode);
+    addUpdateRegion(SkRect::MakeXYWH(rPosAry.mnDestX, rPosAry.mnDestY, rPosAry.mnDestWidth,
+                                     rPosAry.mnDestHeight));
     if (rPosAry.mnSrcWidth == rPosAry.mnDestWidth && rPosAry.mnSrcHeight == rPosAry.mnDestHeight)
     {
         auto srcDebug = [&]() -> std::string {
@@ -1218,7 +1223,6 @@ void SkiaSalGraphicsImpl::copyBits(const SalTwoRect& rPosAry, SalGraphics* pSrcG
                                                         rPosAry.mnDestWidth, rPosAry.mnDestHeight),
                                        &paint);
     }
-    assert(!mXorMode);
     postDraw();
 }
 
@@ -1362,12 +1366,13 @@ void SkiaSalGraphicsImpl::invert(basegfx::B2DPolygon const& rPoly, SalInvert eFl
     // and drawing using CPU.
     bool intelHack
         = (isGPU() && SkiaHelper::getVendor() == DriverBlocklist::VendorIntel && !mXorMode);
+    SkPath aPath;
+    addPolygonToPath(rPoly, aPath);
+    aPath.setFillType(SkPathFillType::kEvenOdd);
+    addUpdateRegion(aPath.getBounds());
     // TrackFrame just inverts a dashed path around the polygon
     if (eFlags == SalInvert::TrackFrame)
     {
-        SkPath aPath;
-        addPolygonToPath(rPoly, aPath);
-        aPath.setFillType(SkPathFillType::kEvenOdd);
         // TrackFrame is not supposed to paint outside of the polygon (usually rectangle),
         // but wider stroke width usually results in that, so ensure the requirement
         // by clipping.
@@ -1401,9 +1406,6 @@ void SkiaSalGraphicsImpl::invert(basegfx::B2DPolygon const& rPoly, SalInvert eFl
     }
     else
     {
-        SkPath aPath;
-        addPolygonToPath(rPoly, aPath);
-        aPath.setFillType(SkPathFillType::kEvenOdd);
         SkPaint aPaint;
         aPaint.setColor(SkColorSetARGB(255, 255, 255, 255));
         aPaint.setStyle(SkPaint::kFill_Style);
@@ -1633,7 +1635,7 @@ void SkiaSalGraphicsImpl::drawImage(const SalTwoRect& rPosAry, const sk_sp<SkIma
     preDraw();
     SAL_INFO("vcl.skia.trace",
              "drawimage(" << this << "): " << rPosAry << ":" << SkBlendMode_Name(eBlendMode));
-    addXorRegion(aDestinationRect);
+    addUpdateRegion(aDestinationRect);
     getDrawCanvas()->drawImageRect(aImage, aSourceRect, aDestinationRect, &aPaint);
     ++mPendingOperationsToFlush; // tdf#136369
     postDraw();
@@ -1648,7 +1650,7 @@ void SkiaSalGraphicsImpl::drawShader(const SalTwoRect& rPosAry, const sk_sp<SkSh
     SAL_INFO("vcl.skia.trace", "drawshader(" << this << "): " << rPosAry);
     SkRect destinationRect = SkRect::MakeXYWH(rPosAry.mnDestX, rPosAry.mnDestY, rPosAry.mnDestWidth,
                                               rPosAry.mnDestHeight);
-    addXorRegion(destinationRect);
+    addUpdateRegion(destinationRect);
     SkPaint paint;
     paint.setBlendMode(blendMode);
     paint.setShader(shader);
@@ -1697,7 +1699,7 @@ bool SkiaSalGraphicsImpl::drawTransformedBitmap(const basegfx::B2DPoint& rNull,
     SAL_INFO("vcl.skia.trace", "drawtransformedbitmap(" << this << "): " << rSourceBitmap.GetSize()
                                                         << " " << rNull << ":" << rX << ":" << rY);
 
-    addXorRegion(SkRect::MakeWH(GetWidth(), GetHeight())); // can't tell, use whole area
+    addUpdateRegion(SkRect::MakeWH(GetWidth(), GetHeight())); // can't tell, use whole area
     // In raster mode scaling and alpha blending is still somewhat expensive if done repeatedly,
     // so use mergeCacheBitmaps(), which will cache the result if useful.
     // It is better to use SkShader if in GPU mode, if the operation is simple or if the temporary
@@ -1795,7 +1797,7 @@ bool SkiaSalGraphicsImpl::drawGradient(const tools::PolyPolygon& rPolyPolygon,
     else
         addPolyPolygonToPath(rPolyPolygon.getB2DPolyPolygon(), path);
     path.setFillType(SkPathFillType::kEvenOdd);
-    addXorRegion(path.getBounds());
+    addUpdateRegion(path.getBounds());
 
     Gradient aGradient(rGradient);
     tools::Rectangle aBoundRect;
@@ -1848,6 +1850,7 @@ bool SkiaSalGraphicsImpl::implDrawGradient(const basegfx::B2DPolyPolygon& rPolyP
     SkPath path;
     addPolyPolygonToPath(rPolyPolygon, path);
     path.setFillType(SkPathFillType::kEvenOdd);
+    addUpdateRegion(path.getBounds());
 
     SkPoint points[2]
         = { SkPoint::Make(toSkX(rGradient.maPoint1.getX()), toSkY(rGradient.maPoint1.getY())),
@@ -1865,7 +1868,6 @@ bool SkiaSalGraphicsImpl::implDrawGradient(const basegfx::B2DPolyPolygon& rPolyP
     paint.setAntiAlias(mParent.getAntiAlias());
     paint.setShader(shader);
     getDrawCanvas()->drawPath(path, paint);
-    addXorRegion(path.getBounds());
     postDraw();
     return true;
 }
@@ -1906,7 +1908,7 @@ void SkiaSalGraphicsImpl::drawGenericLayout(const GenericSalLayout& layout, Colo
     preDraw();
     SAL_INFO("vcl.skia.trace",
              "drawtextblob(" << this << "): " << textBlob->bounds() << ":" << textColor);
-    addXorRegion(textBlob->bounds());
+    addUpdateRegion(textBlob->bounds());
     SkPaint paint;
     paint.setColor(toSkColor(textColor));
     getDrawCanvas()->drawTextBlob(textBlob, 0, 0, paint);
diff --git a/vcl/skia/win/gdiimpl.cxx b/vcl/skia/win/gdiimpl.cxx
index 819f024fa5cd..c3cb3a48b066 100644
--- a/vcl/skia/win/gdiimpl.cxx
+++ b/vcl/skia/win/gdiimpl.cxx
@@ -68,7 +68,11 @@ void WinSkiaSalGraphicsImpl::performFlush()
     SkiaZone zone;
     flushDrawing();
     if (mWindowContext)
-        mWindowContext->swapBuffers();
+    {
+        if (mDirtyRect.intersect(SkIRect::MakeWH(GetWidth(), GetHeight())))
+            mWindowContext->swapBuffers(&mDirtyRect);
+        mDirtyRect.setEmpty();
+    }
 }
 
 bool WinSkiaSalGraphicsImpl::TryRenderCachedNativeControl(ControlCacheKey const& rControlCacheKey,
diff --git a/vcl/skia/x11/gdiimpl.cxx b/vcl/skia/x11/gdiimpl.cxx
index d993cf61d8d7..29fb15d27140 100644
--- a/vcl/skia/x11/gdiimpl.cxx
+++ b/vcl/skia/x11/gdiimpl.cxx
@@ -141,7 +141,11 @@ void X11SkiaSalGraphicsImpl::performFlush()
     flushDrawing();
     // TODO XPutImage() is somewhat inefficient, XShmPutImage() should be preferred.
     if (mWindowContext)
-        mWindowContext->swapBuffers();
+    {
+        if (mDirtyRect.intersect(SkIRect::MakeWH(GetWidth(), GetHeight())))
+            mWindowContext->swapBuffers(&mDirtyRect);
+        mDirtyRect.setEmpty();
+    }
 }
 
 std::unique_ptr<sk_app::WindowContext> createVulkanWindowContext(bool temporary)