commit 56772201997ecd525b535364b00ff2168dcc1f4e
parent cc8d67d595ce8e8d214284093fcb553d177d8782
Author: Jonathan Kew <jkew@mozilla.com>
Date: Sat, 4 Oct 2025 22:40:54 +0000
Bug 1991689 - Cache a PropertyProvider in SVGTextFrame so that we don't have to create a fresh one for every call to the same textframe. r=firefox-svg-reviewers,longsonr
Differential Revision: https://phabricator.services.mozilla.com/D267440
Diffstat:
3 files changed, 47 insertions(+), 17 deletions(-)
diff --git a/layout/generic/nsTextFrame.h b/layout/generic/nsTextFrame.h
@@ -112,9 +112,12 @@ class nsTextFrame : public nsIFrame {
/**
* An implementation of gfxTextRun::PropertyProvider that computes spacing and
* hyphenation based on CSS properties for a text frame.
+ *
+ * nsTextFrame normally creates a PropertyProvider as a temporary object on
+ * on the stack, but this is not marked MOZ_STACK_CLASS because SVGTextFrame
+ * wants to cache an instance across multiple calls using the same textframe.
*/
- class MOZ_STACK_CLASS PropertyProvider final
- : public gfxTextRun::PropertyProvider {
+ class PropertyProvider final : public gfxTextRun::PropertyProvider {
using HyphenType = gfxTextRun::HyphenType;
public:
diff --git a/layout/svg/SVGTextFrame.cpp b/layout/svg/SVGTextFrame.cpp
@@ -835,7 +835,6 @@ SVGBBox TextRenderedRun::GetRunUserSpaceRect(nsPresContext* aContext,
vertical ? -self.y : -self.x);
gfxSkipCharsIterator it = mFrame->EnsureTextRun(nsTextFrame::eInflated);
- gfxSkipCharsIterator start = it;
gfxTextRun* textRun = mFrame->GetTextRun(nsTextFrame::eInflated);
// Get the content range for this rendered run.
@@ -845,10 +844,7 @@ SVGBBox TextRenderedRun::GetRunUserSpaceRect(nsPresContext* aContext,
return r;
}
- // FIXME(heycam): We could create a single PropertyProvider for all
- // TextRenderedRuns that correspond to the text frame, rather than recreate
- // it each time here.
- nsTextFrame::PropertyProvider provider(mFrame, start);
+ auto& provider = mRoot->PropertyProviderFor(mFrame);
// Measure that range.
gfxTextRun::Metrics metrics = textRun->MeasureText(
@@ -946,7 +942,7 @@ void TextRenderedRun::GetClipEdges(nscoord& aVisIStartEdge,
gfxSkipCharsIterator it = mFrame->EnsureTextRun(nsTextFrame::eInflated);
gfxTextRun* textRun = mFrame->GetTextRun(nsTextFrame::eInflated);
- nsTextFrame::PropertyProvider provider(mFrame, it);
+ auto& provider = mRoot->PropertyProviderFor(mFrame);
// Get the covered content offset/length for this rendered run in skipped
// characters, since that is what GetAdvanceWidth expects.
@@ -1038,7 +1034,7 @@ void TextRenderedRun::GetClipEdges(nscoord& aVisIStartEdge,
nscoord TextRenderedRun::GetAdvanceWidth() const {
gfxSkipCharsIterator it = mFrame->EnsureTextRun(nsTextFrame::eInflated);
gfxTextRun* textRun = mFrame->GetTextRun(nsTextFrame::eInflated);
- nsTextFrame::PropertyProvider provider(mFrame, it);
+ auto& provider = mRoot->PropertyProviderFor(mFrame);
Range range = ConvertOriginalToSkipped(it, mTextFrameContentOffset,
mTextFrameContentLength);
@@ -1088,7 +1084,7 @@ int32_t TextRenderedRun::GetCharNumAtPosition(nsPresContext* aContext,
gfxSkipCharsIterator it = mFrame->EnsureTextRun(nsTextFrame::eInflated);
gfxTextRun* textRun = mFrame->GetTextRun(nsTextFrame::eInflated);
- nsTextFrame::PropertyProvider provider(mFrame, it);
+ auto& provider = mRoot->PropertyProviderFor(mFrame);
// Next check that the point lies horizontally within the left and right
// edges of the text.
@@ -1787,6 +1783,11 @@ class TextRenderedRunIterator {
mCurrent(First()) {}
/**
+ * Ensure any cached PropertyProvider is cleared at the end of the iteration.
+ */
+ ~TextRenderedRunIterator() { mFrameIterator.Root()->ForgetCachedProvider(); }
+
+ /**
* Returns the current TextRenderedRun.
*/
TextRenderedRun Current() const { return mCurrent; }
@@ -2016,6 +2017,11 @@ class MOZ_STACK_CLASS CharIterator {
nsIContent* aSubtree, bool aPostReflow = true);
/**
+ * Ensure any cached PropertyProvider is cleared at the end of the iteration.
+ */
+ ~CharIterator() { mFrameIterator.Root()->ForgetCachedProvider(); }
+
+ /**
* Returns whether the iterator is finished.
*/
bool AtEnd() const { return !mFrameIterator.Current(); }
@@ -2374,10 +2380,7 @@ gfxFloat CharIterator::GetAdvance(nsPresContext* aContext) const {
float cssPxPerDevPx =
nsPresContext::AppUnitsToFloatCSSPixels(aContext->AppUnitsPerDevPixel());
- gfxSkipCharsIterator start =
- TextFrame()->EnsureTextRun(nsTextFrame::eInflated);
- nsTextFrame::PropertyProvider provider(TextFrame(), start);
-
+ auto& provider = mFrameIterator.Root()->PropertyProviderFor(TextFrame());
uint32_t offset = mSkipCharsIterator.GetSkippedOffset();
gfxFloat advance =
mTextRun->GetAdvanceWidth(Range(offset, offset + 1), &provider);
@@ -3733,7 +3736,7 @@ float SVGTextFrame::GetSubStringLengthFastPath(nsIContent* aContent,
gfxSkipCharsIterator it = frame->EnsureTextRun(nsTextFrame::eInflated);
gfxTextRun* textRun = frame->GetTextRun(nsTextFrame::eInflated);
- nsTextFrame::PropertyProvider provider(frame, it);
+ auto& provider = PropertyProviderFor(frame);
Range range = ConvertOriginalToSkipped(it, offset, trimmedLength);
@@ -3801,7 +3804,7 @@ float SVGTextFrame::GetSubStringLengthSlowFallback(nsIContent* aContent,
gfxSkipCharsIterator it =
run.mFrame->EnsureTextRun(nsTextFrame::eInflated);
gfxTextRun* textRun = run.mFrame->GetTextRun(nsTextFrame::eInflated);
- nsTextFrame::PropertyProvider provider(run.mFrame, it);
+ auto& provider = PropertyProviderFor(run.mFrame);
Range range = ConvertOriginalToSkipped(it, offset, length);
@@ -4356,7 +4359,7 @@ void SVGTextFrame::DetermineCharPositions(nsTArray<nsPoint>& aPositions) {
for (nsTextFrame* frame = frit.Current(); frame; frame = frit.Next()) {
gfxSkipCharsIterator it = frame->EnsureTextRun(nsTextFrame::eInflated);
gfxTextRun* textRun = frame->GetTextRun(nsTextFrame::eInflated);
- nsTextFrame::PropertyProvider provider(frame, it);
+ auto& provider = PropertyProviderFor(frame);
// Reset the position to the new frame's position.
position = frit.Position();
@@ -4410,6 +4413,10 @@ void SVGTextFrame::DetermineCharPositions(nsTArray<nsPoint>& aPositions) {
for (uint32_t i = 0; i < frit.UndisplayedCharacters(); i++) {
aPositions.AppendElement(position);
}
+
+ // Clear any cached PropertyProvider, to avoid risk of re-using it after
+ // style changes or other mutations may have invalidated it.
+ ForgetCachedProvider();
}
/**
diff --git a/layout/svg/SVGTextFrame.h b/layout/svg/SVGTextFrame.h
@@ -11,6 +11,7 @@
#include "gfxRect.h"
#include "gfxTextRun.h"
#include "mozilla/Attributes.h"
+#include "mozilla/Maybe.h"
#include "mozilla/PresShellForwards.h"
#include "mozilla/RefPtr.h"
#include "mozilla/SVGContainerFrame.h"
@@ -610,9 +611,28 @@ class SVGTextFrame final : public SVGDisplayContainerFrame {
return mCachedRanges[aWhichRange];
}
+ // Return a reference to a PropertyProvider for the given textframe;
+ // the provider is cached by SVGTextFrame to avoid creating it afresh
+ // for repeated operations involving the same textframe.
+ nsTextFrame::PropertyProvider& PropertyProviderFor(nsTextFrame* aFrame) {
+ if (!mCachedProvider || aFrame != mCachedProvider->GetFrame()) {
+ mCachedProvider.reset();
+ mCachedProvider.emplace(aFrame,
+ aFrame->EnsureTextRun(nsTextFrame::eInflated));
+ }
+ return mCachedProvider.ref();
+ }
+
+ // Forget any cached PropertyProvider. This should be called at the end of
+ // any method that relied on PropertyProviderFor(), to avoid leaving a
+ // cached provider that may become invalid.
+ void ForgetCachedProvider() { mCachedProvider.reset(); }
+
private:
const nsTextFrame* mFrameForCachedRanges = nullptr;
CachedMeasuredRange mCachedRanges[CachedRangeCount];
+
+ Maybe<nsTextFrame::PropertyProvider> mCachedProvider;
};
} // namespace mozilla
