From d18731f71c60cbb6c02cabb042004b1aa9454de8 Mon Sep 17 00:00:00 2001 From: Luboš Luňák Date: Tue, 6 Oct 2020 22:14:36 +0200 Subject: track dirty areas for Skia drawing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- external/skia/UnpackedTarball_skia.mk | 1 + external/skia/swap-buffers-rect.patch.1 | 114 ++++++++++++++++++++++++++++++++ vcl/inc/skia/gdiimpl.hxx | 10 ++- vcl/skia/gdiimpl.cxx | 40 +++++------ vcl/skia/win/gdiimpl.cxx | 6 +- vcl/skia/x11/gdiimpl.cxx | 6 +- 6 files changed, 153 insertions(+), 24 deletions(-) create mode 100644 external/skia/swap-buffers-rect.patch.1 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 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 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 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 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 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 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 RasterWindowContext_win::getBackbufferSurface() { return fBackbufferSurface; } + +-void RasterWindowContext_win::swapBuffers() { ++void RasterWindowContext_win::swapBuffers(const SkIRect* rect) { + BITMAPINFO* bmpInfo = reinterpret_cast(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 mergeCacheBitmaps(const SkiaSalBitmap& bitmap, const SkiaSalBitmap* alphaBitmap, @@ -318,6 +321,7 @@ protected: // The Skia surface that is target of all the rendering. sk_sp 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_spdrawImageRect(aImage, aSourceRect, aDestinationRect, &aPaint); ++mPendingOperationsToFlush; // tdf#136369 postDraw(); @@ -1648,7 +1650,7 @@ void SkiaSalGraphicsImpl::drawShader(const SalTwoRect& rPosAry, const sk_spdrawPath(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 createVulkanWindowContext(bool temporary) -- cgit v1.2.3