Skip to content

Commit

Permalink
Auto merge of #12839 - notriddle:11818_sequential_layout_bug, r=<try>
Browse files Browse the repository at this point in the history
Fix a cached style cascade bug that only manifested in sequential mode

When copying cached styles, keep the `writing_mode` up to date.

---

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #11818 (github issue number if applicable).
- [...] There are tests for these changes

NOTE TO THE REVIEWER: This PR contains a test that works, but I need to run the test in sequential mode (couldn't figure out how to make it show up in parallel mode). What do I need to do to make the test harness pass `-y1` to Servo?

<!-- 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/12839)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Aug 13, 2016
2 parents 8828728 + ff0ead5 commit 561d114
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 18 deletions.
4 changes: 2 additions & 2 deletions components/constellation/pipeline.rs
Expand Up @@ -162,7 +162,7 @@ impl Pipeline {
pipeline_port: pipeline_port,
layout_to_constellation_chan: state.layout_to_constellation_chan.clone(),
content_process_shutdown_chan: layout_content_process_shutdown_chan.clone(),
layout_threads: opts::get().layout_threads,
layout_threads: PREFS.get("layout.threads").as_u64().unwrap() as usize,
};

if let Err(e) = script_chan.send(ConstellationControlMsg::AttachLayout(new_layout_info)) {
Expand Down Expand Up @@ -474,7 +474,7 @@ impl UnprivilegedPipelineContent {
self.mem_profiler_chan,
self.layout_content_process_shutdown_chan,
self.webrender_api_sender,
opts::get().layout_threads);
self.prefs.get("layout.threads").unwrap().value().as_u64().unwrap() as usize);

if wait_for_completion {
let _ = self.script_content_process_shutdown_port.recv();
Expand Down
3 changes: 3 additions & 0 deletions components/style/properties/properties.mako.rs
Expand Up @@ -1716,6 +1716,9 @@ fn cascade_with_cached_declarations(
context.mutate_style().mutate_font().compute_font_hash();
}

let mode = get_writing_mode(context.style.get_inheritedbox());
context.style.set_writing_mode(mode);

context.style
}

Expand Down
29 changes: 16 additions & 13 deletions components/util/opts.rs
Expand Up @@ -55,10 +55,6 @@ pub struct Opts {
/// and cause it to produce output on that interval (`-m`).
pub mem_profiler_period: Option<f64>,

/// The number of threads to use for layout (`-y`). Defaults to 1, which results in a recursive
/// sequential algorithm.
pub layout_threads: usize,

pub nonincremental_layout: bool,

/// Where to load userscripts from, if any. An empty string will load from
Expand Down Expand Up @@ -481,7 +477,6 @@ pub fn default_opts() -> Opts {
time_profiling: None,
time_profiler_trace_path: None,
mem_profiler_period: None,
layout_threads: 1,
nonincremental_layout: false,
userscripts: None,
user_stylesheets: Vec::new(),
Expand Down Expand Up @@ -671,7 +666,7 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
Ok(interval) => Some(OutputOptions::Stdout(interval)) ,
Err(_) => Some(OutputOptions::FileName(argument)),
},
None => Some(OutputOptions::Stdout(5 as f64)),
None => Some(OutputOptions::Stdout(5.0 as f64)),
}
} else {
// if the p option doesn't exist:
Expand All @@ -691,11 +686,11 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
period.parse().unwrap_or_else(|err| args_fail(&format!("Error parsing option: -m ({})", err)))
});

let mut layout_threads: usize = match opt_match.opt_str("y") {
Some(layout_threads_str) => layout_threads_str.parse()
.unwrap_or_else(|err| args_fail(&format!("Error parsing option: -y ({})", err))),
None => cmp::max(num_cpus::get() * 3 / 4, 1),
};
let mut layout_threads: Option<usize> = opt_match.opt_str("y")
.map(|layout_threads_str| {
layout_threads_str.parse()
.unwrap_or_else(|err| args_fail(&format!("Error parsing option: -y ({})", err)))
});

let nonincremental_layout = opt_match.opt_present("i");

Expand All @@ -714,7 +709,7 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
let mut bubble_inline_sizes_separately = debug_options.bubble_widths;
if debug_options.trace_layout {
paint_threads = 1;
layout_threads = 1;
layout_threads = Some(1);
bubble_inline_sizes_separately = true;
}

