Base the backface-visibility decision off the containing block, add Picture3DContext enum

Author: kvark

webrender/src/batch.rs

@@ -15,7 +15,7 @@ use gpu_types::{ClipMaskInstance, SplitCompositeInstance};
 use gpu_types::{PrimitiveInstanceData, RasterizationSpace, GlyphInstance};
 use gpu_types::{PrimitiveHeader, PrimitiveHeaderIndex, TransformPaletteId, TransformPalette};
 use internal_types::{FastHashMap, SavedTargetIndex, SourceTexture};
-use picture::{PictureCompositeMode, PicturePrimitive, PictureSurface};
+use picture::{Picture3DContext, PictureCompositeMode, PicturePrimitive, PictureSurface};
 use plane_split::{BspSplitter, Clipper, Polygon, Splitter};
 use prim_store::{BrushKind, BrushPrimitive, BrushSegmentTaskId, DeferredResolve};
 use prim_store::{EdgeAaSegmentMask, ImageSource};
@@ -653,7 +653,7 @@ impl AlphaBatchBuilder {
                         // If this picture is participating in a 3D rendering context,
                         // then don't add it to any batches here. Instead, create a polygon
                         // for it and add it to the current plane splitter.
-                        if picture.is_in_3d_context {
+                        if let Picture3DContext::In { .. } = picture.context_3d {
                             // Push into parent plane splitter.
                             debug_assert!(picture.raster_config.is_some());
                             let transform = transforms.get_world_transform(prim_metadata.spatial_node_index);

webrender/src/display_list_flattener.rs

@@ -14,7 +14,7 @@ use api::{PropertyBinding, ReferenceFrame, RepeatMode, ScrollFrameDisplayItem, S
 use api::{Shadow, SpecificDisplayItem, StackingContext, StickyFrameDisplayItem, TexelRect};
 use api::{ClipMode, TransformStyle, YuvColorSpace, YuvData};
 use clip::{ClipChainId, ClipRegion, ClipItemKey, ClipStore};
-use clip_scroll_tree::{ClipScrollTree, SpatialNodeIndex};
+use clip_scroll_tree::{ROOT_SPATIAL_NODE_INDEX, ClipScrollTree, SpatialNodeIndex};
 use euclid::vec2;
 use frame_builder::{ChasePrimitive, FrameBuilder, FrameBuilderConfig};
 use glyph_rasterizer::FontInstance;
@@ -23,7 +23,7 @@ use gpu_types::BrushFlags;
 use hit_test::{HitTestingItem, HitTestingRun};
 use image::simplify_repeated_primitive;
 use internal_types::{FastHashMap, FastHashSet};
-use picture::{PictureCompositeMode, PictureIdGenerator, PicturePrimitive};
+use picture::{Picture3DContext, PictureCompositeMode, PictureIdGenerator, PicturePrimitive};
 use prim_store::{BrushKind, BrushPrimitive, BrushSegmentDescriptor};
 use prim_store::{EdgeAaSegmentMask, ImageSource, PrimitiveOpacity};
 use prim_store::{BorderSource, BrushSegment, BrushSegmentVec, PrimitiveContainer, PrimitiveIndex, PrimitiveStore};
@@ -1015,11 +1015,42 @@ impl<'a> DisplayListFlattener<'a> {
             composite_mode = Some(PictureCompositeMode::Blit);
         }
 
+        // Find the spatial node index of the containing block, which
+        // defines the context of backface-visibility.
+        let last_leaf_prim_index = self.sc_stack.last().map(|sc| sc.leaf_prim_index);
+        // Note: the logic of obtaining these elements diverge upon the
+        // `establishes_3d_context` flag, but both try to find the parent 3D context,
+        // so we try to unity these code paths.
+        let (context_3d, parent_prim_index) = if participating_in_3d_context {
+            // If we're in a 3D context, we will parent the picture
+            // to the first stacking context we find that is a
+            // 3D rendering context container. This follows the spec
+            // by hoisting these items out into the same 3D context
+            // for plane splitting.
+            let parent = self.sc_stack
+                .iter()
+                .rfind(|sc| sc.establishes_3d_context);
+            let context_3d = Picture3DContext::In {
+                ancestor_index: match parent {
+                    Some(sc) => self.prim_store.get_spatial_node_index(sc.leaf_prim_index),
+                    None => ROOT_SPATIAL_NODE_INDEX,
+                },
+            };
+            let parent_prim_index = if establishes_3d_context {
+                last_leaf_prim_index
+            } else {
+                parent.map(|sc| sc.root_prim_index)
+            };
+            (context_3d, parent_prim_index)
+        } else {
+            (Picture3DContext::Out, last_leaf_prim_index)
+        };
+
         // Add picture for this actual stacking context contents to render into.
         let leaf_picture = PicturePrimitive::new_image(
             self.picture_id_generator.next(),
             composite_mode,
-            participating_in_3d_context,
+            context_3d,
             pipeline_id,
             frame_output_pipeline_id,
             true,
@@ -1055,7 +1086,7 @@ impl<'a> DisplayListFlattener<'a> {
             let mut filter_picture = PicturePrimitive::new_image(
                 self.picture_id_generator.next(),
                 Some(PictureCompositeMode::Filter(filter)),
-                false,
+                Picture3DContext::Out,
                 pipeline_id,
                 None,
                 true,
@@ -1085,7 +1116,7 @@ impl<'a> DisplayListFlattener<'a> {
             let mut blend_picture = PicturePrimitive::new_image(
                 self.picture_id_generator.next(),
                 Some(PictureCompositeMode::MixBlend(mix_blend_mode)),
-                false,
+                Picture3DContext::Out,
                 pipeline_id,
                 None,
                 true,
@@ -1118,7 +1149,7 @@ impl<'a> DisplayListFlattener<'a> {
             let mut container_picture = PicturePrimitive::new_image(
                 self.picture_id_generator.next(),
                 None,
-                false,
+                Picture3DContext::Out,
                 pipeline_id,
                 None,
                 true,
@@ -1149,10 +1180,10 @@ impl<'a> DisplayListFlattener<'a> {
             pipeline_id,
             transform_style,
             establishes_3d_context,
-            participating_in_3d_context,
             leaf_prim_index,
             root_prim_index: current_prim_index,
             has_mix_blend_mode: composite_ops.mix_blend_mode.is_some(),
+            parent_prim_index,
         };
 
         self.sc_stack.push(sc);
@@ -1169,28 +1200,10 @@ impl<'a> DisplayListFlattener<'a> {
             self.prim_store.optimize_picture_if_possible(prim_index);
         }
 
-        if self.sc_stack.is_empty() {
-            // This must be the root stacking context
-            return;
-        }
-
-        let parent_prim_index = if !sc.establishes_3d_context && sc.participating_in_3d_context {
-            // If we're in a 3D context, we will parent the picture
-            // to the first stacking context we find that is a
-            // 3D rendering context container. This follows the spec
-            // by hoisting these items out into the same 3D context
-            // for plane splitting.
-            self.sc_stack
-                .iter()
-                .rev()
-                .find(|sc| sc.establishes_3d_context)
-                .map(|sc| sc.root_prim_index)
-                .unwrap()
-        } else {
-            self.sc_stack.last().unwrap().leaf_prim_index
+        let parent_pic = match sc.parent_prim_index {
+            Some(prim_index) => self.prim_store.get_pic_mut(prim_index),
+            None => return, // This must be the root stacking context
         };
-
-        let parent_pic = self.prim_store.get_pic_mut(parent_prim_index);
         parent_pic.add_primitive(sc.root_prim_index);
 
         // If we have a mix-blend-mode, and we aren't the primary framebuffer,
@@ -1431,7 +1444,7 @@ impl<'a> DisplayListFlattener<'a> {
         let shadow_pic = PicturePrimitive::new_image(
             self.picture_id_generator.next(),
             Some(PictureCompositeMode::Filter(blur_filter)),
-            false,
+            Picture3DContext::Out,
             pipeline_id,
             None,
             is_passthrough,
@@ -2058,6 +2071,7 @@ struct FlattenedStackingContext {
     /// If true, this stacking context establishes a new
     /// 3d rendering context.
     establishes_3d_context: bool,
-    participating_in_3d_context: bool,
     has_mix_blend_mode: bool,
+
+    parent_prim_index: Option<PrimitiveIndex>,
 }

webrender/src/frame_builder.rs

@@ -86,6 +86,7 @@ pub struct FrameBuildingState<'a> {
     pub segment_builder: SegmentBuilder,
 }
 
+#[derive(Debug)]
 pub struct PictureContext {
     pub pipeline_id: PipelineId,
     pub apply_local_clip_rect: bool,
@@ -105,6 +106,7 @@ pub struct PictureState {
     pub map_pic_to_world: SpaceMapper<PicturePixel, WorldPixel>,
     pub map_pic_to_raster: SpaceMapper<PicturePixel, RasterPixel>,
     pub map_raster_to_world: SpaceMapper<RasterPixel, WorldPixel>,
+    pub map_local_to_containing_block: SpaceMapper<LayoutPixel, LayoutPixel>,
     pub surface_spatial_node_index: SpatialNodeIndex,
     pub raster_spatial_node_index: SpatialNodeIndex,
 }

webrender/src/picture.rs

@@ -154,6 +154,22 @@ pub struct PictureCacheKey {
     unclipped_size: DeviceIntSize,
 }
 
+/// Enum value describing the place of a picture in a 3D context.
+#[derive(Debug)]
+pub enum Picture3DContext {
+    /// The picture is not a part of 3D context sub-hierarchy.
+    Out,
+    /// The picture is a part of 3D context.
+    In {
+        /// The spatial node index of an "ancestor" element, i.e. one
+        /// that establishes the transformed element’s containing block.
+        ///
+        /// See CSS spec draft for more details:
+        /// https://drafts.csswg.org/css-transforms-2/#accumulated-3d-transformation-matrix-computation
+        ancestor_index: SpatialNodeIndex,
+    }
+}
+
 #[derive(Debug)]
 pub struct PicturePrimitive {
     // List of primitive runs that make up this picture.
@@ -182,8 +198,8 @@ pub struct PicturePrimitive {
     pub requested_raster_space: RasterSpace,
 
     pub raster_config: Option<RasterConfig>,
-    // If true, this picture is part of a 3D context.
-    pub is_in_3d_context: bool,
+    pub context_3d: Picture3DContext,
+
     // If requested as a frame output (for rendering
     // pages to a texture), this is the pipeline this
     // picture is the root of.
@@ -217,7 +233,7 @@ impl PicturePrimitive {
     pub fn new_image(
         id: PictureId,
         requested_composite_mode: Option<PictureCompositeMode>,
-        is_in_3d_context: bool,
+        context_3d: Picture3DContext,
         pipeline_id: PipelineId,
         frame_output_pipeline_id: Option<PipelineId>,
         apply_local_clip_rect: bool,
@@ -229,7 +245,7 @@ impl PicturePrimitive {
             secondary_render_task_id: None,
             requested_composite_mode,
             raster_config: None,
-            is_in_3d_context,
+            context_3d,
             frame_output_pipeline_id,
             extra_gpu_data_handle: GpuCacheHandle::new(),
             apply_local_clip_rect,
@@ -318,6 +334,15 @@ impl PicturePrimitive {
             frame_context,
         );
 
+        let containing_node_index = match self.context_3d {
+            Picture3DContext::Out => surface_spatial_node_index,
+            Picture3DContext::In { ancestor_index } => ancestor_index,
+        };
+        let map_local_to_containing_block = SpaceMapper::new(
+            containing_node_index,
+            LayoutRect::zero(), // bounds aren't going to be used for this mapping
+        );
+
         self.raster_config = actual_composite_mode.map(|composite_mode| {
             RasterConfig {
                 composite_mode,
@@ -337,6 +362,7 @@ impl PicturePrimitive {
             map_pic_to_world,
             map_pic_to_raster,
             map_raster_to_world,
+            map_local_to_containing_block,
         };
 
         // Disallow subpixel AA if an intermediate surface is needed.

webrender/src/prim_store.rs

@@ -29,7 +29,7 @@ use render_task::{RenderTaskCacheKeyKind, RenderTaskId, RenderTaskCacheEntryHand
 use renderer::{MAX_VERTEX_TEXTURE_WIDTH};
 use resource_cache::{ImageProperties, ImageRequest, ResourceCache};
 use scene::SceneProperties;
-use std::{cmp, fmt, mem, usize};
+use std::{cmp, fmt, mem, ops, usize};
 use util::{ScaleOffset, MatrixHelpers, pack_as_float, project_rect, raster_rect_to_device_pixels};
 use smallvec::SmallVec;
 
@@ -82,6 +82,16 @@ pub enum VisibleFace {
     Back,
 }
 
+impl ops::Not for VisibleFace {
+    type Output = Self;
+    fn not(self) -> Self {
+        match self {
+            VisibleFace::Front => VisibleFace::Back,
+            VisibleFace::Back => VisibleFace::Front,
+        }
+    }
+}
+
 #[derive(Debug)]
 pub enum CoordinateSpaceMapping<F, T> {
     Local,
@@ -1413,6 +1423,10 @@ impl PrimitiveStore {
         self.primitives[index.0].as_pic_mut()
     }
 
+    pub fn get_spatial_node_index(&self, index: PrimitiveIndex) -> SpatialNodeIndex {
+        self.primitives[index.0].metadata.spatial_node_index
+    }
+
     pub fn add_primitive(
         &mut self,
         local_rect: &LayoutRect,
@@ -1883,31 +1897,14 @@ impl PrimitiveStore {
                 (prim.metadata.spatial_node_index, prim.metadata.is_backface_visible)
             };
 
-            let spatial_node = &frame_context
-                .clip_scroll_tree
-                .spatial_nodes[spatial_node_index.0];
-
-            // TODO(gw): Although constructing these is cheap, they are often
-            //           the same for many consecutive primitives, so it may
-            //           be worth caching the most recent context.
-            let prim_context = PrimitiveContext::new(
-                spatial_node,
-                spatial_node_index,
-            );
-
-            // Mark whether this picture contains any complex coordinate
-            // systems, due to either the scroll node or the clip-chain.
-            pic_state.has_non_root_coord_system |=
-                spatial_node.coordinate_system_id != CoordinateSystemId::root();
-
-            pic_state.map_local_to_pic.set_target_spatial_node(
+            pic_state.map_local_to_containing_block.set_target_spatial_node(
                 spatial_node_index,
                 frame_context.clip_scroll_tree,
             );
 
             // Do some basic checks first, that can early out
             // without even knowing the local rect.
-            match pic_state.map_local_to_pic.visible_face() {
+            match pic_state.map_local_to_containing_block.visible_face() {
                 VisibleFace::Back if !is_backface_visible => {
                     if cfg!(debug_assertions) && is_chased {
                         println!("\tculled for not having visible back faces");
@@ -1917,7 +1914,17 @@ impl PrimitiveStore {
                 _ => {}
             }
 
+            if !is_backface_visible {
+                println!("\tprim {:?} with property {} is not culled for visible face {:?}, transform {:?}",
+                    prim_index, is_backface_visible,
+                    pic_state.map_local_to_containing_block.visible_face(),
+                    pic_state.map_local_to_containing_block);
+                println!("\tpicture context {:?}", pic_context);
+            }
 
+            let spatial_node = &frame_context
+                .clip_scroll_tree
+                .spatial_nodes[spatial_node_index.0];
 
             if !spatial_node.invertible {
                 if cfg!(debug_assertions) && is_chased {
@@ -1926,14 +1933,22 @@ impl PrimitiveStore {
                 continue;
             }
 
+            pic_state.map_local_to_pic.set_target_spatial_node(
+                spatial_node_index,
+                frame_context.clip_scroll_tree,
+            );
+
             // Mark whether this picture contains any complex coordinate
             // systems, due to either the scroll node or the clip-chain.
             pic_state.has_non_root_coord_system |=
                 spatial_node.coordinate_system_id != CoordinateSystemId::root();
 
-            pic_state.map_local_to_pic.set_target_spatial_node(
+            // TODO(gw): Although constructing these is cheap, they are often
+            //           the same for many consecutive primitives, so it may
+            //           be worth caching the most recent context.
+            let prim_context = PrimitiveContext::new(
+                spatial_node,
                 spatial_node_index,
-                frame_context.clip_scroll_tree,
             );
 
             if self.prepare_prim_for_render(

wrench/reftests/backface/backface-hidden.yaml

@@ -0,0 +1,18 @@
+# In this test, the leaf green rectangle has only its back visible
+# due to rotate-x(180) transformation,
+# and "backface-visible = false", so it's completely hidden.
+---
+root:
+  items:
+    - type: rect
+      color: red
+      bounds: 0 0 1024 768
+    - type: stacking-context
+      bounds: 0 0 1024 768
+      transform: rotate-x(180)
+      transform-style: preserve-3d
+      items:
+        - type: rect
+          bounds: 0 0 1024 768
+          color: green
+          backface-visible: false

wrench/reftests/backface/reftest.list

@@ -1,4 +1,5 @@
 == backface-leaf.yaml backface-ref.yaml
+== backface-hidden.yaml backface-ref.yaml
 == backface-sc.yaml backface-ref.yaml
 == backface-picture.yaml backface-picture-ref.yaml
 == backface-double-flip.yaml blank.yaml