Skip to content

Commit

Permalink
layout: Centralize the logic that determines whether fragments get
Browse files Browse the repository at this point in the history
layers in the fragment, so that it can be activated when we're forcing
the creation of extra layers due to positioned descendants that
themselves have layers.

The newly failing tests were tests that accidentally passed due to
incorrect stacking order.

Closes servo#7281.
  • Loading branch information
pcwalton committed Aug 22, 2015
1 parent d48f6ff commit ddf598f
Show file tree
Hide file tree
Showing 14 changed files with 166 additions and 66 deletions.
60 changes: 43 additions & 17 deletions components/layout/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,17 @@ use context::LayoutContext;
use display_list_builder::{BlockFlowDisplayListBuilding, BorderPaintingMode};
use display_list_builder::{FragmentDisplayListBuilding};
use floats::{ClearType, FloatKind, Floats, PlacementInfo};
use flow::{BLOCK_POSITION_IS_STATIC, HAS_LEFT_FLOATED_DESCENDANTS, HAS_RIGHT_FLOATED_DESCENDANTS};
use flow::{BLOCK_POSITION_IS_STATIC};
use flow::{CLEARS_LEFT, CLEARS_RIGHT};
use flow::{HAS_LEFT_FLOATED_DESCENDANTS, HAS_RIGHT_FLOATED_DESCENDANTS};
use flow::{IMPACTED_BY_LEFT_FLOATS, IMPACTED_BY_RIGHT_FLOATS, INLINE_POSITION_IS_STATIC};
use flow::{IS_ABSOLUTELY_POSITIONED};
use flow::{ImmutableFlowUtils, MutableFlowUtils, OpaqueFlow, PreorderFlowTraversal};
use flow::{LAYERS_NEEDED_FOR_DESCENDANTS, NEEDS_LAYER};
use flow::{PostorderFlowTraversal, mut_base};
use flow::{self, AbsolutePositionInfo, BaseFlow, ForceNonfloatedFlag, FlowClass, Flow};
use fragment::{CoordinateSystem, Fragment, FragmentBorderBoxIterator, SpecificFragmentInfo};
use fragment::{CoordinateSystem, Fragment, FragmentBorderBoxIterator, HAS_LAYER};
use fragment::{SpecificFragmentInfo};
use incremental::{REFLOW, REFLOW_OUT_OF_FLOW};
use layout_debug;
use layout_task::DISPLAY_PORT_SIZE_FACTOR;
Expand Down Expand Up @@ -787,6 +789,8 @@ impl BlockFlow {
self.base.debug_id());

if self.base.restyle_damage.contains(REFLOW) {
self.determine_if_layer_needed();

// Our current border-box position.
let mut cur_b = Au(0);

Expand Down Expand Up @@ -950,11 +954,6 @@ impl BlockFlow {
}

if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) {
// Fixed position layers get layers.
if self.is_fixed() {
self.base.flags.insert(NEEDS_LAYER);
}

// Store the content block-size for use in calculating the absolute flow's
// dimensions later.
//
Expand Down Expand Up @@ -1564,6 +1563,36 @@ impl BlockFlow {
}
self.base.flags = flags
}

fn determine_if_layer_needed(&mut self) {
if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) {
// Fixed position layers get layers.
if self.is_fixed() {
self.base.flags.insert(NEEDS_LAYER);
return
}
}

// This flow needs a layer if it has a 3d transform, or provides perspective
// to child layers. See http://dev.w3.org/csswg/css-transforms/#3d-rendering-contexts.
let has_3d_transform = self.transform_requires_layer();
let has_perspective = self.fragment.style().get_effects().perspective !=
LengthOrNone::None;

if has_3d_transform || has_perspective {
self.base.flags.insert(NEEDS_LAYER);
return
}

match (self.fragment.style().get_box().overflow_x,
self.fragment.style().get_box().overflow_y.0) {
(overflow_x::T::auto, _) | (overflow_x::T::scroll, _) |
(_, overflow_x::T::auto) | (_, overflow_x::T::scroll) => {
self.base.flags.insert(NEEDS_LAYER);
}
_ => {}
}
}
}

