Skip to content

Commit

Permalink
[SPv175] Fix hittesting of <foreignObject> under non-identity SVG-rel…
Browse files Browse the repository at this point in the history
…ated transform

If the SVG to border box transform or any other transform in SVG above <foreignObject>
is non-identity, then hit testing of the <foreignObject> and content below it is incorrect,
because it does not take into account such transforms.

Before SPv175 + <foreignObject> as stacking context, content underneath the
<foreignObject> did not paint with the correct sizing either, which hid this bug to
some extent.

To make this work requires a little bit of special-casing of foreignObject, because of
the quirk that its location offset is applied *after* transform, not before.

Bug: 820482,831591
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ided2b46f2ea12dd61c254892f5fbabb821f6229e
Reviewed-on: https://chromium-review.googlesource.com/974568
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550825}
  • Loading branch information
chrishtr authored and Commit Bot committed Apr 14, 2018
1 parent 1db8a28 commit b19cea8
Show file tree
Hide file tree
Showing 15 changed files with 196 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@
{
"object": "LayoutSVGForeignObject foreignObject",
"rect": [108, 8, 100, 100],
"reason": "appeared"
"reason": "paint property change"
},
{
"object": "LayoutSVGForeignObject foreignObject",
"rect": [8, 8, 100, 100],
"reason": "disappeared"
"reason": "paint property change"
}
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@
{
"object": "LayoutSVGForeignObject foreignObject",
"rect": [108, 8, 100, 100],
"reason": "appeared"
"reason": "paint property change"
},
{
"object": "LayoutSVGForeignObject foreignObject",
"rect": [8, 8, 100, 100],
"reason": "disappeared"
"reason": "paint property change"
}
]
}
Expand Down
8 changes: 8 additions & 0 deletions third_party/blink/renderer/core/layout/layout_object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2598,6 +2598,14 @@ void LayoutObject::GetTransformFromContainer(
if (layer && layer->Transform())
transform.Multiply(layer->CurrentTransform());

GetTransformFromContainerInternal(container_object, offset_in_container,
transform);
}

void LayoutObject::GetTransformFromContainerInternal(
const LayoutObject* container_object,
const LayoutSize& offset_in_container,
TransformationMatrix& transform) const {
transform.PostTranslate(offset_in_container.Width().ToFloat(),
offset_in_container.Height().ToFloat());

Expand Down
13 changes: 9 additions & 4 deletions third_party/blink/renderer/core/layout/layout_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -1621,10 +1621,11 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
const LayoutBoxModelObject* ancestor_to_stop_at,
LayoutGeometryMap&) const;

bool ShouldUseTransformFromContainer(const LayoutObject* container) const;
void GetTransformFromContainer(const LayoutObject* container,
const LayoutSize& offset_in_container,
TransformationMatrix&) const;
virtual bool ShouldUseTransformFromContainer(
const LayoutObject* container) const;
virtual void GetTransformFromContainer(const LayoutObject* container,
const LayoutSize& offset_in_container,
TransformationMatrix&) const;

