Skip to content

GitLab

T
tangled
Commit 7fab23cf authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot
Browse files
Options

[PE] LayoutObject::PhysicalOutlineRects() 

Previously when painting outlines we used LayoutObject::AddOutlineRects()
which adds outline rects in flipped blocks direction. The previous
flipping code in ObjectPainter::PaintOutline() was incomplete, and we
also missed the flipping in other places.

Add LayoutObject::PhysicalOutlineRects() which flips the outline rects
correctly.

Remove internals.outlineRects and internals.focusRingRects and convert
the layout tests using them into unit tests.

Bug: 910643

Change-Id: Ib8b3e0c7cd646e60378c09955ba26334f0f7f9e9
Reviewed-on: https://chromium-review.googlesource.com/c/1352503Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612786}
parent a4e4bb73
Hide whitespace changes
Inline Side-by-side
Showing with 199 additions and 138 deletions
third_party/blink/renderer/core/html/anchor_element_metrics.cc
View file @ 7fab23cf
......@@ -126,8 +126,8 @@ bool IsUrlIncrementedByOne(const HTMLAnchorElement& anchor_element) {
// overflows.
IntRect AbsoluteElementBoundingBoxRect(const LayoutObject* layout_object) {
Vector<LayoutRect> rects;
layout_object->AddElementVisualOverflowRects(rects, LayoutPoint());
layout_object->AddOutlineRects(rects, LayoutPoint(),
NGOutlineType::kIncludeBlockVisualOverflow);
return layout_object
->LocalToAbsoluteQuad(FloatQuad(FloatRect(UnionRect(rects))))
.EnclosingBoundingBox();
......
third_party/blink/renderer/core/layout/layout_inline_test.cc
View file @ 7fab23cf
......@@ -4,6 +4,7 @@
#include "third_party/blink/renderer/core/layout/layout_inline.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/renderer/core/layout/layout_block_flow.h"
#include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
......@@ -12,6 +13,8 @@
namespace blink {
using ::testing::UnorderedElementsAre;
class LayoutInlineTest : public RenderingTest {};
// Helper class to run the same test code with and without LayoutNG
......@@ -273,4 +276,78 @@ TEST_P(ParameterizedLayoutInlineTest, VisualRectInDocument) {
EXPECT_EQ(visual_rect.Height(), LayoutUnit(222 + 20 * 2));
}
// When adding focus ring rects, we should avoid adding duplicated rect for
// continuations.
TEST_P(ParameterizedLayoutInlineTest, FocusRingRecursiveContinuations) {
// TODO(crbug.com/835484): The test is broken for LayoutNG.
if (RuntimeEnabledFeatures::LayoutNGEnabled())
return;
LoadAhem();
SetBodyInnerHTML(R"HTML(
<style>
body {
margin: 0;
font: 20px/20px Ahem;
}
</style>
<span id="target">SPAN0
<div>DIV1
<span>SPAN1
<div>DIV2</div>
</span>
</div>
</span>
)HTML");
Vector<LayoutRect> rects;
GetLayoutObjectByElementId("target")->AddOutlineRects(
rects, LayoutPoint(), NGOutlineType::kIncludeBlockVisualOverflow);
EXPECT_THAT(rects,
UnorderedElementsAre(LayoutRect(0, 0, 100, 20), // 'SPAN0'
LayoutRect(0, 20, 800, 40), // div DIV1
LayoutRect(0, 20, 200, 20), // 'DIV1 SPAN1'
LayoutRect(0, 40, 800, 20), // div DIV2
LayoutRect(0, 40, 80, 20))); // 'DIV2'
}
// When adding focus ring rects, we should avoid adding line box rects of
// recursive inlines repeatedly.
TEST_P(ParameterizedLayoutInlineTest, FocusRingRecursiveInlines) {
// TODO(crbug.com/835484): The test is broken for LayoutNG.
if (RuntimeEnabledFeatures::LayoutNGEnabled())
return;
LoadAhem();
SetBodyInnerHTML(R"HTML(
<style>
body {
margin: 0;
font: 20px/20px Ahem;
}
</style>
<div style="width: 200px">
<span id="target">
<b><b><b><i><i><i>INLINE</i></i> <i><i>TEXT</i></i>
<div style="position: relative; top: -5px">
<b><b>BLOCK</b> <i>CONTENTS</i></b>
</div>
</i></b></b></b>
</span>
</div>
)HTML");
Vector<LayoutRect> rects;
GetLayoutObjectByElementId("target")->AddOutlineRects(
rects, LayoutPoint(), NGOutlineType::kIncludeBlockVisualOverflow);
EXPECT_THAT(rects,
UnorderedElementsAre(LayoutRect(0, 0, 120, 20), // 'INLINE'
LayoutRect(0, 20, 80, 20), // 'TEXT'
LayoutRect(0, 35, 200, 40), // the inner div
LayoutRect(0, 35, 100, 20), // 'BLOCK'
LayoutRect(0, 55, 160, 20))); // 'CONTENTS'
}
} // namespace blink
third_party/blink/renderer/core/layout/layout_object.cc
View file @ 7fab23cf
......@@ -4177,6 +4177,24 @@ LayoutRect LayoutObject::AdjustVisualRectForInlineBox(
return visual_rect;
}
Vector<LayoutRect> LayoutObject::PhysicalOutlineRects(
const LayoutPoint& additional_offset,
NGOutlineType outline_type) const {
Vector<LayoutRect> outline_rects;
AddOutlineRects(outline_rects, additional_offset, outline_type);
if (IsSVGChild() || !HasFlippedBlocksWritingMode())
return outline_rects;
const auto* writing_mode_container =
IsBox() ? ToLayoutBox(this) : ContainingBlock();
for (auto& r : outline_rects) {
r.MoveBy(-additional_offset);
writing_mode_container->FlipForWritingMode(r);
r.MoveBy(additional_offset);
}
return outline_rects;
}
} // namespace blink
#ifndef NDEBUG
......
third_party/blink/renderer/core/layout/layout_object.h
View file @ 7fab23cf
......@@ -1790,6 +1790,9 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
const LayoutPoint& additional_offset,
NGOutlineType) const {}
Vector<LayoutRect> PhysicalOutlineRects(const LayoutPoint& additional_offset,
NGOutlineType) const;
// For history and compatibility reasons, we draw outline:auto (for focus
// rings) and normal style outline differently.
// Focus rings enclose block visual overflows (of line boxes and descendants),
......@@ -1805,19 +1808,6 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
bitfields_.SetContainsInlineWithOutlineAndContinuation(b);
}
// Collects rectangles enclosing visual overflows of the DOM subtree under
// this object.
// The rects also cover continuations which may be not in the layout subtree
// of this object.
// TODO(crbug.com/614781): Currently the result rects don't cover list markers
// and outlines.
void AddElementVisualOverflowRects(
Vector<LayoutRect>& rects,
const LayoutPoint& additional_offset) const {
AddOutlineRects(rects, additional_offset,
NGOutlineType::kIncludeBlockVisualOverflow);
}
// Compute a list of hit-test rectangles per layer rooted at this
// layoutObject with at most the given touch action.
virtual void ComputeLayerHitTestRects(LayerHitTestRects&, TouchAction) const;
......
third_party/blink/renderer/core/paint/block_painter.cc
View file @ 7fab23cf
......@@ -366,8 +366,8 @@ bool BlockPainter::ShouldPaint(const ScopedPaintState& paint_state) const {
// empty, but we need to continue painting to output <a>'s PDF URL rect
// which covers the continuations, as if we included <a>'s PDF URL rect into
// layout_block_'s visual overflow.
Vector<LayoutRect> rects;
layout_block_.AddElementVisualOverflowRects(rects, LayoutPoint());
auto rects = layout_block_.PhysicalOutlineRects(
LayoutPoint(), NGOutlineType::kIncludeBlockVisualOverflow);
overflow_rect = UnionRect(rects);
}
overflow_rect.Unite(layout_block_.VisualOverflowRect());
......
third_party/blink/renderer/core/paint/object_painter.cc
View file @ 7fab23cf
......@@ -35,9 +35,8 @@ void ObjectPainter::PaintOutline(const PaintInfo& paint_info,
return;
}
Vector<LayoutRect> outline_rects;
layout_object_.AddOutlineRects(
outline_rects, paint_offset,
auto outline_rects = layout_object_.PhysicalOutlineRects(
paint_offset,
layout_object_.OutlineRectsShouldIncludeBlockVisualOverflow());
if (outline_rects.IsEmpty())
return;
......@@ -46,18 +45,6 @@ void ObjectPainter::PaintOutline(const PaintInfo& paint_info,
paint_info.context, layout_object_, paint_info.phase))
return;
// The result rects are in coordinates of m_layoutObject's border box.
// Block flipping is not applied yet if !m_layoutObject.isBox().
if (!layout_object_.IsBox() &&
layout_object_.StyleRef().IsFlippedBlocksWritingMode()) {
LayoutBlock* container = layout_object_.ContainingBlock();
if (container) {
layout_object_.LocalToAncestorRects(outline_rects, container,
-paint_offset, paint_offset);
if (outline_rects.IsEmpty())
return;
}
}
DrawingRecorder recorder(paint_info.context, layout_object_,
paint_info.phase);
PaintOutlineRects(paint_info, outline_rects, style_to_use);
......@@ -87,10 +74,9 @@ void ObjectPainter::AddPDFURLRectIfNeeded(const PaintInfo& paint_info,
if (!url.IsValid())
return;
Vector<LayoutRect> visual_overflow_rects;
layout_object_.AddElementVisualOverflowRects(visual_overflow_rects,
paint_offset);
IntRect rect = PixelSnappedIntRect(UnionRect(visual_overflow_rects));
auto outline_rects = layout_object_.PhysicalOutlineRects(
paint_offset, NGOutlineType::kIncludeBlockVisualOverflow);
IntRect rect = PixelSnappedIntRect(UnionRect(outline_rects));
if (rect.IsEmpty())
return;
......
third_party/blink/renderer/core/testing/internals.cc
View file @ 7fab23cf
......@@ -3403,24 +3403,6 @@ String Internals::unscopableMethod() {
return "unscopableMethod";
}
DOMRectList* Internals::focusRingRects(Element* element) {
Vector<LayoutRect> rects;
if (element && element->GetLayoutObject()) {
element->GetLayoutObject()->AddOutlineRects(
rects, LayoutPoint(), NGOutlineType::kIncludeBlockVisualOverflow);
}
return DOMRectList::Create(rects);
}
DOMRectList* Internals::outlineRects(Element* element) {
Vector<LayoutRect> rects;
if (element && element->GetLayoutObject()) {
element->GetLayoutObject()->AddOutlineRects(
rects, LayoutPoint(), NGOutlineType::kDontIncludeBlockVisualOverflow);
}
return DOMRectList::Create(rects);
}
void Internals::setCapsLockState(bool enabled) {
KeyboardEventManager::SetCurrentCapsLockState(
enabled ? OverrideCapsLockState::kOn : OverrideCapsLockState::kOff);
......
third_party/blink/renderer/core/testing/internals.h
View file @ 7fab23cf
......@@ -546,9 +546,6 @@ class Internals final : public ScriptWrappable {
String unscopableAttribute();
String unscopableMethod();
DOMRectList* focusRingRects(Element*);
DOMRectList* outlineRects(Element*);
void setCapsLockState(bool enabled);
bool setScrollbarVisibilityInScrollableArea(Node*, bool visible);
......
third_party/blink/renderer/core/testing/internals.idl
View file @ 7fab23cf
......@@ -371,8 +371,6 @@ enum EffectiveConnectionType {
[Unscopable] readonly attribute DOMString unscopableAttribute;
[Unscopable] DOMString unscopableMethod();
DOMRectList focusRingRects(Element element);
DOMRectList outlineRects(Element element);
void setCapsLockState(boolean enabled);
// Returns whether the scrollbar was able to be shown or hidden; not all platforms
......
third_party/blink/web_tests/FlagExpectations/enable-blink-features=LayoutNG
View file @ 7fab23cf
......@@ -274,7 +274,6 @@ crbug.com/591099 fast/css-intrinsic-dimensions/height-positioned.html [ Pass ]
crbug.com/807708 fast/css-intrinsic-dimensions/width-avoid-floats.html [ Failure ]
crbug.com/591099 fast/css/absolute-inline-alignment-2.html [ Pass ]
crbug.com/591099 fast/css/css-properties-position-relative-as-parent-fixed.html [ Failure ]
crbug.com/835484 fast/css/focus-ring-recursive-continuations.html [ Failure ]
crbug.com/835484 fast/css/outline-narrowLine.html [ Failure ]
crbug.com/855279 fast/css/text-overflow-ellipsis-vertical-hittest.html [ Failure Pass ]
crbug.com/591099 fast/css/transform-inline-style-remove.html [ Failure ]
......
third_party/blink/web_tests/fast/css/focus-ring-recursive-continuations-expected.txt deleted 100644 → 0
View file @ a4e4bb73
When adding focus ring rects, we should avoid adding duplicated rect for continuations
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS successfullyParsed is true
TEST COMPLETE
PASS internals.focusRingRects(document.getElementById('focus')).length is 5
SPAN0
DIV1 SPAN1
DIV2
third_party/blink/web_tests/fast/css/focus-ring-recursive-continuations.html deleted 100644 → 0
View file @ a4e4bb73
<!DOCTYPE html>
<script src="../../resources/js-test.js"></script>
<script>
description('When adding focus ring rects, we should avoid adding duplicated rect for continuations');
onload = function() {
document.body.offsetTop;
// 6 focus ring rects:
// - 0: 'SPAN0' part of the span;
// - 1: div DIV1
// - 2: first line box of div DIV1
// - 3: div DIV2
// - 4: first line box of div DIV2
if (window.testRunner && window.internals)
shouldBe("internals.focusRingRects(document.getElementById('focus')).length", "5");
};
</script>
<div>
<span id="focus">SPAN0
<div>DIV1
<span>SPAN1
<div>DIV2</div>
</span>
</div>
</span>
</div>
third_party/blink/web_tests/fast/css/focus-ring-recursive-inlines-expected.txt deleted 100644 → 0
View file @ a4e4bb73
When adding focus ring rects, we should avoid adding line box rects of recursive inlines repeatedly
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS successfullyParsed is true
TEST COMPLETE
PASS internals.focusRingRects(document.getElementById('focus')).length is 5
INLINE TEXT
BLOCK CONTENTS
third_party/blink/web_tests/fast/css/focus-ring-recursive-inlines.html deleted 100644 → 0
View file @ a4e4bb73
<!DOCTYPE html>
<style>
#container {
width: 200px;
}
#inner {
position: relative;
top: -3px;
}
</style>
<script src="../../resources/js-test.js"></script>
<script>
description('When adding focus ring rects, we should avoid adding line box rects of recursive inlines repeatedly');
onload = function() {
document.body.offsetTop;
// 5 focus ring rects:
// - 0-2: line boxes of the focused span;
// - 3: the inner block
// - 4: root line box of the inner block
if (window.testRunner && window.internals)
shouldBe("internals.focusRingRects(document.getElementById('focus')).length", "5");
};
</script>
<div id="container">
<span id="focus">
<b><b><b><i><i><i>INLINE</i></i> <i><i>TEXT</i></i>
<div id="inner"><b><b>BLOCK</b> <i>CONTENTS</i></b></div>
</i></b></b></b>
</span>
</div>
third_party/blink/web_tests/fast/css/focus-ring-vrl-overflow-expected.html 0 → 100644
View file @ 7fab23cf
<!DOCTYPE html>
The focus ring should tightly enclose the blue rectangle and the green rectangle.
<div style="width: 200px">
<div style="background: blue; outline: auto; width: 100px; height: 100px; margin-left: 100px">
<div style="margin-left: -100px; width: 200px; height: 50px; background: green"></div>
</div>
</div>
third_party/blink/web_tests/fast/css/focus-ring-vrl-overflow.html 0 → 100644
View file @ 7fab23cf
<!DOCTYPE html>
The focus ring should tightly enclose the blue rectangle and the green rectangle.
<div style="writing-mode: vertical-rl; width: 200px">
<div style="background: blue; outline: auto; width: 100px; height: 100px">
<div style="width: 200px; height: 50px; background: green"></div>
</div>
</div>
third_party/blink/web_tests/svg/custom/focus-ring-vrl-expected.svg 0 → 100644
View file @ 7fab23cf
<svg width="800" height="600" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
<style>
.ring {
outline: -webkit-focus-ring-color 5px auto;
}
text {
writing-mode: vertical-rl;
}
</style>
<circle cx="65" cy="60" r="50" fill="orange" class="ring" />
<path d="M 140 10 l 50 100 l 50 -50 l -20 0 l -20 -30 Z" fill="orange" class="ring" />
<g class="ring">
<rect x="270" y="10" width="100" height="100" fill="orange" />
</g>
<g transform="translate(60, 140) rotate(30)">
<rect width="100" height="100" fill="orange" class="ring" />
</g>
<rect width="100" height="100" fill="orange" transform="translate(200, 140) rotate(30)" class="ring" />
<g class="ring">
<rect width="100" height="100" fill="orange" transform="translate(360, 140) rotate(30)" />
</g>
<text x="10" y="320" class="ring">focused text</text>
<text transform="translate(100, 320) rotate(30)" class="ring">focused text</text>
<g class="ring">
<text transform="translate(200, 320) rotate(30)" class="ring">focused text</text>
</g>
<clipPath id="clip">
<rect width="100" height="100" />
</clipPath>
<image xlink:href="resources/green-checker.png" x="10" y="400" width="530" height="410" clip-path="url(#clip)" class="ring" />
<image xlink:href="resources/green-checker.png" transform="translate(200, 400) rotate(30)" width="530" height="410" class="ring" clip-path="url(#clip)"/>
<g class="ring" transform="translate(360, 400) rotate(30)">
<image xlink:href="resources/green-checker.png" width="530" height="410" clip-path="url(#clip)"/>
</g>
</svg>
third_party/blink/web_tests/svg/custom/focus-ring-vrl.svg 0 → 100644
View file @ 7fab23cf
<svg width="800" height="600" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"
style="writing-mode: vertical-rl">
<style>
.ring {
outline: -webkit-focus-ring-color 5px auto;
}
</style>
<circle cx="65" cy="60" r="50" fill="orange" class="ring" />
<path d="M 140 10 l 50 100 l 50 -50 l -20 0 l -20 -30 Z" fill="orange" class="ring" />
<g class="ring">
<rect x="270" y="10" width="100" height="100" fill="orange" />
</g>
<g transform="translate(60, 140) rotate(30)">
<rect width="100" height="100" fill="orange" class="ring" />
</g>
<rect width="100" height="100" fill="orange" transform="translate(200, 140) rotate(30)" class="ring" />
<g class="ring">
<rect width="100" height="100" fill="orange" transform="translate(360, 140) rotate(30)" />
</g>
<text x="10" y="320" class="ring">focused text</text>
<text transform="translate(100, 320) rotate(30)" class="ring">focused text</text>
<g class="ring">
<text transform="translate(200, 320) rotate(30)" class="ring">focused text</text>
</g>
<clipPath id="clip">
<rect width="100" height="100" />
</clipPath>
<image xlink:href="resources/green-checker.png" x="10" y="400" width="530" height="410" clip-path="url(#clip)" class="ring" />
<image xlink:href="resources/green-checker.png" transform="translate(200, 400) rotate(30)" width="530" height="410" class="ring" clip-path="url(#clip)"/>
<g class="ring" transform="translate(360, 400) rotate(30)">
<image xlink:href="resources/green-checker.png" width="530" height="410" clip-path="url(#clip)"/>
</g>
</svg>
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