tdf#129845: Better solution using already existing info

Use calculateCombinedHoldCyclesInSeconds() in central
places of system-dependent buffering and the zero value
to early exclude data from buffering. This solves the
problem on all system-dependent usages in a central
place. Also enhanced to roughly allow buffering for
bitmaps unchanged, for polygons starting with ca. 50
coordinate pairs.
Added special treatments to Cairo version to allow
temp buffer objects without copying the path data. This
needed some extra stuff due to Cairo not allowing
to work with it's cr-internal path object directly.

Change-Id: Icd0a0d8091707fe356a82f5c7ec48f36ad44ccde
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/87199
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Jenkins

2 files changed, 77 insertions, 75 deletions

diff --git a/vcl/headless/svpgdi.cxx b/vcl/headless/svpgdi.cxx
index 8c76c21f4f9d..0d4a366fc4f1 100644
--- a/vcl/headless/svpgdi.cxx
+++ b/vcl/headless/svpgdi.cxx
@@ -119,18 +119,6 @@ namespace
         aDamageRect.intersect(getClipBox(cr));
         return aDamageRect;
     }
-
-    // The caching logic is surprisingly expensive - so avoid it sometimes.
-    inline bool isTrivial(const basegfx::B2DPolyPolygon& rPolyPolygon)
-    {
-        return rPolyPolygon.count() == 1 && rPolyPolygon.begin()->count() <= 4;
-    }
-
-    // The caching logic is surprisingly expensive - so avoid it sometimes.
-    inline bool isTrivial(const basegfx::B2DPolygon& rPolyLine)
-    {
-        return rPolyLine.count() <= 4;
-    }
 }
 
 bool SvpSalGraphics::blendBitmap( const SalTwoRect&, const SalBitmap& /*rBitmap*/ )
