Auto merge of #17581 - mbrubeck:incremental, r=pcwalton

Fix incremental layout bugs for absolute and cleared elements

Fixes #17307.  <del>There were two underlying bugs:</del>

* <del>Nodes were not marked dirty when their style attribute changed.</del>

* BaseFlow flags set during flow construction could get out of sync with the element's style if repair_style was called.

---

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #17307
- [x] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17581)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-06-30 22:15:26 -07:00 committed by GitHub
commit ece35f9caa
8 changed files with 97 additions and 6 deletions

2
Cargo.lock generated
View file

@ -1415,7 +1415,7 @@ version = "0.0.1"
dependencies = [ dependencies = [
"app_units 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", "app_units 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)",
"atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
"bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "bitflags 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)",
"canvas_traits 0.0.1", "canvas_traits 0.0.1",
"euclid 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)",
"fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)", "fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)",

View file

@ -12,7 +12,7 @@ path = "lib.rs"
[dependencies] [dependencies]
app_units = "0.5" app_units = "0.5"
atomic_refcell = "0.1" atomic_refcell = "0.1"
bitflags = "0.7" bitflags = "0.8"
canvas_traits = {path = "../canvas_traits"} canvas_traits = {path = "../canvas_traits"}
euclid = "0.15" euclid = "0.15"
fnv = "1.0" fnv = "1.0"

View file

@ -1115,6 +1115,28 @@ impl BaseFlow {
} }
} }
/// Update the 'flags' field when computed styles have changed.
///
/// These flags are initially set during flow construction. They only need to be updated here
/// if they are based on properties that can change without triggering `RECONSTRUCT_FLOW`.
pub fn update_flags_if_needed(&mut self, style: &ServoComputedValues) {
// For absolutely-positioned flows, changes to top/bottom/left/right can cause these flags
// to get out of date:
if self.restyle_damage.contains(REFLOW_OUT_OF_FLOW) {
// Note: We don't need to check whether IS_ABSOLUTELY_POSITIONED has changed, because
// changes to the 'position' property trigger flow reconstruction.
if self.flags.contains(IS_ABSOLUTELY_POSITIONED) {
let logical_position = style.logical_position();
self.flags.set(INLINE_POSITION_IS_STATIC,
logical_position.inline_start == LengthOrPercentageOrAuto::Auto &&
logical_position.inline_end == LengthOrPercentageOrAuto::Auto);
self.flags.set(BLOCK_POSITION_IS_STATIC,
logical_position.block_start == LengthOrPercentageOrAuto::Auto &&
logical_position.block_end == LengthOrPercentageOrAuto::Auto);
}
}
}
/// Return a new BaseFlow like this one but with the given children list /// Return a new BaseFlow like this one but with the given children list
pub fn clone_with_children(&self, children: FlowList) -> BaseFlow { pub fn clone_with_children(&self, children: FlowList) -> BaseFlow {
BaseFlow { BaseFlow {
@ -1361,6 +1383,7 @@ impl<'a> MutableFlowUtils for &'a mut Flow {
/// calling them individually, since there is no reason not to perform both operations. /// calling them individually, since there is no reason not to perform both operations.
fn repair_style_and_bubble_inline_sizes(self, style: &::StyleArc<ServoComputedValues>) { fn repair_style_and_bubble_inline_sizes(self, style: &::StyleArc<ServoComputedValues>) {
self.repair_style(style); self.repair_style(style);
mut_base(self).update_flags_if_needed(style);
self.bubble_inline_sizes(); self.bubble_inline_sizes();
} }

View file

@ -202,7 +202,7 @@ fn compute_damage(old: &ServoComputedValues, new: &ServoComputedValues) -> Servo
add_if_not_equal!(old, new, damage, add_if_not_equal!(old, new, damage,
[REPAINT, REPOSITION, STORE_OVERFLOW, BUBBLE_ISIZES, REFLOW_OUT_OF_FLOW, [REPAINT, REPOSITION, STORE_OVERFLOW, BUBBLE_ISIZES, REFLOW_OUT_OF_FLOW,
REFLOW, RECONSTRUCT_FLOW], [ REFLOW, RECONSTRUCT_FLOW], [
get_box.float, get_box.display, get_box.position, get_counters.content, get_box.clear, get_box.float, get_box.display, get_box.position, get_counters.content,
get_counters.counter_reset, get_counters.counter_increment, get_counters.counter_reset, get_counters.counter_increment,
get_inheritedbox._servo_under_display_none, get_inheritedbox._servo_under_display_none,
get_list.quotes, get_list.list_style_type, get_list.quotes, get_list.list_style_type,

View file

@ -1,3 +0,0 @@
[dynamic-top-change-001.htm]
type: reftest
expected: FAIL

View file

@ -2435,6 +2435,18 @@
{} {}
] ]
], ],
"css/incremental_position.html": [
[
"/_mozilla/css/incremental_position.html",
[
[
"/_mozilla/css/incremental_position_ref.html",
"=="
]
],
{}
]
],
"css/incremental_text_color_a.html": [ "css/incremental_text_color_a.html": [
[ [
"/_mozilla/css/incremental_text_color_a.html", "/_mozilla/css/incremental_text_color_a.html",
@ -7911,6 +7923,11 @@
{} {}
] ]
], ],
"css/incremental_position_ref.html": [
[
{}
]
],
"css/incremental_text_color_ref.html": [ "css/incremental_text_color_ref.html": [
[ [
{} {}
@ -22808,6 +22825,14 @@
"3235e9662e37dc723364a1dd39e07157868dd290", "3235e9662e37dc723364a1dd39e07157868dd290",
"support" "support"
], ],
"css/incremental_position.html": [
"849f3f5ae1df8ecf3bcf3f1723b5a755afeb3316",
"reftest"
],
"css/incremental_position_ref.html": [
"67ab51b29fb015b82bf98942e2e886b3de5ea2cd",
"support"
],
"css/incremental_text_color_a.html": [ "css/incremental_text_color_a.html": [
"1846e2691b150cd2eb0b126c6d8f68f47d683fab", "1846e2691b150cd2eb0b126c6d8f68f47d683fab",
"reftest" "reftest"

View file

@ -0,0 +1,27 @@
<!doctype html>
<html class="reftest-wait">
<head>
<meta charset="utf-8">
<title>Incremental absolute position test</title>
<link rel="match" href="incremental_position_ref.html">
<style>
#rect {
position: absolute;
border: 1px solid red;
width: 40px;
height: 40px;
}
</style>
</head>
<body>
<div id="rect"></div>
<script>
window.onload = function() {
document.body.offsetWidth; // force layout
var e = document.getElementById("rect");
e.style.left = "100px";
document.documentElement.classList.remove('reftest-wait');
};
</script>
</body>
</html>

View file

@ -0,0 +1,19 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>Incremental absolute position reference</title>
<style>
#rect {
position: absolute;
border: 1px solid red;
width: 40px;
height: 40px;
left: 100px;
}
</style>
</head>
<body>
<div id="rect"></div>
</body>
</html>