diff options
author | bors-servo <servo-ops@mozilla.com> | 2020-06-09 13:37:30 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-06-09 13:37:30 -0400 |
commit | cb4e3cb16a43f91566155b25a5fdf26af6de6a92 (patch) | |
tree | d1b1ef2ec0363b85617008dce41d3f0094eeabe9 | |
parent | 83e2e7803d6b58d95fdbd09bc48c6b74f5bf4323 (diff) | |
parent | 2a83b80eedeac315a4a5b07eb30d690a7c077c16 (diff) | |
download | servo-cb4e3cb16a43f91566155b25a5fdf26af6de6a92.tar.gz servo-cb4e3cb16a43f91566155b25a5fdf26af6de6a92.zip |
Auto merge of #26758 - jdm:stacking-context-transform-zero, r=mrobinson
Don't create empty stacking contexts in display lists
A recent change to euclid exposed that our display lists can contain Rects that contain NaN values. These NaNs originate from creating stacking contexts with transforms that scale the horizontal or vertical dimensions to 0. WebRender isn't prepared to handle these, so we need to not produce these empty stacking contexts when building the display list.
---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #26592 and fix #26590
- [x] There are tests for these changes
-rw-r--r-- | components/layout/display_list/builder.rs | 10 | ||||
-rw-r--r-- | components/layout/display_list/items.rs | 6 | ||||
-rw-r--r-- | components/layout/flow.rs | 20 | ||||
-rw-r--r-- | components/layout/fragment.rs | 6 | ||||
-rw-r--r-- | components/layout/inline.rs | 5 | ||||
-rw-r--r-- | components/layout/traversal.rs | 4 | ||||
-rw-r--r-- | components/layout_2020/display_list/stacking_context.rs | 21 | ||||
-rw-r--r-- | tests/wpt/mozilla/meta/MANIFEST.json | 17 | ||||
-rw-r--r-- | tests/wpt/mozilla/tests/css/stacking-context-empty-ref.html | 1 | ||||
-rw-r--r-- | tests/wpt/mozilla/tests/css/stacking-context-empty.html | 14 |
10 files changed, 103 insertions, 1 deletions
diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index 9d5f1e100ab..0be8183e2a7 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -1571,6 +1571,11 @@ impl Fragment { return; } + // If this fragment takes up no space, we don't need to build any display items for it. + if self.has_non_invertible_transform() { + return; + } + debug!( "Fragment::build_display_list at rel={:?}, abs={:?}: {:?}", self.border_box, stacking_relative_border_box, self @@ -2376,6 +2381,11 @@ impl BlockFlow { state: &mut StackingContextCollectionState, flags: StackingContextCollectionFlags, ) { + // This block flow produces no stacking contexts if it takes up no space. + if self.has_non_invertible_transform() { + return; + } + let mut preserved_state = SavedStackingContextCollectionState::new(state); let stacking_context_type = self.stacking_context_type(flags); diff --git a/components/layout/display_list/items.rs b/components/layout/display_list/items.rs index 27b4ec1fa88..4800abe7e53 100644 --- a/components/layout/display_list/items.rs +++ b/components/layout/display_list/items.rs @@ -217,6 +217,12 @@ impl StackingContext { parent_clipping_and_scrolling: ClippingAndScrolling, established_reference_frame: Option<ClipScrollNodeIndex>, ) -> StackingContext { + if let Some(ref t) = transform { + // These are used as scale values by webrender, and it can't handle + // divisors of 0 when scaling. + assert_ne!(t.m11, 0.); + assert_ne!(t.m22, 0.); + } StackingContext { id, context_type, diff --git a/components/layout/flow.rs b/components/layout/flow.rs index c2d01b7f9f9..acea20ad4d4 100644 --- a/components/layout/flow.rs +++ b/components/layout/flow.rs @@ -273,6 +273,22 @@ pub trait Flow: HasBaseFlow + fmt::Debug + Sync + Send + 'static { might_have_floats_in_or_out } + fn has_non_invertible_transform(&self) -> bool { + if !self.class().is_block_like() || + self.as_block() + .fragment + .style + .get_box() + .transform + .0 + .is_empty() + { + return false; + } + + self.as_block().fragment.has_non_invertible_transform() + } + fn get_overflow_in_parent_coordinates(&self) -> Overflow { // FIXME(#2795): Get the real container size. let container_size = Size2D::zero(); @@ -1160,7 +1176,9 @@ impl BaseFlow { state: &mut StackingContextCollectionState, ) { for kid in self.children.iter_mut() { - kid.collect_stacking_contexts(state); + if !kid.has_non_invertible_transform() { + kid.collect_stacking_contexts(state); + } } } diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index 062db9c0a7e..10c0fb072cc 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -2730,6 +2730,12 @@ impl Fragment { self.style().get_box().perspective != Perspective::None } + /// Returns true if this fragment has a transform applied that causes it to take up no space. + pub fn has_non_invertible_transform(&self) -> bool { + self.transform_matrix(&Rect::default()) + .map_or(false, |matrix| !matrix.is_invertible()) + } + /// Returns true if this fragment establishes a new stacking context and false otherwise. pub fn establishes_stacking_context(&self) -> bool { // Text fragments shouldn't create stacking contexts. diff --git a/components/layout/inline.rs b/components/layout/inline.rs index dfdb20e309b..ee9980c4f1a 100644 --- a/components/layout/inline.rs +++ b/components/layout/inline.rs @@ -1871,6 +1871,11 @@ impl Flow for InlineFlow { let previous_cb_clipping_and_scrolling = state.containing_block_clipping_and_scrolling; for fragment in self.fragments.fragments.iter_mut() { + // If a particular fragment would establish a stacking context but has a transform + // applied that causes it to take up no space, we can skip it entirely. + if fragment.has_non_invertible_transform() { + continue; + } state.containing_block_clipping_and_scrolling = previous_cb_clipping_and_scrolling; let abspos_containing_block = fragment.style.get_box().position != Position::Static; diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index 88a6514433b..5c66004bc1a 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -346,6 +346,10 @@ pub struct BuildDisplayList<'a> { impl<'a> BuildDisplayList<'a> { #[inline] pub fn traverse(&mut self, flow: &mut dyn Flow) { + if flow.has_non_invertible_transform() { + return; + } + let parent_stacking_context_id = self.state.current_stacking_context_id; self.state.current_stacking_context_id = flow.base().stacking_context_id; diff --git a/components/layout_2020/display_list/stacking_context.rs b/components/layout_2020/display_list/stacking_context.rs index fce6fb6caec..05c2d7558a9 100644 --- a/components/layout_2020/display_list/stacking_context.rs +++ b/components/layout_2020/display_list/stacking_context.rs @@ -447,6 +447,14 @@ impl Fragment { return; } + // If this fragment has a transform applied that makes it take up no spae + // then we don't need to create any stacking contexts for it. + let has_non_invertible_transform = + fragment.has_non_invertible_transform(&containing_block_info.rect.to_untyped()); + if has_non_invertible_transform { + return; + } + fragment.build_stacking_context_tree( fragment_ref, builder, @@ -775,6 +783,15 @@ impl BoxFragment { }) } + /// Returns true if the given style contains a transform that is not invertible. + fn has_non_invertible_transform(&self, containing_block: &Rect<Length>) -> bool { + let list = &self.style.get_box().transform; + match list.to_transform_3d_matrix(Some(containing_block)) { + Ok(t) => !t.0.is_invertible(), + Err(_) => false, + } + } + /// Returns the 4D matrix representing this fragment's transform. pub fn calculate_transform_matrix( &self, @@ -783,6 +800,10 @@ impl BoxFragment { let list = &self.style.get_box().transform; let transform = LayoutTransform::from_untyped(&list.to_transform_3d_matrix(Some(&border_rect)).ok()?.0); + // WebRender will end up dividing by the scale value of this transform, so we + // want to ensure we don't feed it a divisor of 0. + assert_ne!(transform.m11, 0.); + assert_ne!(transform.m22, 0.); let transform_origin = &self.style.get_box().transform_origin; let transform_origin_x = transform_origin diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 5ce021feee6..47ba62031be 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -6080,6 +6080,19 @@ {} ] ], + "stacking-context-empty.html": [ + "952c73f1680805dc3a976446bb509cb924a6a702", + [ + null, + [ + [ + "/_mozilla/css/stacking-context-empty-ref.html", + "==" + ] + ], + {} + ] + ], "stacking_context_overflow_a.html": [ "dc379afb77977b0e99a0a8ce3321c9afff236a37", [ @@ -10311,6 +10324,10 @@ "0525bab6b11800d29f90efc7efef0f43165fba01", [] ], + "stacking-context-empty-ref.html": [ + "8006e2413694b0776f000d3b8138bed29812b7cd", + [] + ], "stacking_context_overflow_ref.html": [ "49991c449ab4f42afae6f512a7f184e70d77bc34", [] diff --git a/tests/wpt/mozilla/tests/css/stacking-context-empty-ref.html b/tests/wpt/mozilla/tests/css/stacking-context-empty-ref.html new file mode 100644 index 00000000000..8006e241369 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/stacking-context-empty-ref.html @@ -0,0 +1 @@ +<p><img src="100x100_green.png"> diff --git a/tests/wpt/mozilla/tests/css/stacking-context-empty.html b/tests/wpt/mozilla/tests/css/stacking-context-empty.html new file mode 100644 index 00000000000..952c73f1680 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/stacking-context-empty.html @@ -0,0 +1,14 @@ +<link rel="match" href="stacking-context-empty-ref.html"> +<style> + div { + border-radius: 2px; + background: #777; + border-bottom: 1px solid #f1f1f1; + } +.test { + transform: scaleX(0); +} +</style> +<p><img src="100x100_green.png"> +<div class="test">aaa</div> +<p><img class="test" src="100x100_green.png"> |