impl Flow for BlockFlow {
Expand Down Expand Up @@ -1745,6 +1774,12 @@ impl Flow for BlockFlow {
}

fn compute_absolute_position(&mut self, layout_context: &LayoutContext) {
if (self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) &&
self.base.absolute_position_info.layers_needed_for_positioned_flows) ||
self.base.flags.contains(NEEDS_LAYER) {
self.fragment.flags.insert(HAS_LAYER)
}

// FIXME (mbrubeck): Get the real container size, taking the container writing mode into
// account. Must handle vertical writing modes.
let container_size = Size2D::new(self.base.block_container_inline_size, Au(0));
Expand All @@ -1754,15 +1789,7 @@ impl Flow for BlockFlow {
self.base.stacking_relative_position_of_display_port = MAX_RECT;
}

// This flow needs a layer if it has a 3d transform, or provides perspective
// to child layers. See http://dev.w3.org/csswg/css-transforms/#3d-rendering-contexts.
let transform_style = self.fragment.style().get_used_transform_style();
let has_3d_transform = self.transform_requires_layer();
let has_perspective = self.fragment.style().get_effects().perspective != LengthOrNone::None;

if has_3d_transform || has_perspective {
self.base.flags.insert(NEEDS_LAYER);
}

if self.base.flags.contains(IS_ABSOLUTELY_POSITIONED) {
// `overflow: auto` and `overflow: scroll` force creation of layers, since we can only
Expand All @@ -1771,7 +1798,6 @@ impl Flow for BlockFlow {
self.fragment.style().get_box().overflow_y.0) {
(overflow_x::T::auto, _) | (overflow_x::T::scroll, _) |
(_, overflow_x::T::auto) | (_, overflow_x::T::scroll) => {
self.base.flags.insert(NEEDS_LAYER);
self.base.clip = ClippingRegion::max();
self.base.stacking_relative_position_of_display_port = MAX_RECT;
}
Expand Down Expand Up @@ -1886,7 +1912,7 @@ impl Flow for BlockFlow {
}

let stacking_relative_position_of_display_port_for_children =
if (is_stacking_context && self.will_get_layer()) || self.is_root() {
if is_stacking_context || self.is_root() {
let visible_rect =
match layout_context.shared.visible_rects.get(&self.layer_id(0)) {
Some(visible_rect) => *visible_rect,
Expand Down
87 changes: 38 additions & 49 deletions components/layout/display_list_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use context::LayoutContext;
use flex::FlexFlow;
use flow::{self, BaseFlow, Flow, IS_ABSOLUTELY_POSITIONED, NEEDS_LAYER};
use flow_ref;
use fragment::{CoordinateSystem, Fragment, IframeFragmentInfo, ImageFragmentInfo};
use fragment::{CoordinateSystem, Fragment, HAS_LAYER, IframeFragmentInfo, ImageFragmentInfo};
use fragment::{ScannedTextFragmentInfo, SpecificFragmentInfo};
use inline::InlineFlow;
use list_item::ListItemFlow;
Expand Down Expand Up @@ -1513,7 +1513,6 @@ pub trait BlockFlowDisplayListBuilding {
display_list: Box<DisplayList>,
layout_context: &LayoutContext,
border_painting_mode: BorderPaintingMode);
fn will_get_layer(&self) -> bool;
}

impl BlockFlowDisplayListBuilding for BlockFlow {
Expand Down Expand Up @@ -1557,35 +1556,32 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
border_painting_mode,
background_border_level);

self.base.display_list_building_result = if self.fragment.establishes_stacking_context() {
if self.will_get_layer() {
// If we got here, then we need a new layer.
let scroll_policy = if self.is_fixed() {
ScrollPolicy::FixedPosition
} else {
ScrollPolicy::Scrollable
};
self.base.display_list_building_result = if self.fragment.flags.contains(HAS_LAYER) {
let scroll_policy = if self.is_fixed() {
ScrollPolicy::FixedPosition
} else {
ScrollPolicy::Scrollable
};

let paint_layer = PaintLayer::new(self.layer_id(0),
color::transparent(),
scroll_policy);
let layer = StackingContextLayer::Existing(paint_layer);
let stacking_context = self.fragment.create_stacking_context(
let paint_layer = PaintLayer::new(self.layer_id(0),
color::transparent(),
scroll_policy);
let layer = StackingContextLayer::Existing(paint_layer);
let stacking_context = self.fragment.create_stacking_context(
&self.base,
display_list,
layout_context,
layer,
StackingContextCreationMode::Normal);
DisplayListBuildingResult::StackingContext(stacking_context)
} else if self.fragment.establishes_stacking_context() {
DisplayListBuildingResult::StackingContext(
self.fragment.create_stacking_context(
&self.base,
display_list,
layout_context,
layer,
StackingContextCreationMode::Normal);
DisplayListBuildingResult::StackingContext(stacking_context)
} else {
DisplayListBuildingResult::StackingContext(
self.fragment.create_stacking_context(
&self.base,
display_list,
layout_context,
StackingContextLayer::IfCanvas(self.layer_id(0)),
StackingContextCreationMode::Normal))
}
StackingContextLayer::IfCanvas(self.layer_id(0)),
StackingContextCreationMode::Normal))
} else {
match self.fragment.style.get_box().position {
position::T::static_ => {}
Expand All @@ -1597,11 +1593,6 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
}
}

fn will_get_layer(&self) -> bool {
self.base.absolute_position_info.layers_needed_for_positioned_flows ||
self.base.flags.contains(NEEDS_LAYER)
}

fn build_display_list_for_absolutely_positioned_block(
&mut self,
mut display_list: Box<DisplayList>,
Expand Down Expand Up @@ -1654,23 +1645,21 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
}
};

if !self.fragment.establishes_stacking_context() {
display_list.form_pseudo_stacking_context_for_positioned_content();
self.base.display_list_building_result =
DisplayListBuildingResult::Normal(display_list);
return
}

if !self.will_get_layer() {
// We didn't need a layer.
self.base.display_list_building_result =
DisplayListBuildingResult::StackingContext(
self.fragment.create_stacking_context(
&self.base,
display_list,
layout_context,
StackingContextLayer::IfCanvas(self.layer_id(0)),
StackingContextCreationMode::Normal));
if !self.fragment.flags.contains(HAS_LAYER) {
if !self.fragment.establishes_stacking_context() {
display_list.form_pseudo_stacking_context_for_positioned_content();
self.base.display_list_building_result =
DisplayListBuildingResult::Normal(display_list);
} else {
self.base.display_list_building_result =
DisplayListBuildingResult::StackingContext(
self.fragment.create_stacking_context(
&self.base,
display_list,
layout_context,
StackingContextLayer::IfCanvas(self.layer_id(0)),
StackingContextCreationMode::Normal));
}
return
}

Expand Down
17 changes: 17 additions & 0 deletions components/layout/fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ pub struct Fragment {
/// The pseudo-element that this fragment represents.
pub pseudo: PseudoElementType<()>,

/// Various flags for this fragment.
pub flags: FragmentFlags,

/// A debug ID that is consistent for the life of this fragment (via transform etc).
pub debug_id: u16,
}
Expand Down Expand Up @@ -761,6 +764,7 @@ impl Fragment {
specific: specific,
inline_context: None,
pseudo: node.get_pseudo_element_type().strip(),
flags: FragmentFlags::empty(),
debug_id: layout_debug::generate_unique_debug_id(),
}
}
Expand All @@ -783,6 +787,7 @@ impl Fragment {
specific: specific,
inline_context: None,
pseudo: pseudo,
flags: FragmentFlags::empty(),
debug_id: layout_debug::generate_unique_debug_id(),
}
}
Expand Down Expand Up @@ -816,6 +821,7 @@ impl Fragment {
specific: info,
inline_context: self.inline_context.clone(),
pseudo: self.pseudo.clone(),
flags: FragmentFlags::empty(),
debug_id: self.debug_id,
}
}
Expand Down Expand Up @@ -1987,6 +1993,9 @@ impl Fragment {

/// Returns true if this fragment establishes a new stacking context and false otherwise.
pub fn establishes_stacking_context(&self) -> bool {
if self.flags.contains(HAS_LAYER) {
return true
}
if self.style().get_effects().opacity != 1.0 {
return true
}
Expand Down Expand Up @@ -2373,3 +2382,11 @@ impl WhitespaceStrippingResult {
}
}
}

bitflags! {
flags FragmentFlags: u8 {
/// Whether this fragment has a layer.
const HAS_LAYER = 0x01,
}
}

1 change: 1 addition & 0 deletions tests/ref/basic.list
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ flaky_cpu == linebreak_simple_a.html linebreak_simple_b.html
# Should be == with expected failure. See #2797
!= overconstrained_block.html overconstrained_block_ref.html
== overflow_auto.html overflow_simple_b.html
== overflow_auto_stacking_order_a.html overflow_auto_stacking_order_ref.html
# Should be ==?
!= overflow_position_abs_inside_normal_a.html overflow_position_abs_inside_normal_b.html
== overflow_position_abs_simple_a.html overflow_position_abs_simple_b.html
Expand Down
22 changes: 22 additions & 0 deletions tests/ref/overflow_auto_stacking_order_a.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!DOCTYPE html>
<style>
div {
width: 100px;
height: 100px;
position: absolute;
top: 0;
left: 0;
}
#a {
background: red;
overflow: auto;
}
#b {
background: green;
top: 0;
left: 0;
}
</style>
<div id=a></div>
<div id=b></div>

18 changes: 18 additions & 0 deletions tests/ref/overflow_auto_stacking_order_ref.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<!DOCTYPE html>
<style>
div {
width: 100px;
height: 100px;
position: absolute;
top: 0;
left: 0;
}
#a {
background: green;
top: 0;
left: 0;
}
</style>
<div id=a></div>


Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[abspos-overflow-010.htm]
type: reftest
expected:
if os == "linux": FAIL
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[abspos-overflow-011.htm]
type: reftest
expected: FAIL
3 changes: 3 additions & 0 deletions tests/wpt/metadata-css/css21_dev/html4/max-height-107.htm.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[max-height-107.htm]
type: reftest
expected: FAIL
3 changes: 3 additions & 0 deletions tests/wpt/metadata-css/css21_dev/html4/max-height-110.htm.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[max-height-110.htm]
type: reftest
expected: FAIL
3 changes: 3 additions & 0 deletions tests/wpt/metadata-css/css21_dev/html4/min-height-104.htm.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[min-height-104.htm]
type: reftest
expected: FAIL
3 changes: 3 additions & 0 deletions tests/wpt/metadata-css/css21_dev/html4/min-height-105.htm.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[min-height-105.htm]
type: reftest
expected: FAIL
3 changes: 3 additions & 0 deletions tests/wpt/metadata-css/css21_dev/html4/min-height-106.htm.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[min-height-106.htm]
type: reftest
expected: FAIL
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[min-width-not-important.html]
type: reftest
reftype: ==
refurl: /html/rendering/non-replaced-elements/the-fieldset-element-0/ref.html
expected: FAIL

0 comments on commit ddf598f

Please sign in to comment.