Skip to content

Commit

Permalink
layout: Rewrite Servo's vertical-align support to match CSS 2.1 §
Browse files Browse the repository at this point in the history
10.8, and implement `vertical-align: middle` per CSS 2.1 § 10.8.1.

`InlineMetrics` has been split into `InlineMetrics` for fragments and
`LineMetrics` for lines. Both structures' fields have been renamed in
order to more clearly delineate the difference between *space* and
*content*. Vertical positioning of fragments has been reworked to take
margins and borders into account only for replaced content.

This patch fixes the `vertical_align_super_a.html` reftest. Servo now
matches the rendering that Gecko and WebKit produce.

Additionally, this includes a test for the popular inline-block
centering technique described here:
https://s.codepen.io/shshaw/fullpage/gEiDt?#Inline-Block
  • Loading branch information
pcwalton committed Oct 11, 2016
1 parent 56b41fa commit 773614f
Show file tree
Hide file tree
Showing 35 changed files with 524 additions and 391 deletions.
11 changes: 3 additions & 8 deletions components/layout/construct.rs
Expand Up @@ -476,14 +476,9 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
{
// FIXME(#6503): Use Arc::get_mut().unwrap() here.
let inline_flow = flow_ref::deref_mut(&mut inline_flow_ref).as_mut_inline();


let (ascent, descent) =
inline_flow.compute_minimum_ascent_and_descent(&mut self.layout_context
.font_context(),
&node.style(self.style_context()));
inline_flow.minimum_block_size_above_baseline = ascent;
inline_flow.minimum_depth_below_baseline = descent;
inline_flow.minimum_line_metrics =
inline_flow.minimum_line_metrics(&mut self.layout_context.font_context(),
&node.style(self.style_context()))
}

