ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

lsan:fix leak in CairoPixelProcessor2D::getOrCreateCairoSurfaceHelper

Browse Source 
shows up when running CppunitTest_sc_shapetest

probably from the recent changes here, when calling tryToGetCairoSurface.

Rather than doing explicit cleanup, lets follow the same path used in
  include/vcl/cairo.hxx
and pass around a shared_ptr that makes cleanup automatic.

Change-Id: I1bec6f579af182b259cb0edbd86192627e5c61ea
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/1858
Tested-by: Jenkins CPCI <releng@collaboraoffice.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
(cherry picked from commit 70966239f816d81cb9dcb208e7c02e08339da28c)
(cherry picked from commit ee2349dba8aa282e30ae65cbf5ffbb27b707e0ec)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/205990
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
This commit is contained in:
Noel Grandin
2026-06-05 08:43:09 +02:00
committed by Xisco Fauli
parent df1ae71700
commit f87c623b4e
3 changed files with 66 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
drawinglayer/source/processor2d/cairopixelprocessor2d.cxx
+54 -55
View File
@@ -431,10 +431,10 @@ nMinimalDiscreteSquareSizeToBuffer(nMinimalDiscreteSize* nMinimalDiscreteSize);
class CairoSurfaceHelper
{
// the buffered CairoSurface (bitmap data)
cairo_surface_t* mpCairoSurface;
cairo::CairoSurfaceSharedPtr mpCairoSurface;
// evtl. MipMapped data (pre-scale to reduce data processing load)
mutable std::unordered_map<sal_uInt64, cairo_surface_t*> maDownscaled;
mutable std::unordered_map<sal_uInt64, cairo::CairoSurfaceSharedPtr> maDownscaled;
// create 32bit RGBA data for given Bitmap
void createRGBA(const Bitmap& rBitmap)
@@ -442,15 +442,17 @@ class CairoSurfaceHelper
BitmapScopedReadAccess pReadAccess(rBitmap);
const tools::Long nHeight(pReadAccess->Height());
const tools::Long nWidth(pReadAccess->Width());
mpCairoSurface = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, nWidth, nHeight);
if (cairo_surface_status(mpCairoSurface) != CAIRO_STATUS_SUCCESS)
mpCairoSurface = cairo::CairoSurfaceSharedPtr(
cairo_image_surface_create(CAIRO_FORMAT_ARGB32, nWidth, nHeight),
&cairo_surface_destroy);
if (cairo_surface_status(mpCairoSurface.get()) != CAIRO_STATUS_SUCCESS)
{
SAL_WARN("drawinglayer",
"cairo_image_surface_create failed for: " << nWidth << " x " << nHeight);
return;
}
const sal_uInt32 nStride(cairo_format_stride_for_width(CAIRO_FORMAT_ARGB32, nWidth));
unsigned char* surfaceData(cairo_image_surface_get_data(mpCairoSurface));
unsigned char* surfaceData(cairo_image_surface_get_data(mpCairoSurface.get()));
for (tools::Long y(0); y < nHeight; ++y)
{
@@ -469,7 +471,7 @@ class CairoSurfaceHelper
}
}
cairo_surface_mark_dirty(mpCairoSurface);
cairo_surface_mark_dirty(mpCairoSurface.get());
}
// create 32bit RGB data for given Bitmap
@@ -478,15 +480,17 @@ class CairoSurfaceHelper
BitmapScopedReadAccess pReadAccess(rBitmap);
const tools::Long nHeight(pReadAccess->Height());
const tools::Long nWidth(pReadAccess->Width());
mpCairoSurface = cairo_image_surface_create(CAIRO_FORMAT_RGB24, nWidth, nHeight);
if (cairo_surface_status(mpCairoSurface) != CAIRO_STATUS_SUCCESS)
mpCairoSurface = cairo::CairoSurfaceSharedPtr(
cairo_image_surface_create(CAIRO_FORMAT_RGB24, nWidth, nHeight),
&cairo_surface_destroy);
if (cairo_surface_status(mpCairoSurface.get()) != CAIRO_STATUS_SUCCESS)
{
SAL_WARN("drawinglayer",
"cairo_image_surface_create failed for: " << nWidth << " x " << nHeight);
return;
}
sal_uInt32 nStride(cairo_format_stride_for_width(CAIRO_FORMAT_RGB24, nWidth));
unsigned char* surfaceData(cairo_image_surface_get_data(mpCairoSurface));
unsigned char* surfaceData(cairo_image_surface_get_data(mpCairoSurface.get()));
for (tools::Long y(0); y < nHeight; ++y)
{
@@ -504,7 +508,7 @@ class CairoSurfaceHelper
}
}
cairo_surface_mark_dirty(mpCairoSurface);
cairo_surface_mark_dirty(mpCairoSurface.get());
}
// #define TEST_RGB16
@@ -553,7 +557,7 @@ public:
CairoSurfaceHelper(const Bitmap& rBitmap)
// When using a cairo-backed Bitmap (i.e. SvpSalBitmap), we can avoid a lot of copying,
// which is beneficial for documents with lots of large images. Try to access directly.
: mpCairoSurface(static_cast<cairo_surface_t*>(rBitmap.tryToGetCairoSurface()))
: mpCairoSurface(rBitmap.tryToGetCairoSurface())
, maDownscaled()
{
if (nullptr != mpCairoSurface)
@@ -569,18 +573,8 @@ public:
#endif
}
~CairoSurfaceHelper()
{
// cleanup surface
cairo_surface_destroy(mpCairoSurface);
// cleanup MipMap surfaces
for (auto& candidate : maDownscaled)
cairo_surface_destroy(candidate.second);
}
cairo_surface_t* getCairoSurface(sal_uInt32 nTargetWidth = 0,
sal_uInt32 nTargetHeight = 0) const
cairo::CairoSurfaceSharedPtr getCairoSurface(sal_uInt32 nTargetWidth = 0,
sal_uInt32 nTargetHeight = 0) const
{
// in simple cases just return the single created surface
if (bDisableDownScale || nullptr == mpCairoSurface || 0 == nTargetWidth
@@ -588,8 +582,8 @@ public:
return mpCairoSurface;
// get width/height of original surface
const sal_uInt32 nSourceWidth(cairo_image_surface_get_width(mpCairoSurface));
const sal_uInt32 nSourceHeight(cairo_image_surface_get_height(mpCairoSurface));
const sal_uInt32 nSourceWidth(cairo_image_surface_get_width(mpCairoSurface.get()));
const sal_uInt32 nSourceHeight(cairo_image_surface_get_height(mpCairoSurface.get()));
// zoomed in on both axes, need to stretch at paint, no pre-scale useful
if (nTargetWidth >= nSourceWidth && nTargetHeight >= nSourceHeight)
@@ -639,15 +633,17 @@ public:
return isHit->second;
// create new surface in the targeted size
cairo_surface_t* pSurfaceTarget(cairo_surface_create_similar(
mpCairoSurface, cairo_surface_get_content(mpCairoSurface), nW, nH));
cairo::CairoSurfaceSharedPtr pSurfaceTarget(
cairo_surface_create_similar(mpCairoSurface.get(),
cairo_surface_get_content(mpCairoSurface.get()), nW, nH),
&cairo_surface_destroy);
cairo_t* cr = cairo_create(pSurfaceTarget);
cairo_t* cr = cairo_create(pSurfaceTarget.get());
const double fScaleX(static_cast<double>(nW) / static_cast<double>(nSourceWidth));
const double fScaleY(static_cast<double>(nH) / static_cast<double>(nSourceHeight));
cairo_scale(cr, fScaleX, fScaleY);
cairo_set_source_surface(cr, mpCairoSurface, 0.0, 0.0);
cairo_set_source_surface(cr, mpCairoSurface.get(), 0.0, 0.0);
cairo_pattern_set_filter(cairo_get_source(cr), CAIRO_FILTER_GOOD);
cairo_paint(cr);
cairo_destroy(cr);
@@ -667,8 +663,8 @@ public:
if (nullptr == mpCairoSurface)
return true;
const sal_uInt32 nSourceWidth(cairo_image_surface_get_width(mpCairoSurface));
const sal_uInt32 nSourceHeight(cairo_image_surface_get_height(mpCairoSurface));
const sal_uInt32 nSourceWidth(cairo_image_surface_get_width(mpCairoSurface.get()));
const sal_uInt32 nSourceHeight(cairo_image_surface_get_height(mpCairoSurface.get()));
return nSourceWidth * nSourceHeight < nMinimalDiscreteSquareSizeToBuffer;
}
@@ -714,9 +710,9 @@ sal_uInt32 SystemDependentData_CairoSurface::calculateCombinedHoldCyclesInSecond
// To detect 'big' graphics, compare to an assumed ScreenSize (may also
// be fetched from real screen if needed). To react, create a buffer
// residing time of one hour (increase if needed).
cairo_surface_t* pSurface(mpCairoSurfaceHelper->getCairoSurface());
const tools::Long nStride(cairo_image_surface_get_stride(pSurface));
const tools::Long nHeight(cairo_image_surface_get_height(pSurface));
cairo::CairoSurfaceSharedPtr pSurface(mpCairoSurfaceHelper->getCairoSurface());
const tools::Long nStride(cairo_image_surface_get_stride(pSurface.get()));
const tools::Long nHeight(cairo_image_surface_get_height(pSurface.get()));
// use a common big screen and assume 1/2 size of a graphic as 'big'
const sal_Int64 nBigGraphicPixels((2560 * 1440) / 2);
@@ -739,9 +735,9 @@ sal_Int64 SystemDependentData_CairoSurface::estimateUsageInBytes() const
if (mpCairoSurfaceHelper)
{
cairo_surface_t* pSurface(mpCairoSurfaceHelper->getCairoSurface());
const tools::Long nStride(cairo_image_surface_get_stride(pSurface));
const tools::Long nHeight(cairo_image_surface_get_height(pSurface));
cairo::CairoSurfaceSharedPtr pSurface(mpCairoSurfaceHelper->getCairoSurface());
const tools::Long nStride(cairo_image_surface_get_stride(pSurface.get()));
const tools::Long nHeight(cairo_image_surface_get_height(pSurface.get()));
// w * h * 4 bytesPerPixel
nRetval = nStride * nHeight * 4;
@@ -760,7 +756,7 @@ sal_Int64 SystemDependentData_CairoSurface::estimateUsageInBytes() const
std::shared_ptr<CairoSurfaceHelper> getOrCreateCairoSurfaceHelper(const Bitmap& rBitmap)
{
cairo_surface_t* pSurface(static_cast<cairo_surface_t*>(rBitmap.tryToGetCairoSurface()));
cairo::CairoSurfaceSharedPtr pSurface(rBitmap.tryToGetCairoSurface());
if (nullptr != pSurface)
{
// in this case we get a cairo_surface_t directly from the underlying
@@ -780,7 +776,8 @@ std::shared_ptr<CairoSurfaceHelper> getOrCreateCairoSurfaceHelper(const Bitmap&
// for !isCairoCompatible cases.
const tools::Long aSmallBitmapSquareSize(160 * 100);
if (cairo_image_surface_get_stride(pSurface) * cairo_image_surface_get_height(pSurface)
if (cairo_image_surface_get_stride(pSurface.get())
* cairo_image_surface_get_height(pSurface.get())
<= aSmallBitmapSquareSize)
{
// take the shortcut: For this small directly accessible size
@@ -1435,8 +1432,9 @@ void CairoPixelProcessor2D::paintBitmapAlpha(const Bitmap& rBitmap,
return;
}
cairo_surface_t* pTarget(aCairoSurfaceHelper->getCairoSurface(nDestWidth, nDestHeight));
if (nullptr == pTarget)
cairo::CairoSurfaceSharedPtr pTarget(
aCairoSurfaceHelper->getCairoSurface(nDestWidth, nDestHeight));
if (!pTarget)
{
SAL_WARN("drawinglayer", "SDPRCairo: No CairoSurface from Bitmap SurfaceHelper (!)");
return;
@@ -1460,7 +1458,7 @@ void CairoPixelProcessor2D::paintBitmapAlpha(const Bitmap& rBitmap,
cairo_stroke(mpRT);
}
cairo_set_source_surface(mpRT, pTarget, 0, 0);
cairo_set_source_surface(mpRT, pTarget.get(), 0, 0);
// get the pattern created by cairo_set_source_surface and
// it's transformation
@@ -1485,8 +1483,8 @@ void CairoPixelProcessor2D::paintBitmapAlpha(const Bitmap& rBitmap,
// simple (we are in unit coordinates)
cairo_rectangle(mpRT, 0, 0, 1, 1);
cairo_clip(mpRT);
cairo_matrix_scale(&aMatrix, cairo_image_surface_get_width(pTarget),
cairo_image_surface_get_height(pTarget));
cairo_matrix_scale(&aMatrix, cairo_image_surface_get_width(pTarget.get()),
cairo_image_surface_get_height(pTarget.get()));
// The alternative wpuld be: resize/scale it SLIGHTLY to force
// that half pixel overlap to be inside the unit range.
@@ -2224,15 +2222,15 @@ void CairoPixelProcessor2D::processMarkerArrayPrimitive2D(
}
// do not use dimensions, these are usually small instances
cairo_surface_t* pTarget(aCairoSurfaceHelper->getCairoSurface());
if (nullptr == pTarget)
cairo::CairoSurfaceSharedPtr pTarget(aCairoSurfaceHelper->getCairoSurface());
if (!pTarget)
{
SAL_WARN("drawinglayer", "SDPRCairo: No CairoSurface from Bitmap SurfaceHelper (!)");
return;
}
const sal_uInt32 nWidth(cairo_image_surface_get_width(pTarget));
const sal_uInt32 nHeight(cairo_image_surface_get_height(pTarget));
const sal_uInt32 nWidth(cairo_image_surface_get_width(pTarget.get()));
const sal_uInt32 nHeight(cairo_image_surface_get_height(pTarget.get()));
const tools::Long nMiX((nWidth / 2) + 1);
const tools::Long nMiY((nHeight / 2) + 1);
@@ -2248,7 +2246,7 @@ void CairoPixelProcessor2D::processMarkerArrayPrimitive2D(
const double fX(ceil(aDiscretePos.getX()));
const double fY(ceil(aDiscretePos.getY()));
cairo_set_source_surface(mpRT, pTarget, fX - nMiX, fY - nMiY);
cairo_set_source_surface(mpRT, pTarget.get(), fX - nMiX, fY - nMiY);
cairo_paint(mpRT);
}
@@ -2716,8 +2714,9 @@ void CairoPixelProcessor2D::processFillGraphicPrimitive2D(
return;
}
cairo_surface_t* pTarget(aCairoSurfaceHelper->getCairoSurface(nDestWidth, nDestHeight));
if (nullptr == pTarget)
cairo::CairoSurfaceSharedPtr pTarget(
aCairoSurfaceHelper->getCairoSurface(nDestWidth, nDestHeight));
if (!pTarget)
{
SAL_WARN("drawinglayer", "SDPRCairo: No CairoSurface from Bitmap SurfaceHelper (!)");
return;
@@ -2731,10 +2730,10 @@ void CairoPixelProcessor2D::processFillGraphicPrimitive2D(
aLocalTransform.d(), aLocalTransform.e(), aLocalTransform.f());
cairo_set_matrix(mpRT, &aMatrix);
const sal_uInt32 nWidth(cairo_image_surface_get_width(pTarget));
const sal_uInt32 nHeight(cairo_image_surface_get_height(pTarget));
const sal_uInt32 nWidth(cairo_image_surface_get_width(pTarget.get()));
const sal_uInt32 nHeight(cairo_image_surface_get_height(pTarget.get()));
cairo_set_source_surface(mpRT, pTarget, 0, 0);
cairo_set_source_surface(mpRT, pTarget.get(), 0, 0);
// get the pattern created by cairo_set_source_surface and
// it's transformation
@@ -2742,7 +2741,7 @@ void CairoPixelProcessor2D::processFillGraphicPrimitive2D(
cairo_pattern_get_matrix(sourcepattern, &aMatrix);
// clip for RGBA (see other places)
if (CAIRO_FORMAT_ARGB32 == cairo_image_surface_get_format(pTarget))
if (CAIRO_FORMAT_ARGB32 == cairo_image_surface_get_format(pTarget.get()))
{
cairo_rectangle(mpRT, 0, 0, 1, 1);
cairo_clip(mpRT);
include/vcl/bitmap.hxx
+8 -1
View File
@@ -101,6 +101,13 @@ class SalBitmap;
namespace basegfx { class SystemDependentDataHolder; }
#if USE_HEADLESS_CODE
typedef struct _cairo_surface cairo_surface_t;
namespace cairo {
typedef std::shared_ptr<cairo_surface_t> CairoSurfaceSharedPtr;
}
#endif
struct BitmapSystemData
{
#if defined(_WIN32)
@@ -721,7 +728,7 @@ public:
void DumpAsPng(const char* pFileName = nullptr) const;
#if USE_HEADLESS_CODE
void* tryToGetCairoSurface() const;
cairo::CairoSurfaceSharedPtr tryToGetCairoSurface() const;
#endif
private:
vcl/source/bitmap/bitmap.cxx
+4 -2
View File
@@ -388,7 +388,7 @@ Bitmap& Bitmap::operator=( const Bitmap& rBitmap )
}
#if USE_HEADLESS_CODE
void* Bitmap::tryToGetCairoSurface() const
cairo::CairoSurfaceSharedPtr Bitmap::tryToGetCairoSurface() const
{
SvpSalBitmap* pSvpSalBitmap(dynamic_cast<SvpSalBitmap*>(ImplGetSalBitmap().get()));
if (nullptr == pSvpSalBitmap)
@@ -398,7 +398,9 @@ void* Bitmap::tryToGetCairoSurface() const
if (nullptr == pBitmapBuffer)
return nullptr;
return CairoCommon::createCairoSurface(pBitmapBuffer);
return cairo::CairoSurfaceSharedPtr(
CairoCommon::createCairoSurface(pBitmapBuffer),
&cairo_surface_destroy);
}
#endif