From 2da7a82f6b7a78e1e4b34b0b3c7a27d2b3c47a35 Mon Sep 17 00:00:00 2001 From: Armin Le Grand Date: Wed, 22 Jan 2020 17:20:13 +0100 Subject: 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 Tested-by: Jenkins --- 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(-) 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((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((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(this)->mnCalculatedCycles = nResult; } - - // set locally (once, on-demand created, non-zero) - const_cast(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 addOrReplaceSystemDependentData(SystemDependentDataManager& manager, Args&&... args) const { std::shared_ptr r = std::make_shared(manager, std::forward(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 addOrReplaceSystemDependentData(SystemDependentDataManager& manager, Args&&... args) const { std::shared_ptr r = std::make_shared(manager, std::forward(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 pSystemDependentData_CairoPath( + rPolyLine.getSystemDependentData()); - if (!bIsTrivial) + if(pSystemDependentData_CairoPath) { - // try to access buffered data - std::shared_ptr pSystemDependentData_CairoPath( - rPolyLine.getSystemDependentData()); - - 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( + pSystemDependentData_CairoPath = rPolyLine.addOrReplaceSystemDependentData( 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 pSystemDependentData_CairoPath( + rPolyPolygon.getSystemDependentData()); - if (!bIsTrivial) + if(pSystemDependentData_CairoPath) { - // try to access buffered data - std::shared_ptr pSystemDependentData_CairoPath( - rPolyPolygon.getSystemDependentData()); - - 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( - 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( + 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 addOrReplaceSystemDependentData(basegfx::SystemDependentDataManager& manager, Args&&... args) const { std::shared_ptr r = std::make_shared(manager, std::forward(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; } }; -- cgit v1.2.3