inline_flow_ref.finish();
Expand Down
2 changes: 1 addition & 1 deletion components/layout/flow.rs
Expand Up @@ -1576,7 +1576,7 @@ impl ContainingBlockLink {
if flow.is_block_like() {
flow.as_block().explicit_block_containing_size(shared_context)
} else if flow.is_inline_flow() {
Some(flow.as_inline().minimum_block_size_above_baseline)
Some(flow.as_inline().minimum_line_metrics.space_above_baseline)
} else {
None
}
Expand Down
223 changes: 168 additions & 55 deletions components/layout/fragment.rs

Large diffs are not rendered by default.

435 changes: 193 additions & 242 deletions components/layout/inline.rs

Large diffs are not rendered by default.

22 changes: 10 additions & 12 deletions components/layout/list_item.rs
Expand Up @@ -18,14 +18,13 @@ use fragment::{CoordinateSystem, Fragment, FragmentBorderBoxIterator, GeneratedC
use fragment::Overflow;
use generated_content;
use gfx::display_list::StackingContext;
use inline::InlineMetrics;
use inline::InlineFlow;
use script_layout_interface::restyle_damage::RESOLVE_GENERATED_CONTENT;
use std::sync::Arc;
use style::computed_values::{list_style_type, position};
use style::context::SharedStyleContext;
use style::logical_geometry::LogicalSize;
use style::properties::ServoComputedValues;
use text;

/// A block with the CSS `display` property equal to `list-item`.
#[derive(Debug)]
Expand Down Expand Up @@ -105,21 +104,20 @@ impl Flow for ListItemFlow {
fn assign_block_size<'a>(&mut self, layout_context: &'a LayoutContext<'a>) {
self.block_flow.assign_block_size(layout_context);

// FIXME(pcwalton): Do this during flow construction, like `InlineFlow` does?
let marker_line_metrics =
InlineFlow::minimum_line_metrics_for_fragments(&self.marker_fragments,
&mut layout_context.font_context(),
&*self.block_flow.fragment.style);
for marker in &mut self.marker_fragments {
let containing_block_block_size =
self.block_flow.base.block_container_explicit_block_size;
marker.assign_replaced_block_size_if_necessary(containing_block_block_size);

let font_metrics =
text::font_metrics_for_style(&mut layout_context.font_context(),
marker.style.get_font_arc());
let line_height = text::line_height_from_style(&*marker.style, &font_metrics);
let item_inline_metrics = InlineMetrics::from_font_metrics(&font_metrics, line_height);
let marker_inline_metrics = marker.inline_metrics(layout_context);
marker.border_box.start.b = item_inline_metrics.block_size_above_baseline -
let marker_inline_metrics = marker.aligned_inline_metrics(layout_context,
&marker_line_metrics,
Some(&marker_line_metrics));
marker.border_box.start.b = marker_line_metrics.space_above_baseline -
marker_inline_metrics.ascent;
marker.border_box.size.block = marker_inline_metrics.ascent +
marker_inline_metrics.depth_below_baseline;
}
}

Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

48 changes: 48 additions & 0 deletions tests/wpt/mozilla/meta/MANIFEST.json
Expand Up @@ -2528,6 +2528,18 @@
"url": "/_mozilla/css/inline_block_border_intrinsic_size_a.html"
}
],
"css/inline_block_centering_a.html": [
{
"path": "css/inline_block_centering_a.html",
"references": [
[
"/_mozilla/css/inline_block_centering_ref.html",
"=="
]
],
"url": "/_mozilla/css/inline_block_centering_a.html"
}
],
"css/inline_block_height_with_out_of_flow_child_a.html": [
{
"path": "css/inline_block_height_with_out_of_flow_child_a.html",
Expand Down Expand Up @@ -5688,6 +5700,18 @@
"url": "/_mozilla/css/vertical_align_inside_table_a.html"
}
],
"css/vertical_align_middle_a.html": [
{
"path": "css/vertical_align_middle_a.html",
"references": [
[
"/_mozilla/css/vertical_align_middle_ref.html",
"=="
]
],
"url": "/_mozilla/css/vertical_align_middle_a.html"
}
],
"css/vertical_align_sub_a.html": [
{
"path": "css/vertical_align_sub_a.html",
Expand Down Expand Up @@ -16286,6 +16310,18 @@
"url": "/_mozilla/css/inline_block_border_intrinsic_size_a.html"
}
],
"css/inline_block_centering_a.html": [
{
"path": "css/inline_block_centering_a.html",
"references": [
[
"/_mozilla/css/inline_block_centering_ref.html",
"=="
]
],
"url": "/_mozilla/css/inline_block_centering_a.html"
}
],
"css/inline_block_height_with_out_of_flow_child_a.html": [
{
"path": "css/inline_block_height_with_out_of_flow_child_a.html",
Expand Down Expand Up @@ -19446,6 +19482,18 @@
"url": "/_mozilla/css/vertical_align_inside_table_a.html"
}
],
"css/vertical_align_middle_a.html": [
{
"path": "css/vertical_align_middle_a.html",
"references": [
[
"/_mozilla/css/vertical_align_middle_ref.html",
"=="
]
],
"url": "/_mozilla/css/vertical_align_middle_a.html"
}
],
"css/vertical_align_sub_a.html": [
{
"path": "css/vertical_align_sub_a.html",
Expand Down
33 changes: 33 additions & 0 deletions tests/wpt/mozilla/tests/css/inline_block_centering_a.html
@@ -0,0 +1,33 @@
<!DOCTYPE html>
<meta charset="utf-8">
<link rel="match" href="inline_block_centering_ref.html">
<!--
Tests that the popular inline-block centering technique works.
See: https://s.codepen.io/shshaw/fullpage/gEiDt?#Inline-Block
-->
<style>
div {
width: 100px;
}
#a {
background: blue;
position: absolute;
height: 100px;
top: 0;
left: 0;
}
#a:after, #b {
display: inline-block;
vertical-align: middle;
}
#a:after {
content: '';
height: 100%;
}
#b {
background: green;
height: 50px;
}
</style>
<div id=a><div id=b>
26 changes: 26 additions & 0 deletions tests/wpt/mozilla/tests/css/inline_block_centering_ref.html
@@ -0,0 +1,26 @@
<!DOCTYPE html>
<meta charset="utf-8">
<!--
Tests that the popular inline-block centering technique works.
See: https://s.codepen.io/shshaw/fullpage/gEiDt?#Inline-Block
-->
<style>
div {
position: absolute;
width: 100px;
left: 0;
}
#a {
background: blue;
height: 100px;
top: 0;
}
#b {
background: green;
height: 50px;
top: 25px;
}
</style>
<div id=a><div id=b>

14 changes: 14 additions & 0 deletions tests/wpt/mozilla/tests/css/vertical_align_middle_a.html
@@ -0,0 +1,14 @@
<!DOCTYPE html>
<meta charset="utf-8">
<link rel="match" href="vertical_align_middle_ref.html">
<style>
html, body {
margin: 0;
}
img {
vertical-align: middle;
}
</style>
<img width=50 height=50 src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVQImWNgYPj/HwADAgH/xCAAOgAAAABJRU5ErkJggg=="><!--
--><img width=100 height=100 src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVQImWNgaGD4DwAChAGA2FJdiQAAAABJRU5ErkJggg==">

27 changes: 27 additions & 0 deletions tests/wpt/mozilla/tests/css/vertical_align_middle_ref.html
@@ -0,0 +1,27 @@
<!DOCTYPE html>
<meta charset="utf-8">
<style>
html, body {
margin: 0;
}
div {
position: absolute;
}
#a {
left: 0;
top: 25px;
width: 50px;
height: 50px;
background: blue;
}
#b {
left: 50px;
top: 0;
width: 100px;
height: 100px;
background: green;
}
</style>
<div id=a></div>
<div id=b></div>

2 changes: 1 addition & 1 deletion tests/wpt/mozilla/tests/css/vertical_align_super_a.html
Expand Up @@ -11,7 +11,7 @@
}
div {
color: blue;
margin-top: 100px;
margin-top: 82px;
}
.align {
color: red;
Expand Down

0 comments on commit 773614f

Please sign in to comment.