diff options
| -rw-r--r-- | basegfx/CppunitTest_basegfx.mk | 1 | ||||
| -rw-r--r-- | basegfx/test/B2DPolyPolygonCutterTest.cxx | 103 | ||||
| -rw-r--r-- | vcl/inc/skia/gdiimpl.hxx | 16 | ||||
| -rw-r--r-- | vcl/skia/gdiimpl.cxx | 129 |
4 files changed, 238 insertions, 11 deletions
diff --git a/basegfx/CppunitTest_basegfx.mk b/basegfx/CppunitTest_basegfx.mk index 91cfc2226774..231854cdf057 100644 --- a/basegfx/CppunitTest_basegfx.mk +++ b/basegfx/CppunitTest_basegfx.mk @@ -29,6 +29,7 @@ $(eval $(call gb_CppunitTest_add_exception_objects,basegfx,\ basegfx/test/B2DPolygonTest \ basegfx/test/B2DPolygonToolsTest \ basegfx/test/B2DPolyPolygonTest \ + basegfx/test/B2DPolyPolygonCutterTest \ basegfx/test/B1DRangeTest \ basegfx/test/B2XRangeTest \ basegfx/test/B2IBoxTest \ diff --git a/basegfx/test/B2DPolyPolygonCutterTest.cxx b/basegfx/test/B2DPolyPolygonCutterTest.cxx new file mode 100644 index 000000000000..b111bf469d5e --- /dev/null +++ b/basegfx/test/B2DPolyPolygonCutterTest.cxx @@ -0,0 +1,103 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + * + * This file incorporates work covered by the following license notice: + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed + * with this work for additional information regarding copyright + * ownership. The ASF licenses this file to you under the Apache + * License, Version 2.0 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a copy of + * the License at http://www.apache.org/licenses/LICENSE-2.0 . + */ + +#include <cppunit/TestAssert.h> +#include <cppunit/TestFixture.h> +#include <cppunit/extensions/HelperMacros.h> + +#include <basegfx/polygon/b2dpolypolygoncutter.hxx> + +namespace basegfx +{ +class b2dpolypolygoncutter : public CppUnit::TestFixture +{ +public: + void testMergeToSinglePolyPolygon() + { + { // Adjacent polygons merged to one, closed manually. + B2DPolygon poly1{ { 0, 0 }, { 1, 0 }, { 1, 1 }, { 0, 1 }, { 0, 0 } }; + B2DPolygon poly2{ { 0, 1 }, { 1, 1 }, { 1, 2 }, { 0, 2 }, { 0, 1 } }; + B2DPolyPolygon expected( + B2DPolygon{ { 1, 0 }, { 1, 1 }, { 1, 2 }, { 0, 2 }, { 0, 1 }, { 0, 0 } }); + expected.setClosed(true); + B2DPolyPolygon result + = utils::mergeToSinglePolyPolygon({ B2DPolyPolygon(poly1), B2DPolyPolygon(poly2) }); + CPPUNIT_ASSERT_EQUAL(expected, result); + } + + { // Adjacent polygons merged to one, closed using setClosed(). + B2DPolygon poly1{ { 0, 0 }, { 1, 0 }, { 1, 1 }, { 0, 1 } }; + B2DPolygon poly2{ { 0, 1 }, { 1, 1 }, { 1, 2 }, { 0, 2 } }; + poly1.setClosed(true); + poly2.setClosed(true); + B2DPolyPolygon expected( + B2DPolygon{ { 0, 0 }, { 1, 0 }, { 1, 1 }, { 1, 2 }, { 0, 2 }, { 0, 1 } }); + expected.setClosed(true); + B2DPolyPolygon result + = utils::mergeToSinglePolyPolygon({ B2DPolyPolygon(poly1), B2DPolyPolygon(poly2) }); + CPPUNIT_ASSERT_EQUAL(expected, result); + } + + { // Non-adjacent polygons, no merge. + B2DPolygon poly1{ { 0, 0 }, { 1, 0 }, { 1, 1 }, { 0, 1 } }; + B2DPolygon poly2{ { 0, 2 }, { 1, 3 }, { 1, 3 }, { 0, 3 } }; + poly1.setClosed(true); + poly2.setClosed(true); + B2DPolyPolygon expected; + expected.append(poly1); + expected.append(poly2); + B2DPolyPolygon result + = utils::mergeToSinglePolyPolygon({ B2DPolyPolygon(poly1), B2DPolyPolygon(poly2) }); + CPPUNIT_ASSERT_EQUAL(expected, result); + } + + { // Horizontal and vertical rectangle that together form a cross. + B2DPolygon poly1{ { 1, 0 }, { 2, 0 }, { 2, 3 }, { 1, 3 } }; + B2DPolygon poly2{ { 0, 1 }, { 3, 1 }, { 3, 2 }, { 0, 2 } }; + poly1.setClosed(true); + poly2.setClosed(true); + B2DPolyPolygon expected(B2DPolygon{ { 1, 0 }, + { 2, 0 }, + { 2, 1 }, + { 3, 1 }, + { 3, 2 }, + { 2, 2 }, + { 2, 3 }, + { 1, 3 }, + { 1, 2 }, + { 0, 2 }, + { 0, 1 }, + { 1, 1 } }); + expected.setClosed(true); + B2DPolyPolygon result + = utils::mergeToSinglePolyPolygon({ B2DPolyPolygon(poly1), B2DPolyPolygon(poly2) }); + CPPUNIT_ASSERT_EQUAL(expected, result); + } + } + + CPPUNIT_TEST_SUITE(b2dpolypolygoncutter); + CPPUNIT_TEST(testMergeToSinglePolyPolygon); + CPPUNIT_TEST_SUITE_END(); +}; + +} // namespace basegfx + +CPPUNIT_TEST_SUITE_REGISTRATION(basegfx::b2dpolypolygoncutter); + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx index b8a5a179d1d0..ab12fd1b521d 100644 --- a/vcl/inc/skia/gdiimpl.hxx +++ b/vcl/inc/skia/gdiimpl.hxx @@ -247,6 +247,7 @@ protected: // Called by SkiaFlushIdle. virtual void performFlush() = 0; + void scheduleFlush(); friend class SkiaFlushIdle; // get the width of the device @@ -277,6 +278,14 @@ protected: // Value to add to be exactly in the middle of the pixel. static constexpr SkScalar toSkXYFix = SkScalar(0.005); + // Perform any pending drawing such as delayed merging of polygons. Called by preDraw() + // and anything that means the next operation cannot be another one in a series (e.g. + // changing colors). + void checkPendingDrawing(); + bool mergePolyPolygonToPrevious(const basegfx::B2DPolyPolygon& polygon, double transparency); + void performDrawPolyPolygon(const basegfx::B2DPolyPolygon& polygon, double transparency, + bool useAA); + template <typename charT, typename traits> friend inline std::basic_ostream<charT, traits>& operator<<(std::basic_ostream<charT, traits>& stream, const SkiaSalGraphicsImpl* graphics) @@ -302,6 +311,13 @@ protected: std::unique_ptr<SkCanvas> mXorCanvas; SkRegion mXorRegion; // the area that needs updating for the xor operation std::unique_ptr<SkiaFlushIdle> mFlush; + // Info about pending polygons to draw (we try to merge adjacent polygons into one). + struct LastPolyPolygonInfo + { + basegfx::B2DPolyPolygon polygon; + double transparency; + }; + LastPolyPolygonInfo mLastPolyPolygonInfo; }; #endif diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx index d9c5ba487237..980f0c688651 100644 --- a/vcl/skia/gdiimpl.cxx +++ b/vcl/skia/gdiimpl.cxx @@ -39,6 +39,7 @@ #include <numeric> #include <basegfx/polygon/b2dpolygontools.hxx> #include <basegfx/polygon/b2dpolypolygontools.hxx> +#include <basegfx/polygon/b2dpolypolygoncutter.hxx> namespace { @@ -326,10 +327,17 @@ void SkiaSalGraphicsImpl::preDraw() assert(comphelper::SolarMutex::get()->IsCurrentThread()); SkiaZone::enter(); // matched in postDraw() checkSurface(); + checkPendingDrawing(); } void SkiaSalGraphicsImpl::postDraw() { + scheduleFlush(); + SkiaZone::leave(); // matched in preDraw() +} + +void SkiaSalGraphicsImpl::scheduleFlush() +{ if (!isOffscreen()) { if (!Application::IsInExecute()) @@ -337,7 +345,6 @@ void SkiaSalGraphicsImpl::postDraw() else if (!mFlush->IsActive()) mFlush->Start(); } - SkiaZone::leave(); // matched in preDraw() } // VCL can sometimes resize us without telling us, update the surface if needed. @@ -385,6 +392,7 @@ void SkiaSalGraphicsImpl::checkSurface() void SkiaSalGraphicsImpl::flushDrawing() { + checkPendingDrawing(); if (mXorMode) applyXor(); mSurface->flush(); @@ -395,6 +403,7 @@ bool SkiaSalGraphicsImpl::setClipRegion(const vcl::Region& region) if (mClipRegion == region) return true; SkiaZone zone; + checkPendingDrawing(); mClipRegion = region; checkSurface(); SAL_INFO("vcl.skia.trace", "setclipregion(" << this << "): " << region); @@ -438,18 +447,35 @@ sal_uInt16 SkiaSalGraphicsImpl::GetBitCount() const { return 32; } long SkiaSalGraphicsImpl::GetGraphicsWidth() const { return GetWidth(); } -void SkiaSalGraphicsImpl::SetLineColor() { mLineColor = SALCOLOR_NONE; } +void SkiaSalGraphicsImpl::SetLineColor() +{ + checkPendingDrawing(); + mLineColor = SALCOLOR_NONE; +} -void SkiaSalGraphicsImpl::SetLineColor(Color nColor) { mLineColor = nColor; } +void SkiaSalGraphicsImpl::SetLineColor(Color nColor) +{ + checkPendingDrawing(); + mLineColor = nColor; +} -void SkiaSalGraphicsImpl::SetFillColor() { mFillColor = SALCOLOR_NONE; } +void SkiaSalGraphicsImpl::SetFillColor() +{ + checkPendingDrawing(); + mFillColor = SALCOLOR_NONE; +} -void SkiaSalGraphicsImpl::SetFillColor(Color nColor) { mFillColor = nColor; } +void SkiaSalGraphicsImpl::SetFillColor(Color nColor) +{ + checkPendingDrawing(); + mFillColor = nColor; +} void SkiaSalGraphicsImpl::SetXORMode(bool set, bool) { if (mXorMode == set) return; + checkPendingDrawing(); SAL_INFO("vcl.skia.trace", "setxormode(" << this << "): " << set); if (set) mXorRegion.setEmpty(); @@ -535,6 +561,7 @@ void SkiaSalGraphicsImpl::applyXor() void SkiaSalGraphicsImpl::SetROPLineColor(SalROPColor nROPColor) { + checkPendingDrawing(); switch (nROPColor) { case SalROPColor::N0: @@ -551,6 +578,7 @@ void SkiaSalGraphicsImpl::SetROPLineColor(SalROPColor nROPColor) void SkiaSalGraphicsImpl::SetROPFillColor(SalROPColor nROPColor) { + checkPendingDrawing(); switch (nROPColor) { case SalROPColor::N0: @@ -687,23 +715,38 @@ bool SkiaSalGraphicsImpl::drawPolyPolygon(const basegfx::B2DHomMatrix& rObjectTo || fTransparency >= 1.0) return true; - preDraw(); - - SkPath aPath; basegfx::B2DPolyPolygon aPolyPolygon(rPolyPolygon); aPolyPolygon.transform(rObjectToDevice); + SAL_INFO("vcl.skia.trace", "drawpolypolygon(" << this << "): " << aPolyPolygon << ":" << mLineColor << ":" << mFillColor); + + if (mergePolyPolygonToPrevious(aPolyPolygon, fTransparency)) + { + scheduleFlush(); + return true; + } + + performDrawPolyPolygon(aPolyPolygon, fTransparency, mParent.getAntiAliasB2DDraw()); + return true; +} + +void SkiaSalGraphicsImpl::performDrawPolyPolygon(const basegfx::B2DPolyPolygon& aPolyPolygon, + double fTransparency, bool useAA) +{ + preDraw(); + + SkPath aPath; addPolyPolygonToPath(aPolyPolygon, aPath); aPath.setFillType(SkPathFillType::kEvenOdd); SkPaint aPaint; - aPaint.setAntiAlias(mParent.getAntiAliasB2DDraw()); + aPaint.setAntiAlias(useAA); // 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; + const SkScalar posFix = useAA ? toSkXYFix : 0; if (mFillColor != SALCOLOR_NONE) { aPaint.setColor(toSkColorWithTransparency(mFillColor, fTransparency)); @@ -724,12 +767,76 @@ bool SkiaSalGraphicsImpl::drawPolyPolygon(const basegfx::B2DHomMatrix& rObjectTo // WORKAROUND: The logo in the about dialog has drawing errors. This seems to happen // only on Linux (not Windows on the same machine), with both AMDGPU and Mesa, // and only when antialiasing is enabled. Flushing seems to avoid the problem. - if (mParent.getAntiAliasB2DDraw() && SkiaHelper::getVendor() == DriverBlocklist::VendorAMD) + if (useAA && SkiaHelper::getVendor() == DriverBlocklist::VendorAMD) mSurface->flush(); #endif +} + +bool SkiaSalGraphicsImpl::mergePolyPolygonToPrevious(const basegfx::B2DPolyPolygon& aPolyPolygon, + double fTransparency) +{ + // There is some code that needlessly subdivides areas into adjacent rectangles, + // but Skia doesn't line them up perfectly if AA is enabled (e.g. Cairo, Qt5 do, + // but Skia devs claim it's working as intended + // https://groups.google.com/d/msg/skia-discuss/NlKpD2X_5uc/Vuwd-kyYBwAJ). + // An example is tdf#133016, which triggers SvgStyleAttributes::add_stroke() + // implementing a line stroke as a bunch of polygons instead of just one, and + // SvgLinearAtomPrimitive2D::create2DDecomposition() creates a gradient + // as a series of polygons of gradually changing color. Those places should be + // changed, but try to merge those split polygons back into the original one, + // where the needlessly created edges causing problems will not exist. + // This means drawing of such polygons needs to be delayed, so that they can + // be possibly merged with the next one. + // Merge only polygons of the same properties (color, etc.), so the gradient problem + // actually isn't handled here. + + // Only AA polygons need merging, because they do not line up well because of the AA of the edges. + if (!mParent.getAntiAliasB2DDraw()) + return false; + // Only filled polygons without an outline are problematic. + if (mFillColor == SALCOLOR_NONE || mLineColor != SALCOLOR_NONE) + return false; + // Merge only simple polygons, real polypolygons most likely aren't needlessly split, + // so they do not need joining. + if (aPolyPolygon.count() != 1) + return false; + + if (mLastPolyPolygonInfo.polygon.count() != 0 + && (mLastPolyPolygonInfo.transparency != fTransparency + || !mLastPolyPolygonInfo.polygon.getB2DRange().overlaps(aPolyPolygon.getB2DRange()))) + { + checkPendingDrawing(); // Cannot be parts of the same larger polygon, draw the last and reset. + } + if (mLastPolyPolygonInfo.polygon.count() == 0) + { + mLastPolyPolygonInfo.polygon = aPolyPolygon; + mLastPolyPolygonInfo.transparency = fTransparency; + return true; + } + basegfx::B2DPolyPolygon merged + = basegfx::utils::mergeToSinglePolyPolygon({ mLastPolyPolygonInfo.polygon, aPolyPolygon }); + if (merged.count() == 0) // it wasn't actually merged + { + checkPendingDrawing(); + mLastPolyPolygonInfo.polygon = aPolyPolygon; + mLastPolyPolygonInfo.transparency = fTransparency; + return true; + } + mLastPolyPolygonInfo.polygon = std::move(merged); return true; } +void SkiaSalGraphicsImpl::checkPendingDrawing() +{ + if (mLastPolyPolygonInfo.polygon.count() != 0) + { // Flush any pending polygon drawing. + basegfx::B2DPolyPolygon polygon; + std::swap(polygon, mLastPolyPolygonInfo.polygon); + double transparency = mLastPolyPolygonInfo.transparency; + performDrawPolyPolygon(polygon, transparency, true); + } +} + bool SkiaSalGraphicsImpl::drawPolyLine(const basegfx::B2DHomMatrix& rObjectToDevice, const basegfx::B2DPolygon& rPolyLine, double fTransparency, double fLineWidth, const std::vector<double>* pStroke, |