summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLuboš Luňák <l.lunak@collabora.com>2020-10-06 22:14:36 +0200
committerLuboš Luňák <l.lunak@collabora.com>2020-10-08 13:40:14 +0200
commitd18731f71c60cbb6c02cabb042004b1aa9454de8 (patch)
tree36870456fc84f62b2ab85a3bad1971641856cbda
parent47fda617fc4dad8273919227ca45ea3b8b61aea1 (diff)
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>
-rw-r--r--external/skia/UnpackedTarball_skia.mk1
-rw-r--r--external/skia/swap-buffers-rect.patch.1114
-rw-r--r--vcl/inc/skia/gdiimpl.hxx10
-rw-r--r--vcl/skia/gdiimpl.cxx40
-rw-r--r--vcl/skia/win/gdiimpl.cxx6
-rw-r--r--vcl/skia/x11/gdiimpl.cxx6
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)