Expand Down Expand Up @@ -786,7 +781,6 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
time_profiling: time_profiling,
time_profiler_trace_path: opt_match.opt_str("profiler-trace-path"),
mem_profiler_period: mem_profiler_period,
layout_threads: layout_threads,
nonincremental_layout: nonincremental_layout,
userscripts: opt_match.opt_default("userscripts", ""),
user_stylesheets: user_stylesheets,
Expand Down Expand Up @@ -855,6 +849,15 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
};
}

if let Some(layout_threads) = layout_threads {
PREFS.set("layout.threads", PrefValue::Number(layout_threads as f64));
} else if let Some(layout_threads) = PREFS.get("layout.threads").as_string() {
PREFS.set("layout.threads", PrefValue::Number(layout_threads.parse::<f64>().unwrap()));
} else if *PREFS.get("layout.threads") == PrefValue::Missing {
let layout_threads = cmp::max(num_cpus::get() * 3 / 4, 1);
PREFS.set("layout.threads", PrefValue::Number(layout_threads as f64));
}

ArgumentParsingResult::ChromeProcess
}

Expand Down
1 change: 0 additions & 1 deletion ports/cef/core.rs
Expand Up @@ -69,7 +69,6 @@ pub extern "C" fn cef_initialize(args: *const cef_main_args_t,

let mut temp_opts = opts::default_opts();
temp_opts.paint_threads = rendering_threads;
temp_opts.layout_threads = rendering_threads;
temp_opts.headless = false;
temp_opts.hard_fail = false;
temp_opts.enable_text_antialiasing = true;
Expand Down
4 changes: 2 additions & 2 deletions tests/wpt/harness/wptrunner/executors/executorservo.py
Expand Up @@ -211,8 +211,8 @@ def screenshot(self, test, viewport_size, dpi):
for stylesheet in self.browser.user_stylesheets:
command += ["--user-stylesheet", stylesheet]

for pref in test.environment.get('prefs', {}):
command += ["--pref", pref]
for pref, value in test.environment.get('prefs', {}).iteritems():
command += ["--pref", "%s=%s" % (pref, value)]

if viewport_size:
command += ["--resolution", viewport_size]
Expand Down
24 changes: 24 additions & 0 deletions tests/wpt/mozilla/meta/MANIFEST.json
Expand Up @@ -1344,6 +1344,18 @@
"url": "/_mozilla/css/data_img_a.html"
}
],
"css/direction_style_caching.html": [
{
"path": "css/direction_style_caching.html",
"references": [
[
"/_mozilla/css/direction_style_caching_ref.html",
"=="
]
],
"url": "/_mozilla/css/direction_style_caching.html"
}
],
"css/empty_cells_a.html": [
{
"path": "css/empty_cells_a.html",
Expand Down Expand Up @@ -10552,6 +10564,18 @@
"url": "/_mozilla/css/data_img_a.html"
}
],
"css/direction_style_caching.html": [
{
"path": "css/direction_style_caching.html",
"references": [
[
"/_mozilla/css/direction_style_caching_ref.html",
"=="
]
],
"url": "/_mozilla/css/direction_style_caching.html"
}
],
"css/empty_cells_a.html": [
{
"path": "css/empty_cells_a.html",
Expand Down
3 changes: 3 additions & 0 deletions tests/wpt/mozilla/meta/css/direction_style_caching.html.ini
@@ -0,0 +1,3 @@
prefs: [layout.threads:1]
[direction_style_caching.html]
type: reftest
28 changes: 28 additions & 0 deletions tests/wpt/mozilla/tests/css/direction_style_caching.html
@@ -0,0 +1,28 @@
<!doctype html>
<meta charset="utf-8">
<title>Style caching with directions</title>
<link rel="match" href="direction_style_caching_ref.html">
<style>
.wrapper {
width: 200px;
}
p { margin: 0 }
.a p, .b p {
width: 100px;
height: 1px;
background: red;
}
.b {
direction: rtl;
}
</style>
<div class=wrapper>
<div class=a><p></p></div>
<div class=a><p></p></div>
<div class=a><p></p></div>
<div class=a><p></p></div>
<div class=b><p></p></div>
<div class=b><p></p></div>
<div class=b><p></p></div>
<div class=b><p></p></div>
</div>
19 changes: 19 additions & 0 deletions tests/wpt/mozilla/tests/css/direction_style_caching_ref.html
@@ -0,0 +1,19 @@
<!DOCTYPE html>
<style>
.wrapper {
width: 200px;
}
p { margin: 0 }
.a p, .b p {
width: 100px;
height: 4px;
background: red;
}
.b p {
margin-left: 100px;
}
</style>
<div class=wrapper>
<div class=a><p></p></div>
<div class=b><p></p></div>
</div>

0 comments on commit 561d114

Please sign in to comment.