[Libreoffice-commits] core.git: Branch 'libreoffice-7-0' - vcl/inc vcl/skia

[Luboš Luňák (via logerrit)]

 vcl/inc/skia/gdiimpl.hxx |    5 +++--
 vcl/skia/gdiimpl.cxx     |   41 ++++++++++++++++-------------------------
 2 files changed, 19 insertions(+), 27 deletions(-)

New commits:
commit 5a9821e626f41fd46dbd19b182301bda24126c9c
Author:     Luboš Luňák <l.lunak at collabora.com>
AuthorDate: Wed Aug 26 10:44:48 2020 +0200
Commit:     Caolán McNamara <caolanm at redhat.com>
CommitDate: Fri Aug 28 17:15:31 2020 +0200

    optimize polygon merging in Skia (tdf#136139)
    
    basegfx::utils::mergeToSinglePolyPolygon() can be an expensive operation
    when many polygons are involved, and the code was calling it for every
    new polygon, which is actually not needed. Simply merge all the collected
    polygons just once before they need to be drawn.
    
    Change-Id: Id29d5dd5647d262b2cdfe9963f8d4e2ace844b66
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/101384
    Tested-by: Jenkins
    Reviewed-by: Luboš Luňák <l.lunak at collabora.com>
    (cherry picked from commit 4f6551d24279aab31b23f4dd668f42d6e3d15348)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/101361
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx
index ab12fd1b521d..06f84e5b5b3e 100644
--- a/vcl/inc/skia/gdiimpl.hxx
+++ b/vcl/inc/skia/gdiimpl.hxx
@@ -282,7 +282,7 @@ protected:
     // 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);
+    bool delayDrawPolyPolygon(const basegfx::B2DPolyPolygon& polygon, double transparency);
     void performDrawPolyPolygon(const basegfx::B2DPolyPolygon& polygon, double transparency,
                                 bool useAA);
 
@@ -314,7 +314,8 @@ protected:
     // Info about pending polygons to draw (we try to merge adjacent polygons into one).
     struct LastPolyPolygonInfo
     {
-        basegfx::B2DPolyPolygon polygon;
+        basegfx::B2DPolyPolygonVector polygons;
+        basegfx::B2DRange bounds;
         double transparency;
     };
     LastPolyPolygonInfo mLastPolyPolygonInfo;
diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx
index 5efab1faaf3d..4ae57d70ea66 100644
--- a/vcl/skia/gdiimpl.cxx
+++ b/vcl/skia/gdiimpl.cxx
@@ -727,7 +727,7 @@ bool SkiaSalGraphicsImpl::drawPolyPolygon(const basegfx::B2DHomMatrix& rObjectTo
     SAL_INFO("vcl.skia.trace", "drawpolypolygon(" << this << "): " << aPolyPolygon << ":"
                                                   << mLineColor << ":" << mFillColor);
 
-    if (mergePolyPolygonToPrevious(aPolyPolygon, fTransparency))
+    if (delayDrawPolyPolygon(aPolyPolygon, fTransparency))
     {
         scheduleFlush();
         return true;
@@ -782,8 +782,8 @@ void SkiaSalGraphicsImpl::performDrawPolyPolygon(const basegfx::B2DPolyPolygon&
 #endif
 }
 
-bool SkiaSalGraphicsImpl::mergePolyPolygonToPrevious(const basegfx::B2DPolyPolygon& aPolyPolygon,
-                                                     double fTransparency)
+bool SkiaSalGraphicsImpl::delayDrawPolyPolygon(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,
@@ -811,39 +811,30 @@ bool SkiaSalGraphicsImpl::mergePolyPolygonToPrevious(const basegfx::B2DPolyPolyg
     if (aPolyPolygon.count() != 1)
         return false;
 
-    if (mLastPolyPolygonInfo.polygon.count() != 0
+    if (mLastPolyPolygonInfo.polygons.size() != 0
         && (mLastPolyPolygonInfo.transparency != fTransparency
-            || !mLastPolyPolygonInfo.polygon.getB2DRange().overlaps(aPolyPolygon.getB2DRange())))
+            || !mLastPolyPolygonInfo.bounds.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);
+    // Collect the polygons that can be possibly merged. Do the merging only once at the end,
+    // because it's not a cheap operation.
+    mLastPolyPolygonInfo.polygons.push_back(aPolyPolygon);
+    mLastPolyPolygonInfo.bounds.expand(aPolyPolygon.getB2DRange());
+    mLastPolyPolygonInfo.transparency = fTransparency;
     return true;
 }
 
 void SkiaSalGraphicsImpl::checkPendingDrawing()
 {
-    if (mLastPolyPolygonInfo.polygon.count() != 0)
+    if (mLastPolyPolygonInfo.polygons.size() != 0)
     { // Flush any pending polygon drawing.
-        basegfx::B2DPolyPolygon polygon;
-        std::swap(polygon, mLastPolyPolygonInfo.polygon);
+        basegfx::B2DPolyPolygonVector polygons;
+        std::swap(polygons, mLastPolyPolygonInfo.polygons);
         double transparency = mLastPolyPolygonInfo.transparency;
-        performDrawPolyPolygon(polygon, transparency, true);
+        mLastPolyPolygonInfo.bounds.reset();
+        basegfx::B2DPolyPolygon merged = basegfx::utils::mergeToSinglePolyPolygon(polygons);
+        performDrawPolyPolygon(merged, transparency, true);
     }
 }