Skip to content

Commit

Permalink
SVGTextMetricsBuilder::measureTextRenderer exhibits O(n^2) behavior
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=264076

Reviewed by Chris Dumez.

Prior to this PR, RenderSVGText::willLayout() called SVGTextLayoutAttributesBuilder's
rebuildMetricsForTextRenderer and therefore SVGTextMetricsBuilder's measureTextRenderer
on each descendant RenderObject of RenderSVGText. Since rebuildMetricsForTextRenderer
does a tree traversal from the ancestor RenderSVGText to the specified node, this
exhibited O(1+2+3+ ... +n) = O(n^2) behavior.

This PR rectifies this situation by combining all rebuildMetricsForTextRenderer calls
for descendants as a single call to SVGTextLayoutAttributesBuilder's now renamed
rebuildMetricsForSubtree.

This PR also eliminates O(n^2) behavior in updateFontInAllDescendants in RenderSVGText.cpp
by combining all calls to rebuildMetricsForTextRenderer.

* Source/WebCore/rendering/svg/RenderSVGText.cpp:
(WebCore::RenderSVGText::willLayout):
(WebCore::updateFontInAllDescendants):
(WebCore::RenderSVGText::layout):
* Source/WebCore/rendering/svg/SVGTextLayoutAttributesBuilder.cpp:
(WebCore::SVGTextLayoutAttributesBuilder::rebuildMetricsForSubtree):
(WebCore::SVGTextLayoutAttributesBuilder::rebuildMetricsForTextRenderer): Deleted.
* Source/WebCore/rendering/svg/SVGTextLayoutAttributesBuilder.h:
* Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp:
(WebCore::SVGTextMetricsBuilder::measureTextRenderer):
* Source/WebCore/rendering/svg/SVGTextMetricsBuilder.h:

Canonical link: https://commits.webkit.org/270110@main
  • Loading branch information
rniwa committed Nov 2, 2023
1 parent ef20478 commit 1f27ea4
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 21 deletions.
19 changes: 8 additions & 11 deletions Source/WebCore/rendering/svg/RenderSVGText.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,7 @@ void RenderSVGText::willLayout()

// Only update the metrics cache, but not the text positioning element cache
// nor the layout attributes cached in the leaf #text renderers.
for (RenderObject* descendant = firstChild(); descendant; descendant = descendant->nextInPreOrder(this)) {
if (is<RenderSVGInlineText>(*descendant))
m_layoutAttributesBuilder.rebuildMetricsForTextRenderer(downcast<RenderSVGInlineText>(*descendant));
}
m_layoutAttributesBuilder.rebuildMetricsForSubtree(*this);
}

void RenderSVGText::subtreeTextDidChange(RenderSVGInlineText* text)
Expand Down Expand Up @@ -296,16 +293,16 @@ void RenderSVGText::subtreeTextDidChange(RenderSVGInlineText* text)
}
}