@@ -894,7 +882,9 @@ static basegfx::B2DPoint impPixelSnap(
 // For support of PixelSnapHairline we also need the ObjectToDevice transformation
 // and a method (same as in gdiimpl.cxx for Win and Gdiplus). This is needed e.g.
 // for Chart-content visualization. CAUTION: It's not the same as PixelSnap (!)
-static void AddPolygonToPath(
+// tdf#129845 add reply value to allow counting a point/byte/size measurement to
+// be included
+static size_t AddPolygonToPath(
     cairo_t* cr,
     const basegfx::B2DPolygon& rPolygon,
     const basegfx::B2DHomMatrix& rObjectToDevice,
@@ -903,10 +893,11 @@ static void AddPolygonToPath(
 {
     // short circuit if there is nothing to do
     const sal_uInt32 nPointCount(rPolygon.count());
+    size_t nSizeMeasure(0);
 
     if(0 == nPointCount)
     {
-        return;
+        return nSizeMeasure;
     }
 
     const bool bHasCurves(rPolygon.areControlPointsUsed());
@@ -985,6 +976,7 @@ static void AddPolygonToPath(
         if( !bPendingCurve )    // line segment
         {
             cairo_line_to(cr, aPoint.getX(), aPoint.getY());
+            nSizeMeasure++;
         }
         else                        // cubic bezier segment
         {
@@ -1007,6 +999,10 @@ static void AddPolygonToPath(
 
             cairo_curve_to(cr, aCP1.getX(), aCP1.getY(), aCP2.getX(), aCP2.getY(),
                                aPoint.getX(), aPoint.getY());
+            // take some bigger measure for curve segments - too expensive to subdivide
+            // here and that precision not needed, but four (2 points, 2 control-points)
+            // would be a too low weight
+            nSizeMeasure += 10;
         }
 
         aLast = aPoint;
@@ -1016,6 +1012,8 @@ static void AddPolygonToPath(
     {
         cairo_close_path(cr);
     }
+
+    return nSizeMeasure;
 }
 
 void SvpSalGraphics::drawLine( long nX1, long nY1, long nX2, long nY2 )
@@ -1070,7 +1068,8 @@ private:
 public:
     SystemDependentData_CairoPath(
         basegfx::SystemDependentDataManager& rSystemDependentDataManager,
-        cairo_path_t* pCairoPath,
+        size_t nSizeMeasure,
+        cairo_t* cr,
         bool bNoJoin,
         bool bAntiAliasB2DDraw);
     virtual ~SystemDependentData_CairoPath() override;
@@ -1086,14 +1085,21 @@ public:
 
 SystemDependentData_CairoPath::SystemDependentData_CairoPath(
     basegfx::SystemDependentDataManager& rSystemDependentDataManager,
-    cairo_path_t* pCairoPath,
+    size_t nSizeMeasure,
+    cairo_t* cr,
     bool bNoJoin,
     bool bAntiAliasB2DDraw)
 :   basegfx::SystemDependentData(rSystemDependentDataManager),
-    mpCairoPath(pCairoPath),
+    mpCairoPath(nullptr),
     mbNoJoin(bNoJoin),
     mbAntiAliasB2DDraw(bAntiAliasB2DDraw)
 {
+    // tdf#129845 only create a copy of the path when nSizeMeasure is
+    // bigger than some decent threshold
+    if(nSizeMeasure > 50)
+    {
+        mpCairoPath = cairo_copy_path(cr);
+    }
 }
 
 SystemDependentData_CairoPath::~SystemDependentData_CairoPath()
@@ -1107,6 +1113,9 @@ SystemDependentData_CairoPath::~SystemDependentData_CairoPath()
 
 sal_Int64 SystemDependentData_CairoPath::estimateUsageInBytes() const
 {
+    // tdf#129845 by using the default return value of zero when no path
+    // was created, SystemDependentData::calculateCombinedHoldCyclesInSeconds
+    // will do the right thing and not buffer this entry at all
     sal_Int64 nRetval(0);
 
     if(nullptr != mpCairoPath)
@@ -1291,43 +1300,37 @@ bool SvpSalGraphics::drawPolyLine(
     cairo_set_line_width(cr, aLineWidths.getX());
     cairo_set_miter_limit(cr, fMiterLimit);
 
-    bool bDone = false;
-    bool bIsTrivial = isTrivial(rPolyLine);
+    // try to access buffered data
+    std::shared_ptr<SystemDependentData_CairoPath> pSystemDependentData_CairoPath(
+        rPolyLine.getSystemDependentData<SystemDependentData_CairoPath>());
 
-    if (!bIsTrivial)
+    if(pSystemDependentData_CairoPath)
     {
-        // try to access buffered data
-        std::shared_ptr<SystemDependentData_CairoPath> pSystemDependentData_CairoPath(
-            rPolyLine.getSystemDependentData<SystemDependentData_CairoPath>());
-
-        if(pSystemDependentData_CairoPath)
+        // check data validity
+        if(nullptr == pSystemDependentData_CairoPath->getCairoPath()
+            || pSystemDependentData_CairoPath->getNoJoin() != bNoJoin
+            || pSystemDependentData_CairoPath->getAntiAliasB2DDraw() != bAntiAliasB2DDraw
+            || bPixelSnapHairline /*tdf#124700*/ )
         {
-            // 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());
-            bDone = true;
+            // data invalid, forget
+            pSystemDependentData_CairoPath.reset();
         }
     }
 
-    if (!bDone)
+    if(pSystemDependentData_CairoPath)
+    {
+        // re-use data
+        cairo_append_path(cr, pSystemDependentData_CairoPath->getCairoPath());
+    }
+    else
     {
         // create data
+        size_t nSizeMeasure(0);
+
         if (!bNoJoin)
         {
             // PixelOffset now reflected in linear transformation used
-            AddPolygonToPath(
+            nSizeMeasure += AddPolygonToPath(
                 cr,
                 rPolyLine,
                 rObjectToDevice, // ObjectToDevice *without* LineDraw-Offset
@@ -1351,7 +1354,7 @@ bool SvpSalGraphics::drawPolyLine(
                 aEdge.setPrevControlPoint(1, rPolyLine.getPrevControlPoint(nNextIndex));
 
                 // PixelOffset now reflected in linear transformation used
-                AddPolygonToPath(
+                nSizeMeasure += AddPolygonToPath(
                     cr,
                     aEdge,
                     rObjectToDevice, // ObjectToDevice *without* LineDraw-Offset
@@ -1364,11 +1367,12 @@ bool SvpSalGraphics::drawPolyLine(
         }
 
         // copy and add to buffering mechanism
-        if (!bIsTrivial && !bPixelSnapHairline /*tdf#124700*/)
+        if (!bPixelSnapHairline /*tdf#124700*/)
         {
-            rPolyLine.addOrReplaceSystemDependentData<SystemDependentData_CairoPath>(
+            pSystemDependentData_CairoPath = rPolyLine.addOrReplaceSystemDependentData<SystemDependentData_CairoPath>(
                 ImplGetSystemDependentDataManager(),
-                cairo_copy_path(cr),
+                nSizeMeasure,
+                cr,
                 bNoJoin,
                 bAntiAliasB2DDraw);
         }
@@ -1417,31 +1421,25 @@ namespace
 {
     void add_polygon_path(cairo_t* cr, const basegfx::B2DPolyPolygon& rPolyPolygon, const basegfx::B2DHomMatrix& rObjectToDevice, bool bPixelSnap)
     {
-        bool bDone = false;
-        bool bIsTrivial = isTrivial(rPolyPolygon);
+        // try to access buffered data
+        std::shared_ptr<SystemDependentData_CairoPath> pSystemDependentData_CairoPath(
+            rPolyPolygon.getSystemDependentData<SystemDependentData_CairoPath>());
 
-        if (!bIsTrivial)
+        if(pSystemDependentData_CairoPath)
         {
-            // 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;
-            }
+            // re-use data
+            cairo_append_path(cr, pSystemDependentData_CairoPath->getCairoPath());
         }
-
-        if (!bDone)
+        else
         {
             // create data
+            size_t nSizeMeasure(0);
+
             for (const auto & rPoly : rPolyPolygon)
             {
                 // PixelOffset used: Was dependent of 'm_aLineColor != SALCOLOR_NONE'
                 // Adapt setupPolyPolygon-users to set a linear transformation to achieve PixelOffset
-                AddPolygonToPath(
+                nSizeMeasure += AddPolygonToPath(
                     cr,
                     rPoly,
                     rObjectToDevice,
@@ -1449,16 +1447,14 @@ namespace
                     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);
-            }
+            // copy and add to buffering mechanism
+            // for decisions how/what to buffer, see Note in WinSalGraphicsImpl::drawPolyPolygon
+            pSystemDependentData_CairoPath = rPolyPolygon.addOrReplaceSystemDependentData<SystemDependentData_CairoPath>(
+                ImplGetSystemDependentDataManager(),
+                nSizeMeasure,
+                cr,
+                false,
+                false);
         }
     }
 }
diff --git a/vcl/inc/win/salbmp.h b/vcl/inc/win/salbmp.h
index 05aed0ee9d30..d6acd14e8c7d 100644
--- a/vcl/inc/win/salbmp.h
+++ b/vcl/inc/win/salbmp.h
@@ -98,8 +98,14 @@ public:
     std::shared_ptr<T> addOrReplaceSystemDependentData(basegfx::SystemDependentDataManager& manager, Args&&... args) const
     {
         std::shared_ptr<T> r = std::make_shared<T>(manager, std::forward<Args>(args)...);
-        basegfx::SystemDependentData_SharedPtr r2(r);
-        const_cast< WinSalBitmap* >(this)->basegfx::SystemDependentDataHolder::addOrReplaceSystemDependentData(r2);
+
+        // tdf#129845 only add to buffer if a relevant buffer time is estimated
+        if(r->calculateCombinedHoldCyclesInSeconds() > 0)
+        {
+            basegfx::SystemDependentData_SharedPtr r2(r);
+            const_cast< WinSalBitmap* >(this)->basegfx::SystemDependentDataHolder::addOrReplaceSystemDependentData(r2);
+        }
+
         return r;
     }
 };