Commit 6ea24ace authored by Fredrik Söderquist's avatar Fredrik Söderquist Committed by Commit Bot

[PE] SVG <use> 'x' and 'y' are presentation attributes

'x' and 'y' were turned into presentation attributes in
https://codereview.chromium.org/896773002 (r190180), but the property
values were not actually applied.
Apply the property values from ComputedStyle, cleaning up the
surrounding code a bit in the process by moving the "is a <use> element"
check into a method. Also fix the FIXME in SVGUseElement::ToClipPath by
using the computed transform from the LayoutObject, rather than
"manually" recomputing it. This code-path should already require a
LayoutObject for the element in question.

Bug: 400725, 697052
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Iff261e468c4476cfb98efbadd6989ada027e47ab
Reviewed-on: https://chromium-review.googlesource.com/829314Reviewed-by: default avatarStephen Chenney <schenney@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#524385}
parent 1f07513d
<!DOCTYPE html>
<div style="width: 100px; height: 100px; background-color: green"></div>
<!DOCTYPE html>
<svg>
<defs>
<rect id="r" x="100" y="50" width="100" height="100"/>
<clipPath id="c">
<use xlink:href="#r" style="x: -100px; y: -50px"/>
</clipPath>
</defs>
<rect width="200" height="200" fill="red" clip-path="url(#c)"/>
<rect width="100" height="100" fill="green"/>
</svg>
<!DOCTYPE html>
<div style="width: 100px; height: 100px; background-color: green"></div>
<!DOCTYPE html>
<svg>
<defs>
<rect id="r" x="100" y="50" width="100" height="100" fill="green"/>
</defs>
<rect width="100" height="100" fill="red"/>
<use href="#r" x="100" y="100" style="x: -100px; y: -50px"/>
</svg>
<!DOCTYPE html>
<div style="width: 100px; height: 100px; background-color: green"></div>
<!DOCTYPE html>
<script src="../../resources/run-after-layout-and-paint.js"></script>
<svg>
<defs>
<rect id="r" x="100" y="50" width="100" height="100" fill="green"/>
</defs>
<rect width="100" height="100" fill="red"/>
<use href="#r" x="100" y="100" style="y: -50px"/>
</svg>
<script>
runAfterLayoutAndPaint(function() {
document.querySelector('use').style.x = '-100px';
}, true);
</script>
......@@ -73,27 +73,33 @@ void LayoutSVGTransformableContainer::SetNeedsTransformUpdate() {
needs_transform_update_ = true;
}
bool LayoutSVGTransformableContainer::IsUseElement() const {
const SVGElement& element = *GetElement();
if (IsSVGUseElement(element))
return true;
// Nested <use> are replaced by <g> during shadow tree expansion.
if (IsSVGGElement(element) && ToSVGGElement(element).InUseShadowTree())
return IsSVGUseElement(element.CorrespondingElement());
return false;
}
SVGTransformChange LayoutSVGTransformableContainer::CalculateLocalTransform() {
SVGGraphicsElement* element = ToSVGGraphicsElement(GetElement());
SVGElement* element = GetElement();
DCHECK(element);
// If we're either the layoutObject for a <use> element, or for any <g>
// If we're either the LayoutObject for a <use> element, or for any <g>
// element inside the shadow tree, that was created during the use/symbol/svg
// expansion in SVGUseElement. These containers need to respect the
// translations induced by their corresponding use elements x/y attributes.
SVGUseElement* use_element = ToSVGUseElementOrNull(*element);
if (!use_element && IsSVGGElement(*element) &&
ToSVGGElement(element)->InUseShadowTree())
use_element = ToSVGUseElementOrNull(element->CorrespondingElement());
if (use_element) {
if (IsUseElement()) {
const ComputedStyle& style = StyleRef();
const SVGComputedStyle& svg_style = style.SvgStyle();
SVGLengthContext length_context(element);
FloatSize translation(
use_element->x()->CurrentValue()->Value(length_context),
use_element->y()->CurrentValue()->Value(length_context));
// TODO(fs): Signal this on style update instead. (Since these are
// suppose to be presentation attributes now, this does feel a bit
// broken...)
FloatSize translation(length_context.ValueForLength(svg_style.X(), style,
SVGLengthMode::kWidth),
length_context.ValueForLength(
svg_style.Y(), style, SVGLengthMode::kHeight));
// TODO(fs): Signal this on style update instead.
if (translation != additional_translation_)
SetNeedsTransformUpdate();
additional_translation_ = translation;
......
......@@ -48,6 +48,7 @@ class LayoutSVGTransformableContainer final : public LayoutSVGContainer {
AffineTransform LocalSVGTransform() const override {
return local_transform_;
}
bool IsUseElement() const;
bool needs_transform_update_ : 1;
AffineTransform local_transform_;
......
......@@ -498,12 +498,11 @@ Path SVGUseElement::ToClipPath() const {
if (!element || !element->IsSVGGeometryElement())
return Path();
DCHECK(GetLayoutObject());
Path path = ToSVGGeometryElement(*element).ToClipPath();
// FIXME: Avoid manual resolution of x/y here. Its potentially harmful.
SVGLengthContext length_context(this);
path.Translate(FloatSize(x_->CurrentValue()->Value(length_context),
y_->CurrentValue()->Value(length_context)));
path.Transform(CalculateTransform(SVGElement::kIncludeMotionTransform));
AffineTransform transform = GetLayoutObject()->LocalSVGTransform();
if (!transform.IsIdentity())
path.Transform(transform);
return path;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment