[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-6.2' - vcl/headless

[Michael Meeks (via logerrit)]

 vcl/headless/svpgdi.cxx |   97 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 64 insertions(+), 33 deletions(-)

New commits:
commit bfe88c34b5917fd68b74d1f2fa04353a7341c282
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Mon Jan 6 20:49:40 2020 +0000
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Tue Jan 7 11:10:54 2020 +0100

    tdf#129845 vcl: avoid expensive system caching of trivial lines & polygons
    
    Interestingly the cache map lookup is rather expensive; why:
    
    polyline output for some trivial impress edits:
            count   # of polylines
            2       2134
            3       141
            4       41
            9       4
    
    polypolygon output for some trivial impress edits:
    
            count   # of polypolygons       geometry.
            3       54    all single polygon
            4       583   ~all single
            9       52    ~ all paired with a 4 node polygon
            13      2     both single
            32      22    all single
    
    Change-Id: I15c0053a84399eaf153b2119b2c28d1f168f16b1
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/86313
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/vcl/headless/svpgdi.cxx b/vcl/headless/svpgdi.cxx
index 8baf93d67693..c4453c7f4560 100644
--- a/vcl/headless/svpgdi.cxx
+++ b/vcl/headless/svpgdi.cxx
@@ -114,6 +114,18 @@ namespace
         aDamageRect.intersect(getClipBox(cr));
         return aDamageRect;
     }
+
+    // The caching logic is surprsingly expensive - so avoid it sometimes.
+    inline bool isTrivial(const basegfx::B2DPolyPolygon& rPolyPolygon)
+    {
+        return rPolyPolygon.count() == 1 && rPolyPolygon.begin()->count() <= 4;
+    }
+
+    // The caching logic is surprsingly expensive - so avoid it sometimes.
+    inline bool isTrivial(const basegfx::B2DPolygon& rPolyLine)
+    {
+        return rPolyLine.count() <= 4;
+    }
 }
 
 bool SvpSalGraphics::blendBitmap( const SalTwoRect&, const SalBitmap& /*rBitmap*/ )
@@ -1270,29 +1282,37 @@ bool SvpSalGraphics::drawPolyLine(
     cairo_set_line_width(cr, aLineWidths.getX());
     cairo_set_miter_limit(cr, fMiterLimit);
 
-    // try to access buffered data
-    std::shared_ptr<SystemDependentData_CairoPath> pSystemDependentData_CairoPath(
-        rPolyLine.getSystemDependentData<SystemDependentData_CairoPath>());
+    bool bDone = false;
+    bool bIsTrivial = isTrivial(rPolyLine);
 
-    if(pSystemDependentData_CairoPath)
+    if (!bIsTrivial)
     {
-        // check data validity
-        if(nullptr == pSystemDependentData_CairoPath->getCairoPath()
-            || pSystemDependentData_CairoPath->getNoJoin() != bNoJoin
-            || pSystemDependentData_CairoPath->getAntiAliasB2DDraw() != bAntiAliasB2DDraw
-            || bPixelSnapHairline /*tdf#124700*/ )
+        // try to access buffered data
+        std::shared_ptr<SystemDependentData_CairoPath> pSystemDependentData_CairoPath(
+            rPolyLine.getSystemDependentData<SystemDependentData_CairoPath>());
+
+        if(pSystemDependentData_CairoPath)
         {
-            // data invalid, forget
-            pSystemDependentData_CairoPath.reset();
+            // check data validity
+            if(nullptr == pSystemDependentData_CairoPath->getCairoPath()
+               || pSystemDependentData_CairoPath->getNoJoin() != bNoJoin
+               || pSystemDependentData_CairoPath->getAntiAliasB2DDraw() != bAntiAliasB2DDraw
+               || bPixelSnapHairline /*tdf#124700*/ )
+            {
+                // data invalid, forget
+                pSystemDependentData_CairoPath.reset();
+            }
         }
-    }
 
-    if(pSystemDependentData_CairoPath)
-    {
-        // re-use data
-        cairo_append_path(cr, pSystemDependentData_CairoPath->getCairoPath());
+        if(pSystemDependentData_CairoPath)
+        {
+            // re-use data
+            cairo_append_path(cr, pSystemDependentData_CairoPath->getCairoPath());
+            bDone = true;
+        }
     }
-    else
+
+    if (!bDone)
     {
         // create data
         if (!bNoJoin)
@@ -1335,9 +1355,9 @@ bool SvpSalGraphics::drawPolyLine(
         }
 
         // copy and add to buffering mechanism
-        if (!bPixelSnapHairline /*tdf#124700*/)
+        if (!bIsTrivial && !bPixelSnapHairline /*tdf#124700*/)
         {
-            pSystemDependentData_CairoPath = rPolyLine.addOrReplaceSystemDependentData<SystemDependentData_CairoPath>(
+            rPolyLine.addOrReplaceSystemDependentData<SystemDependentData_CairoPath>(
                 ImplGetSystemDependentDataManager(),
                 cairo_copy_path(cr),
                 bNoJoin,
@@ -1412,16 +1432,24 @@ bool SvpSalGraphics::drawPolyPolygon(
         cairo_set_matrix(cr, &aMatrix);
     }
 
-    // try to access buffered data
-    std::shared_ptr<SystemDependentData_CairoPath> pSystemDependentData_CairoPath(
-        rPolyPolygon.getSystemDependentData<SystemDependentData_CairoPath>());
+    bool bDone = false;
+    bool bIsTrivial = isTrivial(rPolyPolygon);
 
-    if(pSystemDependentData_CairoPath)
+    if (!bIsTrivial)
     {
-        // re-use data
-        cairo_append_path(cr, pSystemDependentData_CairoPath->getCairoPath());
+        // try to access buffered data
+        std::shared_ptr<SystemDependentData_CairoPath> pSystemDependentData_CairoPath(
+            rPolyPolygon.getSystemDependentData<SystemDependentData_CairoPath>());
+
+        if(pSystemDependentData_CairoPath)
+        {
+            // re-use data
+            cairo_append_path(cr, pSystemDependentData_CairoPath->getCairoPath());
+            bDone = true;
+        }
     }
-    else
+
+    if (!bDone)
     {
         // create data
         for (const auto & rPoly : rPolyPolygon)
@@ -1436,13 +1464,16 @@ bool SvpSalGraphics::drawPolyPolygon(
                 false);
         }
 
-        // copy and add to buffering mechanism
-        // for decisions how/what to buffer, see Note in WinSalGraphicsImpl::drawPolyPolygon
-        pSystemDependentData_CairoPath = rPolyPolygon.addOrReplaceSystemDependentData<SystemDependentData_CairoPath>(
-            ImplGetSystemDependentDataManager(),
-            cairo_copy_path(cr),
-            false,
-            false);
+        if (!bIsTrivial)
+        {
+            // copy and add to buffering mechanism
+            // for decisions how/what to buffer, see Note in WinSalGraphicsImpl::drawPolyPolygon
+            rPolyPolygon.addOrReplaceSystemDependentData<SystemDependentData_CairoPath>(
+                ImplGetSystemDependentDataManager(),
+                cairo_copy_path(cr),
+                false,
+                false);
+        }
     }
 
     // To make releaseCairoContext work, use empty extents