vcl/inc/skia/gdiimpl.hxx | 12 +++++++----- vcl/skia/gdiimpl.cxx | 28 ++++++++++++---------------- 2 files changed, 19 insertions(+), 21 deletions(-)
New commits: commit 468ef8e3cb8746bc072cf8608465fd93375f90c6 Author: Luboš Luňák <[email protected]> AuthorDate: Tue Jul 14 16:12:36 2020 +0200 Commit: Adolfo Jayme Barrientos <[email protected]> CommitDate: Thu Jul 16 13:51:27 2020 +0200 use consistent Skia pixel position adjustments (tdf#134346) What happens in tdf#134346 is that the same shape was drawn twice, once using drawPolyPolygon() and once as an outline using drawPolyLine(). Those had different offsets, so with AA enabled the tiny difference could be actually visible. Be more consistent with how the pixels are positioned in float coordinates. Change-Id: Id852fef70e7bd829ff0108a86d1ebee29c300e3a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/98745 Tested-by: Jenkins Reviewed-by: Luboš Luňák <[email protected]> (cherry picked from commit 4bb931a488b8fe7a0b4961956252f667b683a630) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/98834 Reviewed-by: Adolfo Jayme Barrientos <[email protected]> diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx index b8f4e5da3da1..b8a5a179d1d0 100644 --- a/vcl/inc/skia/gdiimpl.hxx +++ b/vcl/inc/skia/gdiimpl.hxx @@ -268,12 +268,14 @@ protected: sk_sp<SkImage> mergeCacheBitmaps(const SkiaSalBitmap& bitmap, const SkiaSalBitmap* alphaBitmap, const Size targetSize); - // When drawing using GPU, rounding errors may result in off-by-one errors, + // Skia uses floating point coordinates, so when we use integer coordinates, sometimes + // rounding results in off-by-one errors (down), especially when drawing using GPU, // see https://bugs.chromium.org/p/skia/issues/detail?id=9611 . Compensate for - // it by using centers of pixels (Skia uses float coordinates). In raster case - // it seems better to not do this though. - SkScalar toSkX(long x) const { return mIsGPU ? x + 0.5 : x; } - SkScalar toSkY(long y) const { return mIsGPU ? y + 0.5 : y; } + // it by using centers of pixels. Using 0.5 may sometimes round up, so go with 0.495 . + static constexpr SkScalar toSkX(long x) { return x + 0.495; } + static constexpr SkScalar toSkY(long y) { return y + 0.495; } + // Value to add to be exactly in the middle of the pixel. + static constexpr SkScalar toSkXYFix = SkScalar(0.005); template <typename charT, typename traits> friend inline std::basic_ostream<charT, traits>& diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx index 11b59d30d8ed..0dcb74c410b2 100644 --- a/vcl/skia/gdiimpl.cxx +++ b/vcl/skia/gdiimpl.cxx @@ -592,11 +592,7 @@ void SkiaSalGraphicsImpl::drawLine(long nX1, long nY1, long nX2, long nY2) SkPaint paint; paint.setColor(toSkColor(mLineColor)); paint.setAntiAlias(mParent.getAntiAliasB2DDraw()); - // Raster has better results if shifted by 0.25 (unlike the 0.5 done by toSkX/toSkY). - if (!isGPU()) - getDrawCanvas()->drawLine(nX1 + 0.25, nY1 + 0.25, nX2 + 0.25, nY2 + 0.25, paint); - else - getDrawCanvas()->drawLine(toSkX(nX1), toSkY(nY1), toSkX(nX2), toSkY(nY2), paint); + getDrawCanvas()->drawLine(toSkX(nX1), toSkY(nY1), toSkX(nX2), toSkY(nY2), paint); addXorRegion(SkRect::MakeLTRB(nX1, nY1, nX2 + 1, nY2 + 1)); postDraw(); } @@ -703,19 +699,21 @@ bool SkiaSalGraphicsImpl::drawPolyPolygon(const basegfx::B2DHomMatrix& rObjectTo SkPaint aPaint; aPaint.setAntiAlias(mParent.getAntiAliasB2DDraw()); + // We normally use pixel at their center positions, but slightly off (see toSkX/Y()). + // With AA lines that "slightly off" causes tiny changes of color, making some tests + // fail. Since moving AA-ed line slightly to a side doesn't cause any real visual + // difference, just place exactly at the center. tdf#134346 + const SkScalar posFix = mParent.getAntiAliasB2DDraw() ? toSkXYFix : 0; if (mFillColor != SALCOLOR_NONE) { aPaint.setColor(toSkColorWithTransparency(mFillColor, fTransparency)); aPaint.setStyle(SkPaint::kFill_Style); + aPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, nullptr); getDrawCanvas()->drawPath(aPath, aPaint); } if (mLineColor != SALCOLOR_NONE) { - // Raster has better results if shifted by 0.25 (unlike the 0.5 done by toSkX/toSkY). - if (!isGPU()) - aPath.offset(0.25, 0.25, nullptr); - else // Apply the same adjustment as toSkX()/toSkY() do. - aPath.offset(0.5, 0.5, nullptr); + aPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, nullptr); aPaint.setColor(toSkColorWithTransparency(mLineColor, fTransparency)); aPaint.setStyle(SkPaint::kStroke_Style); getDrawCanvas()->drawPath(aPath, aPaint); @@ -806,6 +804,8 @@ bool SkiaSalGraphicsImpl::drawPolyLine(const basegfx::B2DHomMatrix& rObjectToDev aPaint.setStrokeMiter(fMiterLimit); aPaint.setStrokeWidth(fLineWidth); aPaint.setAntiAlias(mParent.getAntiAliasB2DDraw()); + // See the tdf#134346 comment above. + const SkScalar posFix = mParent.getAntiAliasB2DDraw() ? toSkXYFix : 0; if (pStroke && std::accumulate(pStroke->begin(), pStroke->end(), 0.0) != 0) { @@ -824,9 +824,7 @@ bool SkiaSalGraphicsImpl::drawPolyLine(const basegfx::B2DHomMatrix& rObjectToDev aPath.setFillType(SkPathFillType::kEvenOdd); for (sal_uInt32 a(0); a < aPolyPolygonLine.count(); a++) addPolygonToPath(aPolyPolygonLine.getB2DPolygon(a), aPath); - // Apply the same adjustment as toSkX()/toSkY() do. Do it here even in the non-GPU - // case as it seems to produce better results. - aPath.offset(0.5, 0.5, nullptr); + aPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, nullptr); getDrawCanvas()->drawPath(aPath, aPaint); addXorRegion(aPath.getBounds()); } @@ -847,9 +845,7 @@ bool SkiaSalGraphicsImpl::drawPolyLine(const basegfx::B2DHomMatrix& rObjectToDev aPath.lineTo(rPolygon.getB2DPoint(index2).getX(), rPolygon.getB2DPoint(index2).getY()); - // Apply the same adjustment as toSkX()/toSkY() do. Do it here even in the non-GPU - // case as it seems to produce better results. - aPath.offset(0.5, 0.5, nullptr); + aPath.offset(toSkX(0) + posFix, toSkY(0) + posFix, nullptr); getDrawCanvas()->drawPath(aPath, aPaint); addXorRegion(aPath.getBounds()); } _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
