Commit 85b96ea9 authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

[PE] Ensure update of LayoutSVGShape::NoScalingStrokeTransform

In Pre-SPv175 we forced subtree paint invalidation on non-composited
transform change. SPv175 no longer does that, causing
NonScalingStrokeTransform not updated on ancestor transform change.

We also had a non-obvious bug that LayoutSVGShape::StrokeBoundingBox
didn't get updated on ancestor transform change.

Now always explicitly update non-scaling-stroke data during layout.

Bug: 849080
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ia61eb94f43e53a71a80e1102e4d605e4331f44b1
Reviewed-on: https://chromium-review.googlesource.com/1086715
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarFredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#564584}
parent e4f65bdd
<!DOCTYPE html>
<svg width="400" height="400">
<rect stroke="black" fill="white" x="30" y="30" width="300" height="300" vector-effect="non-scaling-stroke"/>
</svg>
<!DOCTYPE html>
<svg width="400" height="400">
<g id="g">
<rect stroke="black" fill="white" x="10" y="10" width="100" height="100" vector-effect="non-scaling-stroke"/>
</g>
</svg>
<script src="../../../resources/run-after-layout-and-paint.js"></script>
<script>
runAfterLayoutAndPaint(function() {
g.setAttribute("transform", "scale(3)");
}, true);
</script>
......@@ -77,8 +77,6 @@ void LayoutSVGShape::CreatePath() {
if (!path_)
path_ = std::make_unique<Path>();
*path_ = ToSVGGeometryElement(GetElement())->AsPath();
if (rare_data_.get())
rare_data_->cached_non_scaling_stroke_path_.Clear();
}
float LayoutSVGShape::DashScaleFactor() const {
......@@ -89,8 +87,15 @@ float LayoutSVGShape::DashScaleFactor() const {
void LayoutSVGShape::UpdateShapeFromElement() {
CreatePath();
fill_bounding_box_ = CalculateObjectBoundingBox();
if (HasNonScalingStroke()) {
// NonScalingStrokeTransform may depend on LocalTransform which in turn may
// depend on ObjectBoundingBox, thus we need to call them in this order.
UpdateLocalTransform();
UpdateNonScalingStrokeData();
}
stroke_bounding_box_ = CalculateStrokeBoundingBox();
}
......@@ -156,11 +161,8 @@ bool LayoutSVGShape::ShapeDependentStrokeContains(const FloatPoint& point) {
DashScaleFactor());
if (HasNonScalingStroke()) {
AffineTransform non_scaling_transform = NonScalingStrokeTransform();
Path* use_path = NonScalingStrokePath(path_.get(), non_scaling_transform);
return use_path->StrokeContains(non_scaling_transform.MapPoint(point),
stroke_data);
return NonScalingStrokePath().StrokeContains(
NonScalingStrokeTransform().MapPoint(point), stroke_data);
}
return path_->StrokeContains(point, stroke_data);
......@@ -254,7 +256,10 @@ void LayoutSVGShape::UpdateLayout() {
// UpdateShapeFromElement() also updates the object & stroke bounds - which
// feeds into the visual rect - so we need to call it for both the
// shape-update and the bounds-update flag.
if (needs_shape_update_ || needs_boundaries_update_) {
// We also need to update stroke bounds if HasNonScalingStroke() because the
// shape may be affected by ancestor transforms.
if (needs_shape_update_ || needs_boundaries_update_ ||
HasNonScalingStroke()) {
FloatRect old_object_bounding_box = ObjectBoundingBox();
UpdateShapeFromElement();
if (old_object_bounding_box != ObjectBoundingBox()) {
......@@ -303,21 +308,9 @@ void LayoutSVGShape::UpdateLayout() {
ClearNeedsLayout();
}
Path* LayoutSVGShape::NonScalingStrokePath(
const Path* path,
const AffineTransform& stroke_transform) const {
LayoutSVGShapeRareData& rare_data = EnsureRareData();
if (!rare_data.cached_non_scaling_stroke_path_.IsEmpty() &&
stroke_transform == rare_data.cached_non_scaling_stroke_transform_)
return &rare_data.cached_non_scaling_stroke_path_;
rare_data.cached_non_scaling_stroke_path_ = *path;
rare_data.cached_non_scaling_stroke_path_.Transform(stroke_transform);
rare_data.cached_non_scaling_stroke_transform_ = stroke_transform;
return &rare_data.cached_non_scaling_stroke_path_;
}
void LayoutSVGShape::UpdateNonScalingStrokeData() {
DCHECK(HasNonScalingStroke());
AffineTransform LayoutSVGShape::NonScalingStrokeTransform() const {
// Compute the CTM to the SVG root. This should probably be the CTM all the
// way to the "canvas" of the page ("host" coordinate system), but with our
// current approach of applying/painting non-scaling-stroke, that can break in
......@@ -331,7 +324,15 @@ AffineTransform LayoutSVGShape::NonScalingStrokeTransform() const {
// here.
t.SetE(0);
t.SetF(0);
return t;
auto& rare_data = EnsureRareData();
if (rare_data.non_scaling_stroke_transform_ != t) {
SetShouldDoFullPaintInvalidation(PaintInvalidationReason::kStyle);
rare_data.non_scaling_stroke_transform_ = t;
}
rare_data.non_scaling_stroke_path_ = *path_;
rare_data.non_scaling_stroke_path_.Transform(t);
}
void LayoutSVGShape::Paint(const PaintInfo& paint_info,
......@@ -411,12 +412,11 @@ FloatRect LayoutSVGShape::CalculateStrokeBoundingBox() const {
SVGLayoutSupport::ApplyStrokeStyleToStrokeData(stroke_data, StyleRef(),
*this, DashScaleFactor());
if (HasNonScalingStroke()) {
AffineTransform non_scaling_transform = NonScalingStrokeTransform();
const auto& non_scaling_transform = NonScalingStrokeTransform();
if (non_scaling_transform.IsInvertible()) {
Path* use_path =
NonScalingStrokePath(path_.get(), non_scaling_transform);
const auto& non_scaling_stroke = NonScalingStrokePath();
FloatRect stroke_bounding_rect =
use_path->StrokeBoundingRect(stroke_data);
non_scaling_stroke.StrokeBoundingRect(stroke_data);
stroke_bounding_rect =
non_scaling_transform.Inverse().MapRect(stroke_bounding_rect);
stroke_bounding_box.Unite(stroke_bounding_rect);
......
......@@ -51,8 +51,8 @@ struct LayoutSVGShapeRareData {
public:
LayoutSVGShapeRareData() = default;
Path cached_non_scaling_stroke_path_;
AffineTransform cached_non_scaling_stroke_transform_;
Path non_scaling_stroke_path_;
AffineTransform non_scaling_stroke_transform_;
DISALLOW_COPY_AND_ASSIGN(LayoutSVGShapeRareData);
};
......@@ -88,8 +88,17 @@ class LayoutSVGShape : public LayoutSVGModelObject {
bool HasNonScalingStroke() const {
return Style()->SvgStyle().VectorEffect() == VE_NON_SCALING_STROKE;
}
Path* NonScalingStrokePath(const Path*, const AffineTransform&) const;
AffineTransform NonScalingStrokeTransform() const;
const Path& NonScalingStrokePath() const {
DCHECK(HasNonScalingStroke());
DCHECK(rare_data_);
return rare_data_->non_scaling_stroke_path_;
}
const AffineTransform& NonScalingStrokeTransform() const {
DCHECK(HasNonScalingStroke());
DCHECK(rare_data_);
return rare_data_->non_scaling_stroke_transform_;
}
AffineTransform LocalSVGTransform() const final { return local_transform_; }
virtual const Vector<MarkerPosition>* MarkerPositions() const {
......@@ -158,6 +167,7 @@ class LayoutSVGShape : public LayoutSVGModelObject {
FloatRect StrokeBoundingBox() const final { return stroke_bounding_box_; }
FloatRect CalculateObjectBoundingBox() const;
FloatRect CalculateStrokeBoundingBox() const;
void UpdateNonScalingStrokeData();
bool UpdateLocalTransform();
private:
......
......@@ -190,10 +190,9 @@ void SVGShapePainter::StrokeShape(GraphicsContext& context,
break;
default:
DCHECK(layout_svg_shape_.HasPath());
Path* use_path = &layout_svg_shape_.GetPath();
const Path* use_path = &layout_svg_shape_.GetPath();
if (layout_svg_shape_.HasNonScalingStroke())
use_path = layout_svg_shape_.NonScalingStrokePath(
use_path, layout_svg_shape_.NonScalingStrokeTransform());
use_path = &layout_svg_shape_.NonScalingStrokePath();
context.DrawPath(use_path->GetSkPath(), flags);
}
}
......
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