[Libreoffice-commits] core.git: Branch 'libreoffice-7-0' - vcl/backendtest vcl/inc vcl/skia
Luboš Luňák (via logerrit)
logerrit at kemper.freedesktop.org
Fri Sep 25 18:51:00 UTC 2020
vcl/backendtest/outputdevice/outputdevice.cxx | 18 +++++++++-
vcl/inc/skia/gdiimpl.hxx | 9 ++++-
vcl/skia/gdiimpl.cxx | 45 ++++++++++++--------------
3 files changed, 47 insertions(+), 25 deletions(-)
New commits:
commit 540eea58881ca17eff012a61b8b953d20b596b21
Author: Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Wed Sep 23 12:13:32 2020 +0200
Commit: Caolán McNamara <caolanm at redhat.com>
CommitDate: Fri Sep 25 20:50:27 2020 +0200
xor drawing done twice in the same place should be a no-op
This extends the VCL backend unittest to check for this, and also
fixes Skia to handle that properly.
This makes tdf#132241 slow again. The problem there is that it
does drawGradient() with xor enabled (for whatever strange reason),
and since Skia does not implement drawGradient(), it gets drawn
using polygons and their bounds overlap, causing applyXor() after
each operation again. Implementing drawGradient() will handle that.
Change-Id: Ibea433ad95f8c6d53049f4a49295e57a5aec184f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103280
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak at collabora.com>
(cherry picked from commit 787bc48c883b70b1721805b5d6a93bd731983410)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103337
Reviewed-by: Caolán McNamara <caolanm at redhat.com>
diff --git a/vcl/backendtest/outputdevice/outputdevice.cxx b/vcl/backendtest/outputdevice/outputdevice.cxx
index c05c03e06a3a..07d66ffb413c 100644
--- a/vcl/backendtest/outputdevice/outputdevice.cxx
+++ b/vcl/backendtest/outputdevice/outputdevice.cxx
@@ -49,6 +49,19 @@ Bitmap OutputDeviceTestAnotherOutDev::setupXOR()
mpVirtualDevice->SetFillColor(constFillColor);
mpVirtualDevice->DrawRect(aDrawRectangle);
+ mpVirtualDevice->SetRasterOp(RasterOp::Xor);
+ mpVirtualDevice->SetLineColor(constFillColor);
+ mpVirtualDevice->SetFillColor();
+ // Rectangle drawn twice is a no-op.
+ aDrawRectangle = maVDRectangle;
+ mpVirtualDevice->DrawRect(aDrawRectangle);
+ mpVirtualDevice->DrawRect(aDrawRectangle);
+ // Rectangle drawn three times is like drawing once.
+ aDrawRectangle.shrink(1);
+ mpVirtualDevice->DrawRect(aDrawRectangle);
+ mpVirtualDevice->DrawRect(aDrawRectangle);
+ mpVirtualDevice->DrawRect(aDrawRectangle);
+
return mpVirtualDevice->GetBitmap(maVDRectangle.TopLeft(), maVDRectangle.GetSize());
}
@@ -64,9 +77,12 @@ TestResult OutputDeviceTestAnotherOutDev::checkDrawOutDev(Bitmap& rBitmap)
TestResult OutputDeviceTestAnotherOutDev::checkXOR(Bitmap& rBitmap)
{
+ Color xorColor( constBackgroundColor.GetRed() ^ constFillColor.GetRed(),
+ constBackgroundColor.GetGreen() ^ constFillColor.GetGreen(),
+ constBackgroundColor.GetBlue() ^ constFillColor.GetBlue());
std::vector<Color> aExpected
{
- constBackgroundColor, constBackgroundColor,
+ constBackgroundColor, xorColor,
constBackgroundColor, constBackgroundColor,
constFillColor, constFillColor,
constFillColor
diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx
index 5b72e4b4e249..e84c251cdde1 100644
--- a/vcl/inc/skia/gdiimpl.hxx
+++ b/vcl/inc/skia/gdiimpl.hxx
@@ -257,12 +257,19 @@ protected:
SkCanvas* getXorCanvas();
void applyXor();
+ // NOTE: This must be called before the operation does any drawing.
void addXorRegion(const SkRect& rect)
{
if (mXorMode)
{
// Make slightly larger, just in case (rounding, antialiasing,...).
- mXorRegion.op(rect.makeOutset(2, 2).round(), SkRegion::kUnion_Op);
+ 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.
+ if (mXorRegion.intersects(addedRect))
+ applyXor();
+ mXorRegion.op(addedRect, SkRegion::kUnion_Op);
}
}
static void setCanvasClipRegion(SkCanvas* canvas, const vcl::Region& region);
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index 743eea5d891a..2f308ad8aba8 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -559,7 +559,7 @@ void SkiaSalGraphicsImpl::applyXor()
// in each operation by extending mXorRegion with the area that should be
// updated.
assert(mXorMode);
- if (!mSurface
+ if (!mSurface || !mXorCanvas
|| !mXorRegion.op(SkIRect::MakeXYWH(0, 0, mSurface->width(), mSurface->height()),
SkRegion::kIntersect_Op))
{
@@ -650,12 +650,12 @@ 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));
SkPaint paint;
paint.setColor(toSkColor(nColor));
// Apparently drawPixel() is actually expected to set the pixel and not draw it.
paint.setBlendMode(SkBlendMode::kSrc); // set as is, including alpha
getDrawCanvas()->drawPoint(toSkX(nX), toSkY(nY), paint);
- addXorRegion(SkRect::MakeXYWH(nX, nY, 1, 1));
postDraw();
}
@@ -666,11 +666,11 @@ 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());
SkPaint paint;
paint.setColor(toSkColor(mLineColor));
paint.setAntiAlias(mParent.getAntiAliasB2DDraw());
getDrawCanvas()->drawLine(toSkX(nX1), toSkY(nY1), toSkX(nX2), toSkY(nY2), paint);
- addXorRegion(SkRect::MakeLTRB(nX1, nY1, nX2 + 1, nY2 + 1));
postDraw();
}
@@ -681,6 +681,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));
SkCanvas* canvas = getDrawCanvas();
SkPaint paint;
paint.setAntiAlias(!blockAA && mParent.getAntiAliasB2DDraw());
@@ -704,7 +705,6 @@ void SkiaSalGraphicsImpl::privateDrawAlphaRect(long nX, long nY, long nWidth, lo
canvas->drawIRect(
SkIRect::MakeXYWH(nX, nY, std::max(1L, nWidth - 1), std::max(1L, nHeight - 1)), paint);
}
- addXorRegion(SkRect::MakeXYWH(nX, nY, nWidth, nHeight));
postDraw();
}
@@ -789,9 +789,10 @@ void SkiaSalGraphicsImpl::performDrawPolyPolygon(const basegfx::B2DPolyPolygon&
{
preDraw();
- SkPath aPath;
- addPolyPolygonToPath(aPolyPolygon, aPath);
- aPath.setFillType(SkPathFillType::kEvenOdd);
+ SkPath polygonPath;
+ addPolyPolygonToPath(aPolyPolygon, polygonPath);
+ polygonPath.setFillType(SkPathFillType::kEvenOdd);
+ addXorRegion(polygonPath.getBounds());
SkPaint aPaint;
aPaint.setAntiAlias(useAA);
@@ -802,23 +803,24 @@ void SkiaSalGraphicsImpl::performDrawPolyPolygon(const basegfx::B2DPolyPolygon&
const SkScalar posFix = useAA ? toSkXYFix : 0;
if (mFillColor != SALCOLOR_NONE)
{
- aPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, nullptr);
+ SkPath path;
+ polygonPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, &path);
aPaint.setColor(toSkColorWithTransparency(mFillColor, fTransparency));
aPaint.setStyle(SkPaint::kFill_Style);
// HACK: If the polygon is just a line, it still should be drawn. But when filling
// Skia doesn't draw empty polygons, so in that case ensure the line is drawn.
- if (mLineColor == SALCOLOR_NONE && aPath.getBounds().isEmpty())
+ if (mLineColor == SALCOLOR_NONE && path.getBounds().isEmpty())
aPaint.setStyle(SkPaint::kStroke_Style);
- getDrawCanvas()->drawPath(aPath, aPaint);
+ getDrawCanvas()->drawPath(path, aPaint);
}
if (mLineColor != SALCOLOR_NONE)
{
- aPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, nullptr);
+ SkPath path;
+ polygonPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, &path);
aPaint.setColor(toSkColorWithTransparency(mLineColor, fTransparency));
aPaint.setStyle(SkPaint::kStroke_Style);
- getDrawCanvas()->drawPath(aPath, aPaint);
+ getDrawCanvas()->drawPath(path, aPaint);
}
- addXorRegion(aPath.getBounds());
postDraw();
#if defined LINUX
// WORKAROUND: The logo in the about dialog has drawing errors. This seems to happen
@@ -1028,8 +1030,8 @@ 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);
- getDrawCanvas()->drawPath(aPath, aPaint);
addXorRegion(aPath.getBounds());
+ getDrawCanvas()->drawPath(aPath, aPaint);
}
else
{
@@ -1049,8 +1051,8 @@ bool SkiaSalGraphicsImpl::drawPolyLine(const basegfx::B2DHomMatrix& rObjectToDev
rPolygon.getB2DPoint(index2).getY());
aPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, nullptr);
- getDrawCanvas()->drawPath(aPath, aPaint);
addXorRegion(aPath.getBounds());
+ getDrawCanvas()->drawPath(aPath, aPaint);
}
}
}
@@ -1120,7 +1122,6 @@ void SkiaSalGraphicsImpl::copyArea(long nDestX, long nDestY, long nSrcX, long nS
assert(!mXorMode);
::copyArea(getDrawCanvas(), mSurface, nDestX, nDestY, nSrcX, nSrcY, nSrcWidth, nSrcHeight,
!isGPU(), !isGPU());
- addXorRegion(SkRect::MakeXYWH(nDestX, nDestY, nSrcWidth, nSrcHeight));
postDraw();
}
@@ -1175,8 +1176,7 @@ void SkiaSalGraphicsImpl::copyBits(const SalTwoRect& rPosAry, SalGraphics* pSrcG
rPosAry.mnDestWidth, rPosAry.mnDestHeight),
&paint);
}
- addXorRegion(SkRect::MakeXYWH(rPosAry.mnDestX, rPosAry.mnDestY, rPosAry.mnDestWidth,
- rPosAry.mnDestHeight));
+ assert(!mXorMode);
postDraw();
}
@@ -1392,7 +1392,6 @@ void SkiaSalGraphicsImpl::invert(basegfx::B2DPolygon const& rPoly, SalInvert eFl
getDrawCanvas()->drawImageRect(SkiaHelper::makeCheckedImageSnapshot(surface), size,
area, ©);
}
- addXorRegion(aPath.getBounds());
}
postDraw();
}
@@ -1588,8 +1587,8 @@ void SkiaSalGraphicsImpl::drawImage(const SalTwoRect& rPosAry, const sk_sp<SkIma
preDraw();
SAL_INFO("vcl.skia.trace",
"drawimage(" << this << "): " << rPosAry << ":" << SkBlendMode_Name(eBlendMode));
- getDrawCanvas()->drawImageRect(aImage, aSourceRect, aDestinationRect, &aPaint);
addXorRegion(aDestinationRect);
+ getDrawCanvas()->drawImageRect(aImage, aSourceRect, aDestinationRect, &aPaint);
postDraw();
}
@@ -1601,6 +1600,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);
SkPaint paint;
paint.setShader(shader);
if (rPosAry.mnSrcWidth != rPosAry.mnDestWidth || rPosAry.mnSrcHeight != rPosAry.mnDestHeight)
@@ -1617,7 +1617,6 @@ void SkiaSalGraphicsImpl::drawShader(const SalTwoRect& rPosAry, const sk_sp<SkSh
matrix.set(SkMatrix::kMTransY, rPosAry.mnDestY - rPosAry.mnSrcY);
canvas->concat(matrix);
canvas->drawPaint(paint);
- addXorRegion(destinationRect);
postDraw();
}
@@ -1642,6 +1641,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
// 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
@@ -1697,7 +1697,6 @@ bool SkiaSalGraphicsImpl::drawTransformedBitmap(const basegfx::B2DPoint& rNull,
canvas->drawImage(rSkiaBitmap.GetSkImage(), 0, 0, &paint);
}
}
- addXorRegion(SkRect::MakeWH(GetWidth(), GetHeight())); // can't tell, use whole area
postDraw();
return true;
}
@@ -1751,10 +1750,10 @@ void SkiaSalGraphicsImpl::drawGenericLayout(const GenericSalLayout& layout, Colo
preDraw();
SAL_INFO("vcl.skia.trace",
"drawtextblob(" << this << "): " << textBlob->bounds() << ":" << textColor);
+ addXorRegion(textBlob->bounds());
SkPaint paint;
paint.setColor(toSkColor(textColor));
getDrawCanvas()->drawTextBlob(textBlob, 0, 0, paint);
- addXorRegion(textBlob->bounds());
postDraw();
}
More information about the Libreoffice-commits
mailing list