Skip to content
Permalink
Browse files
Percentage-based translations don't work with SVG <text>
https://bugs.webkit.org/show_bug.cgi?id=245122
<rdar://99854408>

Reviewed by Myles C. Maxfield.

The two main issues are:
- Stale SVGTextElement::animatedLocalTransform() function:
Historically, all SVG elements have had the same animatedLocalTransform function, see: f8cc29f
This looks like an artifact of when SVGTextElement did not inherit from the common SVGGraphicsElement class, where we had to add the same function twice.
Removing this code puts it in-line with SVGGraphicsElement::animatedLocalTransform() which does take in account the reference box rect (for percentage based transforms) and transform-origin.

- We also need to compute transform at the end of RenderSVGText::layout, to avoid computing with the wrong bounds (which may happen if the font hasn't loaded yet).

Rebaseline tests resulting from these changes (mainly minor differences in dimensions that match other browsers).

Test: imported/w3c/web-platform-tests/css/css-transforms/transform-box/view-box-mutation-002.html

* LayoutTests/TestExpectations:
* Source/WebCore/rendering/svg/RenderSVGText.cpp:
(WebCore::RenderSVGText::layout):
* Source/WebCore/svg/SVGTextElement.cpp:
(WebCore::SVGTextElement::animatedLocalTransform const): Deleted.
* Source/WebCore/svg/SVGTextElement.h:

Canonical link: https://commits.webkit.org/254777@main
  • Loading branch information
nt1m committed Sep 23, 2022
1 parent ee2e2ce commit ec9527d7aa0c7ee697f0b3a0f55e2fb8a7b6ed14
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 35 deletions.
@@ -4174,7 +4174,6 @@ webkit.org/b/230080 imported/w3c/web-platform-tests/css/css-transforms/scale-tra
webkit.org/b/230080 imported/w3c/web-platform-tests/css/css-transforms/transform-background-007.html [ ImageOnlyFailure ]
webkit.org/b/230080 imported/w3c/web-platform-tests/css/css-transforms/transform-background-008.html [ ImageOnlyFailure ]
webkit.org/b/230080 imported/w3c/web-platform-tests/css/css-transforms/transform-box/fill-box-002.html [ ImageOnlyFailure ]
webkit.org/b/230080 imported/w3c/web-platform-tests/css/css-transforms/transform-box/view-box-mutation-002.html [ ImageOnlyFailure ]
webkit.org/b/230080 imported/w3c/web-platform-tests/css/css-transforms/transform-fixed-bg-002.html [ ImageOnlyFailure ]
webkit.org/b/230080 imported/w3c/web-platform-tests/css/css-transforms/transform-fixed-bg-004.html [ ImageOnlyFailure ]
webkit.org/b/230080 imported/w3c/web-platform-tests/css/css-transforms/transform-fixed-bg-005.html [ ImageOnlyFailure ]
@@ -324,12 +324,13 @@ void RenderSVGText::layout()
// textElement().updateLengthContext();

bool updateCachedBoundariesInParents = false;
if (!isLayerBasedSVGEngineEnabled()) {
if (m_needsTransformUpdate) {
m_localTransform = textElement().animatedLocalTransform();
m_needsTransformUpdate = false;
updateCachedBoundariesInParents = true;
}
auto previousReferenceBoxRect = transformReferenceBoxRect();

// We update the transform now because updateScaledFont() needs it, but we do it a second time at the end of the layout,
// since the transform reference box may change because of the font change.
if (!isLayerBasedSVGEngineEnabled() && m_needsTransformUpdate) {
m_localTransform = textElement().animatedLocalTransform();
updateCachedBoundariesInParents = true;
}

if (!everHadLayout()) {
@@ -415,8 +416,15 @@ void RenderSVGText::layout()
if (isLayerBasedSVGEngineEnabled()) {
updateLayerTransform();
updateCachedBoundariesInParents = false; // No longer needed for LBSE.
} else if (!updateCachedBoundariesInParents)
updateCachedBoundariesInParents = oldBoundaries != objectBoundingBox();
} else {
if (m_needsTransformUpdate) {
if (previousReferenceBoxRect != transformReferenceBoxRect())
m_localTransform = textElement().animatedLocalTransform();
m_needsTransformUpdate = false;
}
if (!updateCachedBoundariesInParents)
updateCachedBoundariesInParents = oldBoundaries != objectBoundingBox();
}

// Invalidate all resources of this client if our layout changed.
if (layoutChanged)
@@ -44,30 +44,6 @@ Ref<SVGTextElement> SVGTextElement::create(const QualifiedName& tagName, Documen
return adoptRef(*new SVGTextElement(tagName, document));
}

// We override SVGGraphics::animatedLocalTransform() so that the transform-origin
// is not taken into account.
AffineTransform SVGTextElement::animatedLocalTransform() const
{
AffineTransform matrix;
auto* style = renderer() ? &renderer()->style() : nullptr;

// if CSS property was set, use that, otherwise fallback to attribute (if set)
if (style && style->hasTransform()) {
TransformationMatrix t;
// For now, the transform-origin is not taken into account
// Also, any percentage values will not be taken into account
style->applyTransform(t, FloatRect(0, 0, 0, 0), RenderStyle::individualTransformOperations);
// Flatten any 3D transform
matrix = t.toAffineTransform();
} else
matrix = transform().concatenate();

const AffineTransform* transform = const_cast<SVGTextElement*>(this)->supplementalTransform();
if (transform)
return *transform * matrix;
return matrix;
}

RenderPtr<RenderElement> SVGTextElement::createElementRenderer(RenderStyle&& style, const RenderTreePosition&)
{
return createRenderer<RenderSVGText>(*this, WTFMove(style));
@@ -29,8 +29,6 @@ class SVGTextElement final : public SVGTextPositioningElement {
public:
static Ref<SVGTextElement> create(const QualifiedName&, Document&);

AffineTransform animatedLocalTransform() const override;

private:
SVGTextElement(const QualifiedName&, Document&);

0 comments on commit ec9527d

Please sign in to comment.