static inline void updateFontInAllDescendants(RenderObject* start, SVGTextLayoutAttributesBuilder* builder = nullptr)
static inline void updateFontInAllDescendants(RenderSVGText& text, SVGTextLayoutAttributesBuilder* builder = nullptr)
{
for (RenderObject* descendant = start; descendant; descendant = descendant->nextInPreOrder(start)) {
for (RenderObject* descendant = &text; descendant; descendant = descendant->nextInPreOrder(&text)) {
if (!is<RenderSVGInlineText>(*descendant))
continue;
auto& text = downcast<RenderSVGInlineText>(*descendant);
text.updateScaledFont();
if (builder)
builder->rebuildMetricsForTextRenderer(text);
}
if (builder)
builder->rebuildMetricsForSubtree(text);
}

void RenderSVGText::layout()
Expand Down Expand Up @@ -343,7 +340,7 @@ void RenderSVGText::layout()
// and propogate resulting SVGLayoutAttributes to all RenderSVGInlineText children in the subtree.
ASSERT(m_layoutAttributes.isEmpty());
collectLayoutAttributes(this, m_layoutAttributes);
updateFontInAllDescendants(this);
updateFontInAllDescendants(*this);
m_layoutAttributesBuilder.buildLayoutAttributesForForSubtree(*this);

m_needsReordering = true;
Expand All @@ -354,7 +351,7 @@ void RenderSVGText::layout()
// When the x/y/dx/dy/rotate lists change, recompute the layout attributes, and eventually
// update the on-screen font objects as well in all descendants.
if (m_needsTextMetricsUpdate) {
updateFontInAllDescendants(this);
updateFontInAllDescendants(*this);
m_needsTextMetricsUpdate = false;
}

Expand All @@ -374,7 +371,7 @@ void RenderSVGText::layout()
if (m_needsTextMetricsUpdate || isLayoutSizeChanged) {
// If the root layout size changed (eg. window size changes) or the transform to the root
// context has changed then recompute the on-screen font size.
updateFontInAllDescendants(this, &m_layoutAttributesBuilder);
updateFontInAllDescendants(*this, &m_layoutAttributesBuilder);

ASSERT(!m_needsReordering);
ASSERT(!m_needsPositioningValuesUpdate);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ bool SVGTextLayoutAttributesBuilder::buildLayoutAttributesForForSubtree(RenderSV
return true;
}

void SVGTextLayoutAttributesBuilder::rebuildMetricsForTextRenderer(RenderSVGInlineText& text)
void SVGTextLayoutAttributesBuilder::rebuildMetricsForSubtree(RenderSVGText& text)
{
m_metricsBuilder.measureTextRenderer(text);
m_metricsBuilder.measureTextRenderer(text, nullptr);
}

static inline void processRenderSVGInlineText(const RenderSVGInlineText& text, unsigned& atCharacter, bool& lastCharacterWasSpace)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class SVGTextLayoutAttributesBuilder {
bool buildLayoutAttributesForForSubtree(RenderSVGText&);
void buildLayoutAttributesForTextRenderer(RenderSVGInlineText&);

void rebuildMetricsForTextRenderer(RenderSVGInlineText&);
void rebuildMetricsForSubtree(RenderSVGText&);

// Invoked whenever the underlying DOM tree changes, so that m_textPositions is rebuild.
void clearTextPositioningElements() { m_textPositions.clear(); }
Expand Down
8 changes: 2 additions & 6 deletions Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,10 @@ void SVGTextMetricsBuilder::walkTree(RenderElement& start, RenderSVGInlineText*
}
}

void SVGTextMetricsBuilder::measureTextRenderer(RenderSVGInlineText& text)
void SVGTextMetricsBuilder::measureTextRenderer(RenderSVGText& textRoot, RenderSVGInlineText* stopAtLeaf)
{
auto* textRoot = RenderSVGText::locateRenderSVGTextAncestor(text);
if (!textRoot)
return;

MeasureTextData data(nullptr);
walkTree(*textRoot, &text, &data);
walkTree(textRoot, stopAtLeaf, &data);
}

void SVGTextMetricsBuilder::buildMetricsAndLayoutAttributes(RenderSVGText& textRoot, RenderSVGInlineText* stopAtLeaf, SVGCharacterDataMap& allCharactersMap)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/rendering/svg/SVGTextMetricsBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class SVGTextMetricsBuilder {
WTF_MAKE_NONCOPYABLE(SVGTextMetricsBuilder);
public:
SVGTextMetricsBuilder();
void measureTextRenderer(RenderSVGInlineText&);
void measureTextRenderer(RenderSVGText&, RenderSVGInlineText* stopAtLeaf);
void buildMetricsAndLayoutAttributes(RenderSVGText&, RenderSVGInlineText* stopAtLeaf, SVGCharacterDataMap& allCharactersMap);

private:
Expand Down

0 comments on commit 1f27ea4

Please sign in to comment.