avoid deep copies of pixels with Skia raster surfaces (tdf#132856)
SkCanvas::draw() leads to deep copies of the source bitmap, moreover
RasterWindowContext_win allocates the pixels in a way that
SkSurface_Raster has to do a deep copy because of not owning
the pixels.
Change-Id: I22f6a2c0f96faf99f94140eff26ec0c22fae96d9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/93958
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>
diff --git a/external/skia/UnpackedTarball_skia.mk b/external/skia/UnpackedTarball_skia.mk
index e0841e4..08769b2 100644
--- a/external/skia/UnpackedTarball_skia.mk
+++ b/external/skia/UnpackedTarball_skia.mk
@@ -34,6 +34,7 @@ skia_patches := \
fix-without-gl.patch.0 \
extend-rgb-to-rgba.patch.0 \
windows-typeface-directwrite.patch.0 \
windows-raster-surface-no-copies.patch.1 \
$(eval $(call gb_UnpackedTarball_set_patchlevel,skia,1))
diff --git a/external/skia/windows-raster-surface-no-copies.patch.1 b/external/skia/windows-raster-surface-no-copies.patch.1
new file mode 100644
index 0000000..0c5804d
--- /dev/null
+++ b/external/skia/windows-raster-surface-no-copies.patch.1
@@ -0,0 +1,39 @@
diff --git a/tools/sk_app/win/RasterWindowContext_win.cpp b/tools/sk_app/win/RasterWindowContext_win.cpp
index 9548220ce6..49f1f9ed17 100644
--- a/tools/sk_app/win/RasterWindowContext_win.cpp
+++ b/tools/sk_app/win/RasterWindowContext_win.cpp
@@ -55,7 +55,7 @@ void RasterWindowContext_win::resize(int w, int h) {
fWidth = w;
fHeight = h;
fBackbufferSurface.reset();
- const size_t bmpSize = sizeof(BITMAPINFOHEADER) + w * h * sizeof(uint32_t);
+ const size_t bmpSize = sizeof(BITMAPINFO);
fSurfaceMemory.reset(bmpSize);
BITMAPINFO* bmpInfo = reinterpret_cast<BITMAPINFO*>(fSurfaceMemory.get());
ZeroMemory(bmpInfo, sizeof(BITMAPINFO));
@@ -65,11 +65,12 @@ void RasterWindowContext_win::resize(int w, int h) {
bmpInfo->bmiHeader.biPlanes = 1;
bmpInfo->bmiHeader.biBitCount = 32;
bmpInfo->bmiHeader.biCompression = BI_RGB;
- void* pixels = bmpInfo->bmiColors;
+ // Do not use a packed DIB bitmap, SkSurface_Raster::onNewImageSnapshot() does
+ // a deep copy if it does not own the pixels.
SkImageInfo info = SkImageInfo::Make(w, h, fDisplayParams.fColorType, kPremul_SkAlphaType,
fDisplayParams.fColorSpace);
- fBackbufferSurface = SkSurface::MakeRasterDirect(info, pixels, sizeof(uint32_t) * w);
+ fBackbufferSurface = SkSurface::MakeRaster(info);
}
sk_sp<SkSurface> RasterWindowContext_win::getBackbufferSurface() { return fBackbufferSurface; }
@@ -77,7 +78,9 @@ sk_sp<SkSurface> RasterWindowContext_win::getBackbufferSurface() { return fBackb
void RasterWindowContext_win::swapBuffers() {
BITMAPINFO* bmpInfo = reinterpret_cast<BITMAPINFO*>(fSurfaceMemory.get());
HDC dc = GetDC(fWnd);
- StretchDIBits(dc, 0, 0, fWidth, fHeight, 0, 0, fWidth, fHeight, bmpInfo->bmiColors, bmpInfo,
+ SkPixmap pixmap;
+ fBackbufferSurface->peekPixels(&pixmap);
+ StretchDIBits(dc, 0, 0, fWidth, fHeight, 0, 0, fWidth, fHeight, pixmap.addr(), bmpInfo,
DIB_RGB_COLORS, SRCCOPY);
ReleaseDC(fWnd, dc);
}
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index 94de013..9fc3b50 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -902,14 +902,16 @@ bool SkiaSalGraphicsImpl::drawPolyPolygonBezier(sal_uInt32, const sal_uInt32*,
}
static void copyArea(SkCanvas* canvas, sk_sp<SkSurface> surface, long nDestX, long nDestY,
long nSrcX, long nSrcY, long nSrcWidth, long nSrcHeight)
long nSrcX, long nSrcY, long nSrcWidth, long nSrcHeight, bool srcIsRaster)
{
// Using SkSurface::draw() should be more efficient than SkSurface::makeImageSnapshot(),
// because it may detect copying to itself and avoid some needless copies.
// It cannot do a subrectangle though, so clip.
if (canvas == surface->getCanvas())
// But it has problems with drawing to iself
// (https://groups.google.com/forum/#!topic/skia-discuss/6yiuw24jv0I) and also
// raster surfaces do not avoid a copy of the source
// (https://groups.google.com/forum/#!topic/skia-discuss/S3FMpCi82k0).
if (canvas == surface->getCanvas() || srcIsRaster)
{
// TODO: Currently copy-to-self is buggy with SkSurface::draw().
SkPaint paint;
paint.setBlendMode(SkBlendMode::kSrc); // copy as is, including alpha
canvas->drawImageRect(surface->makeImageSnapshot(),
@@ -917,6 +919,7 @@ static void copyArea(SkCanvas* canvas, sk_sp<SkSurface> surface, long nDestX, lo
SkRect::MakeXYWH(nDestX, nDestY, nSrcWidth, nSrcHeight), &paint);
return;
}
// SkCanvas::draw() cannot do a subrectangle, so clip.
canvas->save();
canvas->clipRect(SkRect::MakeXYWH(nDestX, nDestY, nSrcWidth, nSrcHeight));
SkPaint paint;
@@ -935,7 +938,8 @@ void SkiaSalGraphicsImpl::copyArea(long nDestX, long nDestY, long nSrcX, long nS
<< this << "): " << Point(nSrcX, nSrcY) << "->"
<< SkIRect::MakeXYWH(nDestX, nDestY, nSrcWidth, nSrcHeight));
assert(!mXorMode);
::copyArea(getDrawCanvas(), mSurface, nDestX, nDestY, nSrcX, nSrcY, nSrcWidth, nSrcHeight);
::copyArea(getDrawCanvas(), mSurface, nDestX, nDestY, nSrcX, nSrcY, nSrcWidth, nSrcHeight,
!isGPU());
addXorRegion(SkRect::MakeXYWH(nDestX, nDestY, nSrcWidth, nSrcHeight));
postDraw();
}
@@ -971,7 +975,7 @@ void SkiaSalGraphicsImpl::copyBits(const SalTwoRect& rPosAry, SalGraphics* pSrcG
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);
rPosAry.mnSrcY, rPosAry.mnDestWidth, rPosAry.mnDestHeight, !src->isGPU());
}
else
{
