From fb5952fed09eac96f2fa8709fb751c7ef73a5723 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 26 Sep 2014 20:34:22 -0700 Subject: [PATCH] layout: Don't double-count padding, etc, and don't draw backgrounds/borders/etc. twice for inline-blocks. Improves Reddit, GitHub, and many other sites. --- components/layout/fragment.rs | 79 ++++++++++++++++++-------- tests/ref/basic.list | 1 + tests/ref/inline_block_border_a.html | 7 +++ tests/ref/inline_block_border_ref.html | 8 +++ 4 files changed, 71 insertions(+), 24 deletions(-) create mode 100644 tests/ref/inline_block_border_a.html create mode 100644 tests/ref/inline_block_border_ref.html diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index 455941e40ba..8b177ee94a6 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -706,6 +706,11 @@ impl Fragment { /// Returns true if this element can be split. This is true for text fragments. pub fn can_split(&self) -> bool { + self.is_scanned_text_fragment() + } + + /// Returns true if and only if this is a scanned text fragment. + fn is_scanned_text_fragment(&self) -> bool { match self.specific { ScannedTextFragment(..) => true, _ => false, @@ -990,7 +995,7 @@ impl Fragment { debug!("Fragment::build_display_list: intersected. Adding display item..."); - { + if self.is_primary_fragment() { let level = StackingLevel::from_background_and_border_level(background_and_border_level); @@ -1002,11 +1007,12 @@ impl Fragment { match self.inline_context { Some(ref inline_context) => { for style in inline_context.styles.iter().rev() { - self.build_display_list_for_background_if_applicable(&**style, - display_list, - layout_context, - level, - &absolute_fragment_bounds); + self.build_display_list_for_background_if_applicable( + &**style, + display_list, + layout_context, + level, + &absolute_fragment_bounds); } } None => {} @@ -1197,13 +1203,12 @@ impl Fragment { } /// Returns the intrinsic inline-sizes of this fragment. - pub fn intrinsic_inline_sizes(&mut self) - -> IntrinsicISizes { + pub fn intrinsic_inline_sizes(&mut self) -> IntrinsicISizes { let mut result = self.style_specified_intrinsic_inline_size(); - let use_border_padding = match self.specific { - GenericFragment | IframeFragment(_) | TableFragment | TableCellFragment | TableColumnFragment(_) | TableRowFragment | - TableWrapperFragment => { true } + match self.specific { + GenericFragment | IframeFragment(_) | TableFragment | TableCellFragment | + TableColumnFragment(_) | TableRowFragment | TableWrapperFragment => {} InlineBlockFragment(ref mut info) => { let block_flow = info.flow_ref.get_mut().as_block(); result.minimum_inline_size = max(result.minimum_inline_size, @@ -1212,13 +1217,12 @@ impl Fragment { result.preferred_inline_size = max(result.preferred_inline_size, block_flow.base.intrinsic_inline_sizes.preferred_inline_size + block_flow.base.intrinsic_inline_sizes.surround_inline_size); - false }, ImageFragment(ref mut image_fragment_info) => { let image_inline_size = image_fragment_info.image_inline_size(); result.minimum_inline_size = max(result.minimum_inline_size, image_inline_size); - result.preferred_inline_size = max(result.preferred_inline_size, image_inline_size); - true + result.preferred_inline_size = max(result.preferred_inline_size, + image_inline_size); } ScannedTextFragment(ref text_fragment_info) => { let range = &text_fragment_info.range; @@ -1226,25 +1230,33 @@ impl Fragment { // See http://dev.w3.org/csswg/css-sizing/#max-content-inline-size. // TODO: Account for soft wrap opportunities. - let max_line_inline_size = text_fragment_info.run.metrics_for_range(range).advance_width; + let max_line_inline_size = text_fragment_info.run + .metrics_for_range(range) + .advance_width; - result.minimum_inline_size = max(result.minimum_inline_size, min_line_inline_size); - result.preferred_inline_size = max(result.preferred_inline_size, max_line_inline_size); - true + result.minimum_inline_size = max(result.minimum_inline_size, + min_line_inline_size); + result.preferred_inline_size = max(result.preferred_inline_size, + max_line_inline_size); + } + UnscannedTextFragment(..) => { + fail!("Unscanned text fragments should have been scanned by now!") } - UnscannedTextFragment(..) => fail!("Unscanned text fragments should have been scanned by now!"), }; // Take borders and padding for parent inline fragments into account, if necessary. - if use_border_padding { + if self.is_primary_fragment() { match self.inline_context { None => {} Some(ref context) => { for style in context.styles.iter() { let border_width = style.logical_border_width().inline_start_end(); - let padding_inline_size = model::padding_from_style(&**style, Au(0)).inline_start_end(); - result.minimum_inline_size = result.minimum_inline_size + border_width + padding_inline_size; - result.preferred_inline_size = result.preferred_inline_size + border_width + padding_inline_size; + let padding_inline_size = + model::padding_from_style(&**style, Au(0)).inline_start_end(); + result.minimum_inline_size = result.minimum_inline_size + border_width + + padding_inline_size; + result.preferred_inline_size = result.preferred_inline_size + + border_width + padding_inline_size; } } } @@ -1581,8 +1593,9 @@ impl Fragment { self.border_box.size.block = self.border_box.size.block + noncontent_block_size } InlineBlockFragment(ref mut info) => { + // Not the primary fragment, so we do not take the noncontent size into account. let block_flow = info.flow_ref.get_mut().as_block(); - self.border_box.size.block = block_flow.base.position.size.block + noncontent_block_size; + self.border_box.size.block = block_flow.base.position.size.block; } _ => fail!("should have been handled above"), } @@ -1660,6 +1673,24 @@ impl Fragment { let ConstellationChan(ref chan) = layout_context.shared.constellation_chan; chan.send(msg) } + + /// Returns true if and only if this is the *primary fragment* for the fragment's style object + /// (conceptually, though style sharing makes this not really true, of course). The primary + /// fragment is the one that draws backgrounds, borders, etc., and takes borders, padding and + /// margins into account. Every style object has at most one primary fragment. + /// + /// At present, all fragments are primary fragments except for inline-block and table wrapper + /// fragments. Inline-block fragments are not primary fragments because the corresponding block + /// flow is the primary fragment, while table wrapper fragments are not primary fragments + /// because the corresponding table flow is the primary fragment. + fn is_primary_fragment(&self) -> bool { + match self.specific { + InlineBlockFragment(_) | TableWrapperFragment => false, + GenericFragment | IframeFragment(_) | ImageFragment(_) | ScannedTextFragment(_) | + TableFragment | TableCellFragment | TableColumnFragment(_) | TableRowFragment | + UnscannedTextFragment(_) => true, + } + } } impl fmt::Show for Fragment { diff --git a/tests/ref/basic.list b/tests/ref/basic.list index 8a2654dac02..406fbbf6283 100644 --- a/tests/ref/basic.list +++ b/tests/ref/basic.list @@ -150,3 +150,4 @@ flaky_linux == acid2_noscroll.html acid2_ref_broken.html == block_formatting_context_complex_a.html block_formatting_context_complex_ref.html == block_formatting_context_containing_floats_a.html block_formatting_context_containing_floats_ref.html == clear_generated_content_table_a.html clear_generated_content_table_ref.html +== inline_block_border_a.html inline_block_border_ref.html diff --git a/tests/ref/inline_block_border_a.html b/tests/ref/inline_block_border_a.html new file mode 100644 index 00000000000..2a5626112d6 --- /dev/null +++ b/tests/ref/inline_block_border_a.html @@ -0,0 +1,7 @@ + + + +
+ + + diff --git a/tests/ref/inline_block_border_ref.html b/tests/ref/inline_block_border_ref.html new file mode 100644 index 00000000000..b76c09fb113 --- /dev/null +++ b/tests/ref/inline_block_border_ref.html @@ -0,0 +1,8 @@ + + + +
+ + + +