drawinglayer/source/primitive2d/BufferedDecompositionGroupPrimitive2D.cxx |
25 ++++++----
drawinglayer/source/primitive2d/glowprimitive2d.cxx |
6 +-
drawinglayer/source/primitive2d/shadowprimitive2d.cxx |
6 +-
drawinglayer/source/primitive2d/softedgeprimitive2d.cxx |
6 +-
include/drawinglayer/primitive2d/BufferedDecompositionGroupPrimitive2D.hxx |
2
5 files changed, 25 insertions(+), 20 deletions(-)
New commits:
commit ab4cf371eda16668b875dbd99950034b09d0dc4c
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Fri Mar 14 11:36:30 2025 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Fri Mar 14 12:27:17 2025 +0100
use mutex before touching maBuffered2DDecomposition (tdf#131595 related)
Otherwise we could be reading stale data.
And change it to return a bool, the call-sites only care if we
have/dont-have the data, and returning a value of a field that should only
be accessed under a lock is dodgy.
And move the the setRemainingTime call to get2DDecomposition(), because
that where we really "use" the data ie. paint with it.
Change-Id: Ie795465a1177a47f846cf9310d419079e52b8770
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/182908
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git
a/drawinglayer/source/primitive2d/BufferedDecompositionGroupPrimitive2D.cxx
b/drawinglayer/source/primitive2d/BufferedDecompositionGroupPrimitive2D.cxx
index 4a09d0490ce9..fcdbe8d02d78 100644
--- a/drawinglayer/source/primitive2d/BufferedDecompositionGroupPrimitive2D.cxx
+++ b/drawinglayer/source/primitive2d/BufferedDecompositionGroupPrimitive2D.cxx
@@ -58,16 +58,17 @@ void
flushBufferedDecomposition(BufferedDecompositionGroupPrimitive2D& rTarget)
rTarget.release();
}
-const Primitive2DContainer&
-BufferedDecompositionGroupPrimitive2D::getBuffered2DDecomposition() const
+bool BufferedDecompositionGroupPrimitive2D::hasBuffered2DDecomposition() const
{
- if (0 != maCallbackSeconds && maCallbackTimer.is())
+ if (0 != maCallbackSeconds)
{
- // decomposition was used, touch/restart time
-
maCallbackTimer->setRemainingTime(salhelper::TTimeValue(maCallbackSeconds, 0));
+ std::lock_guard Guard(maCallbackLock);
+ return !maBuffered2DDecomposition.empty();
+ }
+ else
+ {
+ return !maBuffered2DDecomposition.empty();
}
-
- return maBuffered2DDecomposition;
}
void
BufferedDecompositionGroupPrimitive2D::setBuffered2DDecomposition(Primitive2DContainer&&
rNew)
@@ -130,7 +131,7 @@ void
BufferedDecompositionGroupPrimitive2D::get2DDecomposition(
Primitive2DDecompositionVisitor& rVisitor,
const geometry::ViewInformation2D& rViewInformation) const
{
- if (getBuffered2DDecomposition().empty())
+ if (!hasBuffered2DDecomposition())
{
Primitive2DContainer aNewSequence;
create2DDecomposition(aNewSequence, rViewInformation);
@@ -141,15 +142,19 @@ void
BufferedDecompositionGroupPrimitive2D::get2DDecomposition(
if (0 == maCallbackSeconds)
{
// no flush/multithreading is in use, just call
- rVisitor.visit(getBuffered2DDecomposition());
+ rVisitor.visit(maBuffered2DDecomposition);
return;
}
+ // decomposition was used, touch/restart time
+ if (maCallbackTimer)
+
maCallbackTimer->setRemainingTime(salhelper::TTimeValue(maCallbackSeconds, 0));
+
// tdf#158913 need to secure 'visit' when flush/multithreading is in use,
// so that the local non-ref-Counted instance of the decomposition gets not
// manipulated (e.g. deleted)
std::lock_guard Guard(maCallbackLock);
- rVisitor.visit(getBuffered2DDecomposition());
+ rVisitor.visit(maBuffered2DDecomposition);
}
} // end of namespace drawinglayer::primitive2d
diff --git a/drawinglayer/source/primitive2d/glowprimitive2d.cxx
b/drawinglayer/source/primitive2d/glowprimitive2d.cxx
index 18fa5224887d..ed212ae4da6d 100644
--- a/drawinglayer/source/primitive2d/glowprimitive2d.cxx
+++ b/drawinglayer/source/primitive2d/glowprimitive2d.cxx
@@ -261,7 +261,7 @@ void
GlowPrimitive2D::get2DDecomposition(Primitive2DDecompositionVisitor& rVisit
fDiscreteGlowRadius, rViewInformation))
return;
- if (!getBuffered2DDecomposition().empty())
+ if (hasBuffered2DDecomposition())
{
// First check is to detect if the last created decompose is capable
// to represent the now requested visualization.
@@ -295,7 +295,7 @@ void
GlowPrimitive2D::get2DDecomposition(Primitive2DDecompositionVisitor& rVisit
}
}
- if (!getBuffered2DDecomposition().empty())
+ if (hasBuffered2DDecomposition())
{
// Second check is to react on changes of the DiscreteGlowRadius when
// zooming in/out.
@@ -324,7 +324,7 @@ void
GlowPrimitive2D::get2DDecomposition(Primitive2DDecompositionVisitor& rVisit
}
}
- if (getBuffered2DDecomposition().empty())
+ if (!hasBuffered2DDecomposition())
{
// refresh last used DiscreteGlowRadius and ClippedRange to new
remembered values
const_cast<GlowPrimitive2D*>(this)->mfLastDiscreteGlowRadius =
fDiscreteGlowRadius;
diff --git a/drawinglayer/source/primitive2d/shadowprimitive2d.cxx
b/drawinglayer/source/primitive2d/shadowprimitive2d.cxx
index c6a7a2e66707..ff5600d1f5cf 100644
--- a/drawinglayer/source/primitive2d/shadowprimitive2d.cxx
+++ b/drawinglayer/source/primitive2d/shadowprimitive2d.cxx
@@ -313,7 +313,7 @@ void ShadowPrimitive2D::get2DDecomposition(
fDiscreteBlurRadius, rViewInformation))
return;
- if (!getBuffered2DDecomposition().empty())
+ if (hasBuffered2DDecomposition())
{
// First check is to detect if the last created decompose is capable
// to represent the now requested visualization (see similar
@@ -340,7 +340,7 @@ void ShadowPrimitive2D::get2DDecomposition(
}
}
- if (!getBuffered2DDecomposition().empty())
+ if (hasBuffered2DDecomposition())
{
// Second check is to react on changes of the DiscreteSoftRadius when
// zooming in/out (see similar implementation at ShadowPrimitive2D).
@@ -365,7 +365,7 @@ void ShadowPrimitive2D::get2DDecomposition(
}
}
- if (getBuffered2DDecomposition().empty())
+ if (!hasBuffered2DDecomposition())
{
// refresh last used DiscreteBlurRadius and ClippedRange to new
remembered values
const_cast<ShadowPrimitive2D*>(this)->mfLastDiscreteBlurRadius =
fDiscreteBlurRadius;
diff --git a/drawinglayer/source/primitive2d/softedgeprimitive2d.cxx
b/drawinglayer/source/primitive2d/softedgeprimitive2d.cxx
index e6f92f312f59..df05771f27a6 100644
--- a/drawinglayer/source/primitive2d/softedgeprimitive2d.cxx
+++ b/drawinglayer/source/primitive2d/softedgeprimitive2d.cxx
@@ -268,7 +268,7 @@ void SoftEdgePrimitive2D::get2DDecomposition(
fDiscreteSoftRadius,
rViewInformation))
break;
- if (!getBuffered2DDecomposition().empty())
+ if (hasBuffered2DDecomposition())
{
// First check is to detect if the last created decompose is
capable
// to represent the now requested visualization (see similar
@@ -295,7 +295,7 @@ void SoftEdgePrimitive2D::get2DDecomposition(
}
}
- if (!getBuffered2DDecomposition().empty())
+ if (hasBuffered2DDecomposition())
{
// Second check is to react on changes of the DiscreteSoftRadius
when
// zooming in/out (see similar implementation at GlowPrimitive2D).
@@ -321,7 +321,7 @@ void SoftEdgePrimitive2D::get2DDecomposition(
}
}
- if (getBuffered2DDecomposition().empty())
+ if (!hasBuffered2DDecomposition())
{
// refresh last used DiscreteSoftRadius and ClippedRange to new
remembered values
const_cast<SoftEdgePrimitive2D*>(this)->mfLastDiscreteSoftRadius =
fDiscreteSoftRadius;
diff --git
a/include/drawinglayer/primitive2d/BufferedDecompositionGroupPrimitive2D.hxx
b/include/drawinglayer/primitive2d/BufferedDecompositionGroupPrimitive2D.hxx
index f160a1ec118b..9ea787bb9d38 100644
--- a/include/drawinglayer/primitive2d/BufferedDecompositionGroupPrimitive2D.hxx
+++ b/include/drawinglayer/primitive2d/BufferedDecompositionGroupPrimitive2D.hxx
@@ -50,7 +50,7 @@ private:
protected:
/// identical to BufferedDecompositionPrimitive2D, see there please
- const Primitive2DContainer& getBuffered2DDecomposition() const;
+ bool hasBuffered2DDecomposition() const;
void setBuffered2DDecomposition(Primitive2DContainer&& rNew);
/// method which is to be used to implement the local decomposition of a
2D group primitive.