bool CreatesGroup() const {
return IsTransparent() || HasMask() || HasClipPath() ||
Expand Down Expand Up @@ -2054,6 +2055,10 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
// changes at all).
virtual bool AnonymousHasStylePropagationOverride() { return false; }

void GetTransformFromContainerInternal(const LayoutObject* container,
const LayoutSize& offset_in_container,
TransformationMatrix&) const;

// A fast path for MapToVisualRectInAncestorSpace for when GeometryMapper
// can be used.
bool MapToVisualRectInAncestorSpaceInternalFastPath(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "third_party/blink/renderer/core/layout/hit_test_result.h"
#include "third_party/blink/renderer/core/layout/layout_analyzer.h"
#include "third_party/blink/renderer/core/layout/layout_box_model_object.h"
#include "third_party/blink/renderer/core/layout/svg/svg_layout_support.h"
#include "third_party/blink/renderer/core/layout/svg/svg_resources.h"
#include "third_party/blink/renderer/core/layout/svg/svg_resources_cache.h"
Expand Down Expand Up @@ -191,6 +192,9 @@ bool LayoutSVGContainer::NodeAtFloatPoint(HitTestResult& result,

for (LayoutObject* child = LastChild(); child;
child = child->PreviousSibling()) {
if (child->IsBoxModelObject() &&
ToLayoutBoxModelObject(child)->HasSelfPaintingLayer())
continue;
if (child->NodeAtFloatPoint(result, local_point, hit_test_action)) {
const LayoutPoint& local_layout_point = LayoutPoint(local_point);
UpdateHitTestResult(result, local_layout_point);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,10 @@ void LayoutSVGForeignObject::UpdateLayout() {
bool LayoutSVGForeignObject::NodeAtFloatPoint(HitTestResult& result,
const FloatPoint& point_in_parent,
HitTestAction hit_test_action) {
// Embedded content is drawn in the foreground phase.
if (hit_test_action != kHitTestForeground)
if (RuntimeEnabledFeatures::SlimmingPaintV175Enabled()) {
NOTREACHED();
return false;

}
AffineTransform local_transform = LocalSVGTransform();
if (!local_transform.IsInvertible())
return false;
Expand All @@ -152,14 +152,24 @@ bool LayoutSVGForeignObject::NodeAtFloatPoint(HitTestResult& result,
kHitTestChildBlockBackgrounds);
}

void LayoutSVGForeignObject::GetTransformFromContainer(
const LayoutObject* container,
const LayoutSize& offset_in_container,
TransformationMatrix& matrix) const {
AffineTransform to_svg_root_transform;
SVGLayoutSupport::ComputeTransformToSVGRoot(*this, to_svg_root_transform);
matrix = to_svg_root_transform;
GetTransformFromContainerInternal(container, offset_in_container, matrix);
}

bool LayoutSVGForeignObject::NodeAtPoint(
HitTestResult& result,
const HitTestLocation& location_in_parent,
const LayoutPoint& accumulated_offset,
HitTestAction hit_test_action) {
if (RuntimeEnabledFeatures::SlimmingPaintV175Enabled()) {
return NodeAtFloatPoint(result, FloatPoint(accumulated_offset),
hit_test_action);
return LayoutBlock::NodeAtPoint(result, location_in_parent,
accumulated_offset, hit_test_action);
}
NOTREACHED();
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@ class LayoutSVGForeignObject final : public LayoutSVGBlock {
return ObjectBoundingBox();
}

bool ShouldUseTransformFromContainer(
const LayoutObject* container) const override {
return true;
}
void GetTransformFromContainer(const LayoutObject* container,
const LayoutSize& offset_in_container,
TransformationMatrix&) const override;

bool NodeAtPoint(HitTestResult&,
const HitTestLocation&,
const LayoutPoint&,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ TEST_F(LayoutSVGForeignObjectTest, DivInForeignObject) {
)HTML");

const auto& svg = *GetDocument().getElementById("svg");
const auto& foreign = *GetDocument().getElementById("foreign");
const auto& foreign_object = *GetLayoutObjectByElementId("foreign");
const auto& div = *GetLayoutObjectByElementId("div");

Expand Down Expand Up @@ -69,10 +70,12 @@ TEST_F(LayoutSVGForeignObjectTest, DivInForeignObject) {
rgm.MapToAncestor(FloatRect(0, 0, 1, 2), nullptr));

// Hit testing
EXPECT_EQ(svg, HitTest(149, 149));
EXPECT_EQ(svg, HitTest(1, 1));
EXPECT_EQ(foreign, HitTest(149, 149));
EXPECT_EQ(div.GetNode(), HitTest(150, 150));
EXPECT_EQ(div.GetNode(), HitTest(349, 249));
EXPECT_EQ(svg, HitTest(350, 250));
EXPECT_EQ(foreign, HitTest(350, 250));
EXPECT_EQ(svg, HitTest(450, 350));
}

TEST_F(LayoutSVGForeignObjectTest, IframeInForeignObject) {
Expand All @@ -95,6 +98,7 @@ TEST_F(LayoutSVGForeignObjectTest, IframeInForeignObject) {
GetDocument().View()->UpdateAllLifecyclePhases();

const auto& svg = *GetDocument().getElementById("svg");
const auto& foreign = *GetDocument().getElementById("foreign");
const auto& foreign_object = *GetLayoutObjectByElementId("foreign");
const auto& div = *ChildDocument().getElementById("div")->GetLayoutObject();

Expand Down Expand Up @@ -130,14 +134,99 @@ TEST_F(LayoutSVGForeignObjectTest, IframeInForeignObject) {
rgm.MapToAncestor(FloatRect(0, 0, 1, 2), nullptr));

// Hit testing
EXPECT_EQ(svg, HitTest(129, 129));
EXPECT_EQ(svg, HitTest(90, 90));
EXPECT_EQ(foreign, HitTest(129, 129));
EXPECT_EQ(ChildDocument().documentElement(), HitTest(130, 130));
EXPECT_EQ(ChildDocument().documentElement(), HitTest(199, 199));
EXPECT_EQ(div.GetNode(), HitTest(200, 200));
EXPECT_EQ(div.GetNode(), HitTest(299, 249));
EXPECT_EQ(ChildDocument().documentElement(), HitTest(300, 250));
EXPECT_EQ(ChildDocument().documentElement(), HitTest(369, 319));
EXPECT_EQ(svg, HitTest(370, 320));
EXPECT_EQ(foreign, HitTest(370, 320));
EXPECT_EQ(svg, HitTest(450, 400));
}

TEST_F(LayoutSVGForeignObjectTest, HitTestZoomedForeignObject) {
SetBodyInnerHTML(R"HTML(
<style>* { margin: 0; zoom: 150% }</style>
<svg id='svg' style='width: 200px; height: 200px'>
<foreignObject id='foreign' x='10' y='10' width='100' height='150'>
<div id='div' style='margin: 50px; width: 50px; height: 50px'>
</div>
</foreignObject>
</svg>
)HTML");

const auto& svg = *GetDocument().getElementById("svg");
const auto& foreign = *GetDocument().getElementById("foreign");
const auto& foreign_object = *GetLayoutObjectByElementId("foreign");
const auto& div = *GetDocument().getElementById("div");

EXPECT_EQ(FloatRect(10, 10, 100, 150), foreign_object.ObjectBoundingBox());
EXPECT_EQ(AffineTransform(), foreign_object.LocalSVGTransform());
EXPECT_EQ(AffineTransform(), foreign_object.LocalToSVGParentTransform());

// mapToVisualRectInAncestorSpace
LayoutRect div_rect(0, 0, 100, 50);
EXPECT_TRUE(div.GetLayoutObject()->MapToVisualRectInAncestorSpace(
&GetLayoutView(), div_rect));
EXPECT_EQ(LayoutRect(286, 286, 339, 170), div_rect);

// mapLocalToAncestor
TransformState transform_state(TransformState::kApplyTransformDirection,
FloatPoint());
div.GetLayoutObject()->MapLocalToAncestor(&GetLayoutView(), transform_state,
kTraverseDocumentBoundaries);
transform_state.Flatten();
EXPECT_EQ(FloatPoint(286.875, 286.875), transform_state.LastPlanarPoint());

// mapAncestorToLocal
TransformState transform_state1(
TransformState::kUnapplyInverseTransformDirection,
FloatPoint(286.875, 286.875));
div.GetLayoutObject()->MapAncestorToLocal(&GetLayoutView(), transform_state1,
kTraverseDocumentBoundaries);
transform_state1.Flatten();
EXPECT_EQ(FloatPoint(), transform_state1.LastPlanarPoint());

EXPECT_EQ(svg, HitTest(20, 20));
EXPECT_EQ(foreign, HitTest(280, 280));
EXPECT_EQ(div, HitTest(290, 290));
}

TEST_F(LayoutSVGForeignObjectTest, HitTestViewBoxForeignObject) {
SetBodyInnerHTML(R"HTML(
<svg id='svg' style='width: 200px; height: 200px' viewBox='0 0 100 100'>
<foreignObject id='foreign' x='10' y='10' width='100' height='150'>
<div id='div' style='margin: 50px; width: 50px; height: 50px'>
</div>
</foreignObject>
</svg>
)HTML");

const auto& svg = *GetDocument().getElementById("svg");
const auto& foreign = *GetDocument().getElementById("foreign");
const auto& div = *GetDocument().getElementById("div");

// mapLocalToAncestor
TransformState transform_state(TransformState::kApplyTransformDirection,
FloatPoint());
div.GetLayoutObject()->MapLocalToAncestor(&GetLayoutView(), transform_state,
kTraverseDocumentBoundaries);
transform_state.Flatten();
EXPECT_EQ(FloatPoint(128, 128), transform_state.LastPlanarPoint());

// mapAncestorToLocal
TransformState transform_state1(
TransformState::kUnapplyInverseTransformDirection, FloatPoint(128, 128));
div.GetLayoutObject()->MapAncestorToLocal(&GetLayoutView(), transform_state1,
kTraverseDocumentBoundaries);
transform_state1.Flatten();
EXPECT_EQ(FloatPoint(), transform_state1.LastPlanarPoint());

EXPECT_EQ(svg, HitTest(20, 20));
EXPECT_EQ(foreign, HitTest(120, 110));
EXPECT_EQ(div, HitTest(160, 160));
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "third_party/blink/renderer/core/dom/element_traversal.h"
#include "third_party/blink/renderer/core/layout/hit_test_result.h"
#include "third_party/blink/renderer/core/layout/layout_box_model_object.h"
#include "third_party/blink/renderer/core/layout/svg/svg_layout_support.h"
#include "third_party/blink/renderer/core/paint/paint_info.h"
#include "third_party/blink/renderer/core/svg/svg_geometry_element.h"
Expand Down Expand Up @@ -243,6 +244,11 @@ bool LayoutSVGResourceClipper::HitTestClipContent(
IntPoint hit_point;
HitTestResult result(HitTestRequest::kSVGClipContent, hit_point);
LayoutObject* layout_object = child_element.GetLayoutObject();

if (layout_object->IsBoxModelObject() &&
ToLayoutBoxModelObject(layout_object)->HasSelfPaintingLayer())
continue;

if (layout_object->NodeAtFloatPoint(result, point, kHitTestForeground))
return true;
}
Expand Down
3 changes: 3 additions & 0 deletions third_party/blink/renderer/core/layout/svg/layout_svg_root.cc
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,9 @@ bool LayoutSVGRoot::NodeAtPoint(HitTestResult& result,

for (LayoutObject* child = LastChild(); child;
child = child->PreviousSibling()) {
if (child->IsBoxModelObject() &&
ToLayoutBoxModelObject(child)->HasSelfPaintingLayer())
continue;
// FIXME: nodeAtFloatPoint() doesn't handle rect-based hit tests yet.
if (child->NodeAtFloatPoint(result, local_point, hit_test_action)) {
UpdateHitTestResult(result, point_in_border_box);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ LayoutRect SVGLayoutSupport::TransformVisualRect(
return LayoutRect(EnclosingIntRect(adjusted_rect));
}

static const LayoutSVGRoot& ComputeTransformToSVGRoot(
const LayoutSVGRoot& SVGLayoutSupport::ComputeTransformToSVGRoot(
const LayoutObject& object,
AffineTransform& root_border_box_transform) {
DCHECK(object.IsSVGChild());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,10 @@ class CORE_EXPORT SVGLayoutSupport {
static LayoutObject* FindClosestLayoutSVGText(const LayoutObject*,
const FloatPoint&);

static const LayoutSVGRoot& ComputeTransformToSVGRoot(
const LayoutObject& object,
AffineTransform& root_border_box_transform);

private:
static void UpdateObjectBoundingBox(FloatRect& object_bounding_box,
bool& object_bounding_box_valid,
Expand Down
Loading

0 comments on commit b19cea8

Please sign in to comment.