[Libreoffice-commits] core.git: basegfx/source include/basegfx vcl/headless vcl/inc

Armin Le Grand (via logerrit) logerrit at kemper.freedesktop.org
Thu Jan 23 16:49:27 UTC 2020


 basegfx/source/tools/systemdependentdata.cxx |   59 ++++++-----
 include/basegfx/polygon/b2dpolygon.hxx       |   10 +
 include/basegfx/polygon/b2dpolypolygon.hxx   |   10 +
 vcl/headless/svpgdi.cxx                      |  142 +++++++++++++--------------
 vcl/inc/win/salbmp.h                         |   10 +
 5 files changed, 127 insertions(+), 104 deletions(-)

New commits:
commit 2da7a82f6b7a78e1e4b34b0b3c7a27d2b3c47a35
Author:     Armin Le Grand <Armin.Le.Grand at me.com>
AuthorDate: Wed Jan 22 17:20:13 2020 +0100
Commit:     Armin Le Grand <Armin.Le.Grand at me.com>
CommitDate: Thu Jan 23 17:48:55 2020 +0100

    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 at collabora.com>
    Tested-by: Jenkins

diff --git a/basegfx/source/tools/systemdependentdata.cxx b/basegfx/source/tools/systemdependentdata.cxx
index 223b607ffae0..53b1465eaf55 100644
--- a/basegfx/source/tools/systemdependentdata.cxx
+++ b/basegfx/source/tools/systemdependentdata.cxx
@@ -77,39 +77,48 @@ namespace basegfx
         if(0 == mnCalculatedCycles)
         {
             const sal_Int64 nBytes(estimateUsageInBytes());
-            const sal_uInt32 nSeconds = 60; // HoldCyclesInSeconds
 
-            // default is Seconds (minimal is one)
-            sal_uInt32 nResult(0 == nSeconds ? 1 : nSeconds);
-
-            if(0 != nBytes)
+            // tdf#129845 as indicator for no need to buffer trivial data, stay at and
+            // return zero. As border, use 450 bytes. For polygons, this means to buffer
+            // starting with ca. 50 points (GDIPLUS uses 9 bytes per coordinate). For
+            // Bitmap data this means to more or less always buffer (as it was before).
+            // For the future, a more sophisticated differentioation may be added
+            if(nBytes > 450)
             {
-                // use sqrt to get some curved shape. With a default of 60s we get
-                // a single second at 3600 byte. To get close to 10mb, multiply by
-                // a corresponding scaling factor
-                const double fScaleToMB(3600.0 / (1024.0 * 1024.0 * 10.0));
-
-                // also use a multiplier to move the start point higher
-                const double fMultiplierSeconds(10.0);
+                const sal_uInt32 nSeconds = 60; // HoldCyclesInSeconds
 
-                // calculate
-                nResult = static_cast<sal_uInt32>((fMultiplierSeconds * nSeconds) / sqrt(nBytes * fScaleToMB));
+                // default is Seconds (minimal is one)
+                sal_uInt32 nResult(0 == nSeconds ? 1 : nSeconds);
 
-                // minimal value is 1
-                if(nResult < 1)
+                if(0 != nBytes)
                 {
-                    nResult = 1;
+                    // use sqrt to get some curved shape. With a default of 60s we get
+                    // a single second at 3600 byte. To get close to 10mb, multiply by
+                    // a corresponding scaling factor
+                    const double fScaleToMB(3600.0 / (1024.0 * 1024.0 * 10.0));
+
+                    // also use a multiplier to move the start point higher
+                    const double fMultiplierSeconds(10.0);
+
+                    // calculate
+                    nResult = static_cast<sal_uInt32>((fMultiplierSeconds * nSeconds) / sqrt(nBytes * fScaleToMB));
+
+                    // minimal value is 1
+                    if(nResult < 1)
+                    {
+                        nResult = 1;
+                    }
+
+                    // maximal value is nSeconds
+                    if(nResult > nSeconds)
+                    {
+                        nResult = nSeconds;
+                    }
                 }
 
-                // maximal value is nSeconds
-                if(nResult > nSeconds)
-                {
-                    nResult = nSeconds;
-                }
+                // set locally (once, on-demand created, non-zero)
+                const_cast<SystemDependentData*>(this)->mnCalculatedCycles = nResult;
             }
-
-            // set locally (once, on-demand created, non-zero)
-            const_cast<SystemDependentData*>(this)->mnCalculatedCycles = nResult;
         }
 
         return mnCalculatedCycles;
diff --git a/include/basegfx/polygon/b2dpolygon.hxx b/include/basegfx/polygon/b2dpolygon.hxx
index cbef3159705b..72be3525fc0f 100644
--- a/include/basegfx/polygon/b2dpolygon.hxx
+++ b/include/basegfx/polygon/b2dpolygon.hxx
@@ -239,8 +239,14 @@ namespace basegfx
         std::shared_ptr<T> addOrReplaceSystemDependentData(SystemDependentDataManager& manager, Args&&... args) const
         {
             std::shared_ptr<T> r = std::make_shared<T>(manager, std::forward<Args>(args)...);
-            basegfx::SystemDependentData_SharedPtr r2(r);
-            addOrReplaceSystemDependentDataInternal(r2);
+
+            // tdf#129845 only add to buffer if a relevant buffer time is estimated
+            if(r->calculateCombinedHoldCyclesInSeconds() > 0)
+            {
+                basegfx::SystemDependentData_SharedPtr r2(r);
+                addOrReplaceSystemDependentDataInternal(r2);
+            }
+
             return r;
         }
 
diff --git a/include/basegfx/polygon/b2dpolypolygon.hxx b/include/basegfx/polygon/b2dpolypolygon.hxx
index 65e3b97cfc96..d1af340048fd 100644
--- a/include/basegfx/polygon/b2dpolypolygon.hxx
+++ b/include/basegfx/polygon/b2dpolypolygon.hxx
@@ -138,8 +138,14 @@ namespace basegfx
         std::shared_ptr<T> addOrReplaceSystemDependentData(SystemDependentDataManager& manager, Args&&... args) const
         {
             std::shared_ptr<T> r = std::make_shared<T>(manager, std::forward<Args>(args)...);
-            basegfx::SystemDependentData_SharedPtr r2(r);
-            addOrReplaceSystemDependentDataInternal(r2);
+
+            // tdf#129845 only add to buffer if a relevant buffer time is estimated
+            if(r->calculateCombinedHoldCyclesInSeconds() > 0)
+            {
+                basegfx::SystemDependentData_SharedPtr r2(r);
+                addOrReplaceSystemDependentDataInternal(r2);
+            }
+
             return r;
         }
 
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;
     }
 };


More information about the Libreoffice-commits mailing list