Bug 1849680 - Add support for masks on opaque compositor surfaces. r=gfx-reviewers,lsalzman

CLOSED TREE

Differential Revision: https://phabricator.services.mozilla.com/D188155

dom/media/test/mochitest.ini

@@ -856,6 +856,7 @@ scheme=https
 skip-if = toolkit == 'android' # android(bug 1232305), bugs 1320418,1347953,1347954,1348140,1348386
 [test_video_low_power_telemetry.html]
 skip-if = (os != 'mac') # This telemetry is macOS-specific.
+disabled = https://bugzilla.mozilla.org/show_bug.cgi?id=1849680
 [test_vp9_superframes.html]
 [test_temporary_file_blob_video_plays.html]
 skip-if = toolkit == 'android'

gfx/wr/webrender/src/batch.rs

@@ -7,6 +7,7 @@ use api::{FontInstanceFlags, YuvColorSpace, YuvFormat, ColorDepth, ColorRange, P
 use api::units::*;
 use crate::clip::{ClipNodeFlags, ClipNodeRange, ClipItemKind, ClipStore};
 use crate::command_buffer::{PrimitiveCommand, QuadFlags};
+use crate::composite::CompositorSurfaceKind;
 use crate::spatial_tree::{SpatialTree, SpatialNodeIndex, CoordinateSystemId};
 use glyph_rasterizer::{GlyphFormat, SubpixelDirection};
 use crate::gpu_cache::{GpuBlockData, GpuCache, GpuCacheAddress};
@@ -2315,8 +2316,23 @@ impl BatchBuilder {
                     render_tasks,
                 );
             }
-            PrimitiveInstanceKind::YuvImage { data_handle, segment_instance_index, is_compositor_surface, .. } => {
-                debug_assert!(!is_compositor_surface);
+            PrimitiveInstanceKind::YuvImage { data_handle, segment_instance_index, compositor_surface_kind, .. } => {
+                if compositor_surface_kind.needs_cutout() {
+                    self.add_compositor_surface_cutout(
+                        prim_rect,
+                        prim_info.clip_chain.local_clip_rect,
+                        prim_info.clip_task_index,
+                        transform_id,
+                        z_id,
+                        bounding_rect,
+                        ctx,
+                        gpu_cache,
+                        render_tasks,
+                        prim_headers,
+                    );
+
+                    return;
+                }
 
                 let yuv_image_data = &ctx.data_stores.yuv_image[data_handle].kind;
                 let mut textures = TextureSet::UNTEXTURED;
@@ -2420,8 +2436,23 @@ impl BatchBuilder {
                     render_tasks,
                 );
             }
-            PrimitiveInstanceKind::Image { data_handle, image_instance_index, is_compositor_surface, .. } => {
-                debug_assert!(!is_compositor_surface);
+            PrimitiveInstanceKind::Image { data_handle, image_instance_index, compositor_surface_kind, .. } => {
+                if compositor_surface_kind.needs_cutout() {
+                    self.add_compositor_surface_cutout(
+                        prim_rect,
+                        prim_info.clip_chain.local_clip_rect,
+                        prim_info.clip_task_index,
+                        transform_id,
+                        z_id,
+                        bounding_rect,
+                        ctx,
+                        gpu_cache,
+                        render_tasks,
+                        prim_headers,
+                    );
+
+                    return;
+                }
 
                 let image_data = &ctx.data_stores.image[data_handle].kind;
                 let common_data = &ctx.data_stores.image[data_handle].common;
@@ -3138,6 +3169,61 @@ impl BatchBuilder {
         }
     }
 
+    /// Draw a (potentially masked) alpha cutout so that a video underlay will be blended
+    /// through by the compositor
+    fn add_compositor_surface_cutout(
+        &mut self,
+        prim_rect: LayoutRect,
+        local_clip_rect: LayoutRect,
+        clip_task_index: ClipTaskIndex,
+        transform_id: TransformPaletteId,
+        z_id: ZBufferId,
+        bounding_rect: &PictureRect,
+        ctx: &RenderTargetContext,
+        gpu_cache: &mut GpuCache,
+        render_tasks: &RenderTaskGraph,
+        prim_headers: &mut PrimitiveHeaders,
+    ) {
+        let prim_cache_address = gpu_cache.get_address(&ctx.globals.default_black_rect_handle);
+
+        let (clip_task_address, clip_mask_texture_id) = ctx.get_prim_clip_task_and_texture(
+            clip_task_index,
+            render_tasks,
+        ).unwrap();
+
+        let prim_header = PrimitiveHeader {
+            local_rect: prim_rect,
+            local_clip_rect,
+            specific_prim_address: prim_cache_address,
+            transform_id,
+        };
+
+        let prim_header_index = prim_headers.push(
+            &prim_header,
+            z_id,
+            [get_shader_opacity(1.0), 0, 0, 0],
+        );
+
+        let batch_key = BatchKey {
+            blend_mode: BlendMode::PremultipliedDestOut,
+            kind: BatchKind::Brush(BrushBatchKind::Solid),
+            textures: BatchTextures::prim_untextured(clip_mask_texture_id),
+        };
+
+        self.add_brush_instance_to_batches(
+            batch_key,
+            BatchFeatures::ALPHA_PASS | BatchFeatures::CLIP_MASK,
+            bounding_rect,
+            z_id,
+            INVALID_SEGMENT_INDEX,
+            EdgeAaSegmentMask::empty(),
+            clip_task_address,
+            BrushFlags::empty(),
+            prim_header_index,
+            0,
+        );
+    }
+
     /// Add a single segment instance to a batch.
     ///
     /// `edge_aa_mask` Specifies the edges that are *allowed* to have anti-aliasing, if and only
@@ -3904,3 +3990,13 @@ pub fn add_quad_to_batch<F>(
         f(edge_batch_key, instance.into());
     }
 }
+
+impl CompositorSurfaceKind {
+    /// Returns true if the type of compositor surface needs an alpha cutout rendered
+    fn needs_cutout(&self) -> bool {
+        match self {
+            CompositorSurfaceKind::Underlay => true,
+            CompositorSurfaceKind::Overlay | CompositorSurfaceKind::Blit => false,
+        }
+    }
+}

gfx/wr/webrender/src/composite.rs

@@ -16,6 +16,7 @@ use crate::resource_cache::{ImageRequest, ResourceCache};
 use crate::util::{Preallocator, ScaleOffset};
 use crate::tile_cache::PictureCacheDebugInfo;
 use crate::device::Device;
+use crate::space::SpaceMapper;
 use std::{ops, u64, os::raw::c_void};
 
 /*
@@ -23,6 +24,19 @@ use std::{ops, u64, os::raw::c_void};
  and/or OS compositor integration.
  */
 
+/// Which method is being used to draw a requested compositor surface
+#[cfg_attr(feature = "capture", derive(Serialize))]
+#[cfg_attr(feature = "replay", derive(Deserialize))]
+#[derive(Debug, Copy, Clone, MallocSizeOf)]
+pub enum CompositorSurfaceKind {
+    /// Don't create a native compositor surface, blit it as a regular primitive
+    Blit,
+    /// Create a native surface, draw it under content (must be opaque)
+    Underlay,
+    /// Create a native surface, draw it between sub-slices (supports transparent)
+    Overlay,
+}
+
 /// Describes details of an operation to apply to a native surface
 #[derive(Debug, Clone)]
 #[cfg_attr(feature = "capture", derive(Serialize))]
@@ -191,6 +205,28 @@ pub struct ExternalSurfaceDescriptor {
     pub update_params: Option<DeviceIntSize>,
 }
 
+impl ExternalSurfaceDescriptor {
+    /// Calculate an optional occlusion rect for a given compositor surface
+    pub fn get_occluder_rect(
+        &self,
+        local_clip_rect: &PictureRect,
+        map_pic_to_world: &SpaceMapper<PicturePixel, WorldPixel>,
+    ) -> Option<WorldRect> {
+        let local_surface_rect = self
+            .local_rect
+            .intersection(&self.local_clip_rect)
+            .and_then(|r| {
+                r.intersection(local_clip_rect)
+            });
+
+        local_surface_rect.map(|local_surface_rect| {
+            map_pic_to_world
+                .map(&local_surface_rect)
+                .expect("bug: unable to map external surface to world space")
+        })
+    }
+}
+
 /// Information about a plane in a YUV or RGB surface.
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
@@ -630,6 +666,110 @@ impl CompositeState {
         self.occluders.push(world_rect, z_id);
     }
 
+    /// Push a compositor surface on to the list of tiles to be passed to the compositor
+    fn push_compositor_surface(
+        &mut self,
+        external_surface: &ExternalSurfaceDescriptor,
+        is_opaque: bool,
+        device_clip_rect: DeviceRect,
+        resource_cache: &ResourceCache,
+        gpu_cache: &mut GpuCache,
+        deferred_resolves: &mut Vec<DeferredResolve>,
+    ) {
+        let clip_rect = external_surface
+            .clip_rect
+            .intersection(&device_clip_rect)
+            .unwrap_or_else(DeviceRect::zero);
+
+        // Skip compositor surfaces with empty clip rects.
+        if clip_rect.is_empty() {
+            return;
+        }
+
+        let required_plane_count =
+            match external_surface.dependency {
+                ExternalSurfaceDependency::Yuv { format, .. } => {
+                    format.get_plane_num()
+                },
+                ExternalSurfaceDependency::Rgb { .. } => {
+                    1
+                }
+            };
+
+        let mut image_dependencies = [ImageDependency::INVALID; 3];
+
+        for i in 0 .. required_plane_count {
+            let dependency = match external_surface.dependency {
+                ExternalSurfaceDependency::Yuv { image_dependencies, .. } => {
+                    image_dependencies[i]
+                },
+                ExternalSurfaceDependency::Rgb { image_dependency, .. } => {
+                    image_dependency
+                }
+            };
+            image_dependencies[i] = dependency;
+        }
+
+        // Get a new z_id for each compositor surface, to ensure correct ordering
+        // when drawing with the simple (Draw) compositor, and to schedule compositing
+        // of any required updates into the surfaces.
+        let needs_external_surface_update = match self.compositor_kind {
+            CompositorKind::Draw { .. } => true,
+            _ => external_surface.update_params.is_some(),
+        };
+        let external_surface_index = if needs_external_surface_update {
+            let external_surface_index = self.compute_external_surface_dependencies(
+                &external_surface,
+                &image_dependencies,
+                required_plane_count,
+                resource_cache,
+                gpu_cache,
+                deferred_resolves,
+            );
+            if external_surface_index == ResolvedExternalSurfaceIndex::INVALID {
+                return;
+            }
+            external_surface_index
+        } else {
+            ResolvedExternalSurfaceIndex::INVALID
+        };
+
+        let surface = CompositeTileSurface::ExternalSurface { external_surface_index };
+        let local_rect = external_surface.local_surface_size.cast_unit().into();
+
+        let tile = CompositeTile {
+            kind: tile_kind(&surface, is_opaque),
+            surface,
+            local_rect,
+            local_valid_rect: local_rect,
+            local_dirty_rect: local_rect,
+            device_clip_rect: clip_rect,
+            z_id: external_surface.z_id,
+            transform_index: external_surface.transform_index,
+        };
+
+        // Add a surface descriptor for each compositor surface. For the Draw
+        // compositor, this is used to avoid composites being skipped by adding
+        // a dependency on the compositor surface external image keys / generations.
+        self.descriptor.surfaces.push(
+            CompositeSurfaceDescriptor {
+                surface_id: external_surface.native_surface_id,
+                clip_rect,
+                transform: self.get_compositor_transform(external_surface.transform_index),
+                image_dependencies: image_dependencies,
+                image_rendering: external_surface.image_rendering,
+                tile_descriptors: Vec::new(),
+            }
+        );
+
+        let device_rect =
+            self.get_device_rect(&local_rect, external_surface.transform_index);
+        self.descriptor.external_surfaces_rect =
+            self.descriptor.external_surfaces_rect.union(&device_rect);
+
+        self.tiles.push(tile);
+    }
+
     /// Add a picture cache to be composited
     pub fn push_surface(
         &mut self,
@@ -661,6 +801,18 @@ impl CompositeState {
             );
         }
 
+        // Add any underlay surfaces to the compositing tree
+        for underlay in &tile_cache.underlays {
+            self.push_compositor_surface(
+                underlay,
+                true,
+                device_clip_rect,
+                resource_cache,
+                gpu_cache,
+                deferred_resolves,
+            );
+        }
+
         for sub_slice in &tile_cache.sub_slices {
             let mut surface_device_rect = DeviceRect::zero();
 
@@ -708,7 +860,7 @@ impl CompositeState {
                         }
                     );
                 }
-    
+
                 // Add alpha tiles after opaque surfaces
                 if !sub_slice.alpha_tile_descriptors.is_empty() {
                     self.descriptor.surfaces.push(
@@ -727,100 +879,14 @@ impl CompositeState {
             // For each compositor surface that was promoted, build the
             // information required for the compositor to draw it
             for compositor_surface in &sub_slice.compositor_surfaces {
-                let external_surface = &compositor_surface.descriptor;
-
-                let clip_rect = external_surface
-                    .clip_rect
-                    .intersection(&device_clip_rect)
-                    .unwrap_or_else(DeviceRect::zero);
-                    
-                // Skip compositor surfaces with empty clip rects.
-                if clip_rect.is_empty() {
-                    continue;
-                }
-
-                let required_plane_count =
-                    match external_surface.dependency {
-                        ExternalSurfaceDependency::Yuv { format, .. } => {
-                            format.get_plane_num()
-                        },
-                        ExternalSurfaceDependency::Rgb { .. } => {
-                            1
-                        }
-                    };
-
-                let mut image_dependencies = [ImageDependency::INVALID; 3];
-
-                for i in 0 .. required_plane_count {
-                    let dependency = match external_surface.dependency {
-                        ExternalSurfaceDependency::Yuv { image_dependencies, .. } => {
-                            image_dependencies[i]
-                        },
-                        ExternalSurfaceDependency::Rgb { image_dependency, .. } => {
-                            image_dependency
-                        }
-                    };
-                    image_dependencies[i] = dependency;
-                }
-
-                // Get a new z_id for each compositor surface, to ensure correct ordering
-                // when drawing with the simple (Draw) compositor, and to schedule compositing
-                // of any required updates into the surfaces.
-                let needs_external_surface_update = match self.compositor_kind {
-                    CompositorKind::Draw { .. } => true,
-                    _ => external_surface.update_params.is_some(),
-                };
-                let external_surface_index = if needs_external_surface_update {
-                    let external_surface_index = self.compute_external_surface_dependencies(
-                        &external_surface,
-                        &image_dependencies,
-                        required_plane_count,
-                        resource_cache,
-                        gpu_cache,
-                        deferred_resolves,
-                    );
-                    if external_surface_index == ResolvedExternalSurfaceIndex::INVALID {
-                        continue;
-                    }
-                    external_surface_index
-                } else {
-                    ResolvedExternalSurfaceIndex::INVALID
-                };
-
-                let surface = CompositeTileSurface::ExternalSurface { external_surface_index };
-                let local_rect = external_surface.local_surface_size.cast_unit().into();
-
-                let tile = CompositeTile {
-                    kind: tile_kind(&surface, compositor_surface.is_opaque),
-                    surface,
-                    local_rect,
-                    local_valid_rect: local_rect,
-                    local_dirty_rect: local_rect,
-                    device_clip_rect: clip_rect,
-                    z_id: external_surface.z_id,
-                    transform_index: external_surface.transform_index,
-                };
-
-                // Add a surface descriptor for each compositor surface. For the Draw
-                // compositor, this is used to avoid composites being skipped by adding
-                // a dependency on the compositor surface external image keys / generations.
-                self.descriptor.surfaces.push(
-                    CompositeSurfaceDescriptor {
-                        surface_id: external_surface.native_surface_id,
-                        clip_rect,
-                        transform: self.get_compositor_transform(external_surface.transform_index),
-                        image_dependencies: image_dependencies,
-                        image_rendering: external_surface.image_rendering,
-                        tile_descriptors: Vec::new(),
-                    }
+                self.push_compositor_surface(
+                    &compositor_surface.descriptor,
+                    compositor_surface.is_opaque,
+                    device_clip_rect,
+                    resource_cache,
+                    gpu_cache,
+                    deferred_resolves,
                 );
-
-                let device_rect =
-                    self.get_device_rect(&local_rect, external_surface.transform_index);
-                self.descriptor.external_surfaces_rect =
-                    self.descriptor.external_surfaces_rect.union(&device_rect);
-
-                self.tiles.push(tile);
             }
         }
     }

gfx/wr/webrender/src/frame_builder.rs

@@ -75,17 +75,17 @@ pub struct FrameGlobalResources {
     /// set of image parameters (color white, stretch == rect.size).
     pub default_image_handle: GpuCacheHandle,
 
-    /// A GPU cache config for drawing transparent rectangle primitives.
+    /// A GPU cache config for drawing cut-out rectangle primitives.
     /// This is used to 'cut out' overlay tiles where a compositor
     /// surface exists.
-    pub default_transparent_rect_handle: GpuCacheHandle,
+    pub default_black_rect_handle: GpuCacheHandle,
 }
 
 impl FrameGlobalResources {
     pub fn empty() -> Self {
         FrameGlobalResources {
             default_image_handle: GpuCacheHandle::new(),
-            default_transparent_rect_handle: GpuCacheHandle::new(),
+            default_black_rect_handle: GpuCacheHandle::new(),
         }
     }
 
@@ -104,8 +104,8 @@ impl FrameGlobalResources {
             ]);
         }
 
-        if let Some(mut request) = gpu_cache.request(&mut self.default_transparent_rect_handle) {
-            request.push(PremultipliedColorF::TRANSPARENT);
+        if let Some(mut request) = gpu_cache.request(&mut self.default_black_rect_handle) {
+            request.push(PremultipliedColorF::BLACK);
         }
     }
 }

gfx/wr/webrender/src/picture.rs

@@ -105,7 +105,7 @@ use crate::clip::{ClipStore, ClipChainInstance, ClipLeafId, ClipNodeId, ClipTree
 use crate::spatial_tree::{SpatialTree, CoordinateSpaceMapping, SpatialNodeIndex, VisibleFace};
 use crate::composite::{CompositorKind, CompositeState, NativeSurfaceId, NativeTileId, CompositeTileSurface, tile_kind};
 use crate::composite::{ExternalSurfaceDescriptor, ExternalSurfaceDependency, CompositeTileDescriptor, CompositeTile};
-use crate::composite::{CompositorTransformIndex};
+use crate::composite::{CompositorTransformIndex, CompositorSurfaceKind};
 use crate::debug_colors;
 use euclid::{vec3, Point2D, Scale, Vector2D, Box2D};
 use euclid::approxeq::ApproxEq;
@@ -518,7 +518,7 @@ struct TileUpdateDirtyState<'a> {
 }
 
 // Immutable context passed to picture cache tiles during post_update
-struct TilePostUpdateContext {
+struct TilePostUpdateContext<'a> {
     /// The local clip rect (in picture space) of the entire picture cache
     local_clip_rect: PictureRect,
 
@@ -530,6 +530,9 @@ struct TilePostUpdateContext {
 
     /// Pre-allocated z-id to assign to tiles during post_update.
     z_id: ZBufferId,
+
+    /// The list of compositor underlays for this picture cache
+    underlays: &'a [ExternalSurfaceDescriptor],
 }
 
 // Mutable state passed to picture cache tiles during post_update
@@ -1214,7 +1217,16 @@ impl Tile {
 
         let has_opaque_bg_color = self.background_color.map_or(false, |c| c.a >= 1.0);
         let has_opaque_backdrop = ctx.backdrop.map_or(false, |b| b.opaque_rect.contains_box(&clipped_rect));
-        let is_opaque = has_opaque_bg_color || has_opaque_backdrop;
+        let mut is_opaque = has_opaque_bg_color || has_opaque_backdrop;
+
+        // If this tile intersects with any underlay surfaces, we need to consider it
+        // translucent, since it will contain an alpha cutout
+        for underlay in ctx.underlays {
+            if clipped_rect.intersects(&underlay.local_rect) {
+                is_opaque = false;
+                break;
+            }
+        }
 
         // Set the correct z_id for this tile
         self.z_id = ctx.z_id;
@@ -1620,7 +1632,7 @@ pub struct TileCacheParams {
     // The number of compositor surfaces that are being requested for this tile cache.
     // This is only a suggestion - the tile cache will clamp this as a reasonable number
     // and only promote a limited number of surfaces.
-    pub compositor_surface_count: usize,
+    pub overlay_surface_count: usize,
 }
 
 /// Defines which sub-slice (effectively a z-index) a primitive exists on within
@@ -1831,6 +1843,8 @@ pub struct TileCacheInstance {
     /// Is there a backdrop associated with this cache
     found_prims_after_backdrop: bool,
     pub backdrop_surface: Option<BackdropSurface>,
+    /// List of underlay compositor surfaces that exist in this picture cache
+    pub underlays: Vec<ExternalSurfaceDescriptor>,
 }
 
 enum SurfacePromotionResult {
@@ -1842,7 +1856,7 @@ impl TileCacheInstance {
     pub fn new(params: TileCacheParams) -> Self {
         // Determine how many sub-slices we need. Clamp to an arbitrary limit to ensure
         // we don't create a huge number of OS compositor tiles and sub-slices.
-        let sub_slice_count = params.compositor_surface_count.min(MAX_COMPOSITOR_SURFACES) + 1;
+        let sub_slice_count = params.overlay_surface_count.min(MAX_COMPOSITOR_SURFACES) + 1;
 
         let mut sub_slices = Vec::with_capacity(sub_slice_count);
         for _ in 0 .. sub_slice_count {
@@ -1893,6 +1907,7 @@ impl TileCacheInstance {
             deferred_dirty_tests: Vec::new(),
             found_prims_after_backdrop: false,
             backdrop_surface: None,
+            underlays: Vec::new(),
         }
     }
 
@@ -1933,7 +1948,7 @@ impl TileCacheInstance {
 
         // Determine how many sub-slices we need, based on how many compositor surface prims are
         // in the supplied primitive list.
-        let required_sub_slice_count = params.compositor_surface_count.min(MAX_COMPOSITOR_SURFACES) + 1;
+        let required_sub_slice_count = params.overlay_surface_count.min(MAX_COMPOSITOR_SURFACES) + 1;
 
         if self.sub_slices.len() != required_sub_slice_count {
             self.tile_rect = TileRect::zero();
@@ -2035,6 +2050,7 @@ impl TileCacheInstance {
         self.local_rect = pic_rect;
         self.local_clip_rect = PictureRect::max_rect();
         self.deferred_dirty_tests.clear();
+        self.underlays.clear();
 
         for sub_slice in &mut self.sub_slices {
             sub_slice.reset();
@@ -2438,6 +2454,7 @@ impl TileCacheInstance {
         prim_spatial_node_index: SpatialNodeIndex,
         is_root_tile_cache: bool,
         sub_slice_index: usize,
+        is_opaque: bool,
         frame_context: &FrameVisibilityContext,
     ) -> SurfacePromotionResult {
         // Check if this primitive _wants_ to be promoted to a compositor surface.
@@ -2446,16 +2463,19 @@ impl TileCacheInstance {
         }
 
         // For now, only support a small (arbitrary) number of compositor surfaces.
-        if sub_slice_index == MAX_COMPOSITOR_SURFACES {
-            return SurfacePromotionResult::Failed;
-        }
+        if !is_opaque {
+            // Non-opaque compositor surfaces require sub-slices, as they are drawn
+            // as overlays.
+            if sub_slice_index == self.sub_slices.len() - 1 {
+                return SurfacePromotionResult::Failed;
+            }
 
-        // If a complex clip is being applied to this primitive, it can't be
-        // promoted directly to a compositor surface (we might be able to
-        // do this in limited cases in future, some native compositors do
-        // support rounded rect clips, for example)
-        if prim_clip_chain.needs_mask {
-            return SurfacePromotionResult::Failed;
+            // If a complex clip is being applied to this primitive, it can't be
+            // promoted directly to a compositor surface unless it's opaque (in
+            // which case we draw as an underlay + alpha cutout)
+            if prim_clip_chain.needs_mask {
+                return SurfacePromotionResult::Failed;
+            }
         }
 
         // If not on the root picture cache, it has some kind of
@@ -2550,6 +2570,7 @@ impl TileCacheInstance {
         composite_state: &mut CompositeState,
         gpu_cache: &mut GpuCache,
         image_rendering: ImageRendering,
+        is_opaque: bool,
     ) -> bool {
         let mut api_keys = [ImageKey::DUMMY; 3];
         api_keys[0] = api_key;
@@ -2568,9 +2589,6 @@ impl TileCacheInstance {
             gpu_cache,
         );
 
-        let is_opaque = resource_cache.get_image_properties(api_key)
-            .map_or(false, |properties| properties.descriptor.is_opaque());
-
         self.setup_compositor_surfaces_impl(
             sub_slice_index,
             prim_info,
@@ -2789,28 +2807,36 @@ impl TileCacheInstance {
             }
         };
 
-        // For compositor surfaces, if we didn't find an earlier sub-slice to add to,
-        // we know we can append to the current slice.
-        assert!(sub_slice_index < self.sub_slices.len() - 1);
-        let sub_slice = &mut self.sub_slices[sub_slice_index];
+        let descriptor = ExternalSurfaceDescriptor {
+            local_surface_size: local_prim_rect.size(),
+            local_rect: prim_rect,
+            local_clip_rect: prim_info.prim_clip_box,
+            dependency,
+            image_rendering,
+            clip_rect,
+            transform_index: compositor_transform_index,
+            z_id: ZBufferId::invalid(),
+            native_surface_id,
+            update_params,
+        };
 
-        // Each compositor surface allocates a unique z-id
-        sub_slice.compositor_surfaces.push(CompositorSurface {
-            prohibited_rect: pic_coverage_rect,
-            is_opaque,
-            descriptor: ExternalSurfaceDescriptor {
-                local_surface_size: local_prim_rect.size(),
-                local_rect: prim_rect,
-                local_clip_rect: prim_info.prim_clip_box,
-                dependency,
-                image_rendering,
-                clip_rect,
-                transform_index: compositor_transform_index,
-                z_id: ZBufferId::invalid(),
-                native_surface_id,
-                update_params,
-            },
-        });
+        // If the surface is opaque, we can draw it an an underlay (which avoids
+        // additional sub-slice surfaces, and supports clip masks)
+        if is_opaque {
+            self.underlays.push(descriptor);
+        } else {
+            // For compositor surfaces, if we didn't find an earlier sub-slice to add to,
+            // we know we can append to the current slice.
+            assert!(sub_slice_index < self.sub_slices.len() - 1);
+            let sub_slice = &mut self.sub_slices[sub_slice_index];
+
+            // Each compositor surface allocates a unique z-id
+            sub_slice.compositor_surfaces.push(CompositorSurface {
+                prohibited_rect: pic_coverage_rect,
+                is_opaque,
+                descriptor,
+            });
+        }
 
         true
     }
@@ -3053,29 +3079,10 @@ impl TileCacheInstance {
                     prim_info.color_binding = Some(color_bindings[color_binding_index].into());
                 }
             }
-            PrimitiveInstanceKind::Image { data_handle, ref mut is_compositor_surface, .. } => {
+            PrimitiveInstanceKind::Image { data_handle, ref mut compositor_surface_kind, .. } => {
                 let image_key = &data_stores.image[data_handle];
                 let image_data = &image_key.kind;
-
-                let mut promote_to_surface = false;
-                match self.can_promote_to_surface(image_key.common.flags,
-                                                  prim_clip_chain,
-                                                  prim_spatial_node_index,
-                                                  is_root_tile_cache,
-                                                  sub_slice_index,
-                                                  frame_context) {
-                    SurfacePromotionResult::Failed => {
-                    }
-                    SurfacePromotionResult::Success => {
-                        promote_to_surface = true;
-                    }
-                }
-
-                // Native OS compositors (DC and CA, at least) support premultiplied alpha
-                // only. If we have an image that's not pre-multiplied alpha, we can't promote it.
-                if image_data.alpha_type == AlphaType::Alpha {
-                    promote_to_surface = false;
-                }
+                let mut is_opaque = false;
 
                 if let Some(image_properties) = resource_cache.get_image_properties(image_data.key) {
                     // For an image to be a possible opaque backdrop, it must:
@@ -3094,6 +3101,29 @@ impl TileCacheInstance {
                             backdrop_rect: PictureRect::zero(),
                         });
                     }
+
+                    is_opaque = image_properties.descriptor.is_opaque();
+                }
+
+                let mut promote_to_surface = false;
+                match self.can_promote_to_surface(image_key.common.flags,
+                                                  prim_clip_chain,
+                                                  prim_spatial_node_index,
+                                                  is_root_tile_cache,
+                                                  sub_slice_index,
+                                                  is_opaque,
+                                                  frame_context) {
+                    SurfacePromotionResult::Failed => {
+                    }
+                    SurfacePromotionResult::Success => {
+                        promote_to_surface = true;
+                    }
+                }
+
+                // Native OS compositors (DC and CA, at least) support premultiplied alpha
+                // only. If we have an image that's not pre-multiplied alpha, we can't promote it.
+                if image_data.alpha_type == AlphaType::Alpha {
+                    promote_to_surface = false;
                 }
 
                 if promote_to_surface {
@@ -3114,22 +3144,28 @@ impl TileCacheInstance {
                         composite_state,
                         gpu_cache,
                         image_data.image_rendering,
+                        is_opaque,
                     );
                 }
 
-                *is_compositor_surface = promote_to_surface;
-
                 if promote_to_surface {
-                    prim_instance.vis.state = VisibilityState::Culled;
-                    return;
+                    if is_opaque {
+                        *compositor_surface_kind = CompositorSurfaceKind::Underlay;
+                    } else {
+                        *compositor_surface_kind = CompositorSurfaceKind::Overlay;
+                        prim_instance.vis.state = VisibilityState::Culled;
+                        return;
+                    }
                 } else {
+                    *compositor_surface_kind = CompositorSurfaceKind::Blit;
+
                     prim_info.images.push(ImageDependency {
                         key: image_data.key,
                         generation: resource_cache.get_image_generation(image_data.key),
                     });
                 }
             }
-            PrimitiveInstanceKind::YuvImage { data_handle, ref mut is_compositor_surface, .. } => {
+            PrimitiveInstanceKind::YuvImage { data_handle, ref mut compositor_surface_kind, .. } => {
                 let prim_data = &data_stores.yuv_image[data_handle];
                 let mut promote_to_surface = match self.can_promote_to_surface(
                                             prim_data.common.flags,
@@ -3137,6 +3173,7 @@ impl TileCacheInstance {
                                             prim_spatial_node_index,
                                             is_root_tile_cache,
                                             sub_slice_index,
+                                            true,
                                             frame_context) {
                     SurfacePromotionResult::Failed => false,
                     SurfacePromotionResult::Success => true,
@@ -3184,12 +3221,12 @@ impl TileCacheInstance {
                 // Store on the YUV primitive instance whether this is a promoted surface.
                 // This is used by the batching code to determine whether to draw the
                 // image to the content tiles, or just a transparent z-write.
-                *is_compositor_surface = promote_to_surface;
 
                 if promote_to_surface {
-                    prim_instance.vis.state = VisibilityState::Culled;
-                    return;
+                    *compositor_surface_kind = CompositorSurfaceKind::Underlay;
                 } else {
+                    *compositor_surface_kind = CompositorSurfaceKind::Blit;
+
                     prim_info.images.extend(
                         prim_data.kind.yuv_key.iter().map(|key| {
                             ImageDependency {
@@ -3617,6 +3654,7 @@ impl TileCacheInstance {
             backdrop: None,
             current_tile_size: self.current_tile_size,
             z_id: ZBufferId::invalid(),
+            underlays: &self.underlays,
         };
 
         let mut state = TilePostUpdateState {
@@ -3641,27 +3679,36 @@ impl TileCacheInstance {
             }
         }
 
+        // Assign z-order for each underlay
+        for underlay in &mut self.underlays {
+            underlay.z_id = state.composite_state.z_generator.next();
+        }
+
         // Register any opaque external compositor surfaces as potential occluders. This
         // is especially useful when viewing video in full-screen mode, as it is
         // able to occlude every background tile (avoiding allocation, rasterizion
         // and compositing).
 
+        // Register any underlays as occluders where possible
+        for underlay in &self.underlays {
+            if let Some(world_surface_rect) = underlay.get_occluder_rect(
+                &self.local_clip_rect,
+                &map_pic_to_world,
+            ) {
+                frame_state.composite_state.register_occluder(
+                    underlay.z_id,
+                    world_surface_rect,
+                );
+            }
+        }
+
         for sub_slice in &self.sub_slices {
             for compositor_surface in &sub_slice.compositor_surfaces {
                 if compositor_surface.is_opaque {
-                    let local_surface_rect = compositor_surface
-                        .descriptor
-                        .local_rect
-                        .intersection(&compositor_surface.descriptor.local_clip_rect)
-                        .and_then(|r| {
-                            r.intersection(&self.local_clip_rect)
-                        });
-
-                    if let Some(local_surface_rect) = local_surface_rect {
-                        let world_surface_rect = map_pic_to_world
-                            .map(&local_surface_rect)
-                            .expect("bug: unable to map external surface to world space");
-
+                    if let Some(world_surface_rect) = compositor_surface.descriptor.get_occluder_rect(
+                        &self.local_clip_rect,
+                        &map_pic_to_world,
+                    ) {
                         frame_state.composite_state.register_occluder(
                             compositor_surface.descriptor.z_id,
                             world_surface_rect,
@@ -4257,8 +4304,10 @@ pub struct PrimitiveList {
     pub clusters: Vec<PrimitiveCluster>,
     pub child_pictures: Vec<PictureIndex>,
     /// The number of preferred compositor surfaces that were found when
-    /// adding prims to this list.
-    pub compositor_surface_count: usize,
+    /// adding prims to this list, which would be rendered as overlays
+    pub overlay_surface_count: usize,
+    /// If true, we found an opaque compositor surface
+    pub has_opaque_compositor_surface: bool,
     pub needs_scissor_rect: bool,
 }
 
@@ -4271,16 +4320,18 @@ impl PrimitiveList {
         PrimitiveList {
             clusters: Vec::new(),
             child_pictures: Vec::new(),
-            compositor_surface_count: 0,
+            overlay_surface_count: 0,
             needs_scissor_rect: false,
+            has_opaque_compositor_surface: false,
         }
     }
 
     pub fn merge(&mut self, other: PrimitiveList) {
         self.clusters.extend(other.clusters);
         self.child_pictures.extend(other.child_pictures);
-        self.compositor_surface_count += other.compositor_surface_count;
+        self.overlay_surface_count += other.overlay_surface_count;
         self.needs_scissor_rect |= other.needs_scissor_rect;
+        self.has_opaque_compositor_surface |= other.has_opaque_compositor_surface;
     }
 
     /// Add a primitive instance to the end of the list
@@ -4304,6 +4355,22 @@ impl PrimitiveList {
             PrimitiveInstanceKind::TextRun { .. } => {
                 self.needs_scissor_rect = true;
             }
+            PrimitiveInstanceKind::YuvImage { .. } => {
+                // Any YUV image that requests a compositor surface is implicitly opaque
+                if prim_flags.contains(PrimitiveFlags::PREFER_COMPOSITOR_SURFACE) {
+                    self.has_opaque_compositor_surface = true;
+                }
+            }
+            PrimitiveInstanceKind::Image { .. } => {
+                // For now, we assume that any image that wants a compositor surface
+                // is transparent, and uses the existing overlay compositor surface
+                // infrastructure. In future, we could detect opaque images, however
+                // it's a little bit of work, as scene building doesn't have access
+                // to the opacity state of an image key at this point.
+                if prim_flags.contains(PrimitiveFlags::PREFER_COMPOSITOR_SURFACE) {
+                    self.overlay_surface_count += 1;
+                }
+            }
             _ => {}
         }
 
@@ -4311,10 +4378,6 @@ impl PrimitiveList {
             flags.insert(ClusterFlags::IS_BACKFACE_VISIBLE);
         }
 
-        if prim_flags.contains(PrimitiveFlags::PREFER_COMPOSITOR_SURFACE) {
-            self.compositor_surface_count += 1;
-        }
-
         let clip_leaf = clip_tree_builder.get_leaf(prim_instance.clip_leaf_id);
         let culling_rect = clip_leaf.local_clip_rect
             .intersection(&prim_rect)

gfx/wr/webrender/src/prepare.rs

@@ -11,6 +11,7 @@ use api::{BoxShadowClipMode, BorderStyle, ClipMode};
 use api::units::*;
 use euclid::Scale;
 use smallvec::SmallVec;
+use crate::composite::CompositorSurfaceKind;
 use crate::command_buffer::{PrimitiveCommand, QuadFlags, CommandBufferIndex};
 use crate::image_tiling::{self, Repetition};
 use crate::border::{get_max_scale_for_border, build_border_instances};
@@ -1962,19 +1963,26 @@ fn build_segments_if_needed(
             assert!(use_legacy_path);
             segment_instance_index
         }
-        PrimitiveInstanceKind::YuvImage { ref mut segment_instance_index, .. } => {
+        PrimitiveInstanceKind::YuvImage { ref mut segment_instance_index, compositor_surface_kind, .. } => {
+            // Only use segments for YUV images if not drawing as a compositor surface
+            if !compositor_surface_kind.supports_segments() {
+                *segment_instance_index = SegmentInstanceIndex::UNUSED;
+                return;
+            }
+
             segment_instance_index
         }
-        PrimitiveInstanceKind::Image { data_handle, image_instance_index, .. } => {
+        PrimitiveInstanceKind::Image { data_handle, image_instance_index, compositor_surface_kind, .. } => {
             let image_data = &data_stores.image[data_handle].kind;
             let image_instance = &mut prim_store.images[image_instance_index];
+
             //Note: tiled images don't support automatic segmentation,
             // they strictly produce one segment per visible tile instead.
-            if frame_state
-                .resource_cache
-                .get_image_properties(image_data.key)
-                .and_then(|properties| properties.tiling)
-                .is_some()
+            if !compositor_surface_kind.supports_segments() ||
+                frame_state.resource_cache
+                    .get_image_properties(image_data.key)
+                    .and_then(|properties| properties.tiling)
+                    .is_some()
             {
                 image_instance.segment_instance_index = SegmentInstanceIndex::UNUSED;
                 return;
@@ -2201,3 +2209,13 @@ fn add_composite_prim(
         targets,
     );
 }
+
+impl CompositorSurfaceKind {
+    /// Returns true if the compositor surface strategy supports segment rendering
+    fn supports_segments(&self) -> bool {
+        match self {
+            CompositorSurfaceKind::Underlay | CompositorSurfaceKind::Overlay => false,
+            CompositorSurfaceKind::Blit => true,
+        }
+    }
+}

gfx/wr/webrender/src/prim_store/image.rs

@@ -8,6 +8,7 @@ use api::{
     RasterSpace, Shadow, YuvColorSpace, ColorRange, YuvFormat,
 };
 use api::units::*;
+use crate::composite::CompositorSurfaceKind;
 use crate::scene_building::{CreateShadow, IsVisible};
 use crate::frame_builder::{FrameBuildingContext, FrameBuildingState};
 use crate::gpu_cache::{GpuCache, GpuDataRequest};
@@ -455,7 +456,7 @@ impl InternablePrimitive for Image {
         PrimitiveInstanceKind::Image {
             data_handle,
             image_instance_index,
-            is_compositor_surface: false,
+            compositor_surface_kind: CompositorSurfaceKind::Blit,
         }
     }
 }
@@ -652,7 +653,7 @@ impl InternablePrimitive for YuvImage {
         PrimitiveInstanceKind::YuvImage {
             data_handle,
             segment_instance_index: SegmentInstanceIndex::INVALID,
-            is_compositor_surface: false,
+            compositor_surface_kind: CompositorSurfaceKind::Blit,
         }
     }
 }

gfx/wr/webrender/src/prim_store/mod.rs

@@ -9,6 +9,7 @@ use api::{PrimitiveKeyKind, FillRule, POLYGON_CLIP_VERTEX_MAX};
 use api::units::*;
 use euclid::{SideOffsets2D, Size2D};
 use malloc_size_of::MallocSizeOf;
+use crate::composite::CompositorSurfaceKind;
 use crate::clip::ClipLeafId;
 use crate::segment::EdgeAaSegmentMask;
 use crate::border::BorderSegmentCacheKey;
@@ -1002,13 +1003,13 @@ pub enum PrimitiveInstanceKind {
         /// Handle to the common interned data for this primitive.
         data_handle: YuvImageDataHandle,
         segment_instance_index: SegmentInstanceIndex,
-        is_compositor_surface: bool,
+        compositor_surface_kind: CompositorSurfaceKind,
     },
     Image {
         /// Handle to the common interned data for this primitive.
         data_handle: ImageDataHandle,
         image_instance_index: ImageInstanceIndex,
-        is_compositor_surface: bool,
+        compositor_surface_kind: CompositorSurfaceKind,
     },
     /// Always rendered directly into the picture. This tends to be
     /// faster with SWGL.

gfx/wr/webrender/src/tile_cache.rs

@@ -602,6 +602,15 @@ fn create_tile_cache(
 
     let slice_id = SliceId::new(slice);
 
+    // If we found an opaque compositor surface, use underlays. This implies
+    // we disable overlays for any transparent compositor surfaces and composite
+    // them in to regular picture cache tiles.
+    let overlay_surface_count = if prim_list.has_opaque_compositor_surface {
+        0
+    } else {
+        prim_list.overlay_surface_count
+    };
+
     // Store some information about the picture cache slice. This is used when we swap the
     // new scene into the frame builder to either reuse existing slices, or create new ones.
     tile_caches.insert(slice_id, TileCacheParams {
@@ -612,7 +621,7 @@ fn create_tile_cache(
         shared_clip_node_id,
         shared_clip_leaf_id,
         virtual_surface_size: frame_builder_config.compositor_kind.get_virtual_surface_size(),
-        compositor_surface_count: prim_list.compositor_surface_count,
+        overlay_surface_count,
     });
 
     let pic_index = prim_store.pictures.alloc().init(PicturePrimitive::new_image(

gfx/wr/wrench/reftests/compositor-surface/mask.yaml

@@ -0,0 +1,20 @@
+# Test that basic functionality of opaque compositor surfaces with
+# clip mask is blending correctly.
+---
+root:
+  items:
+    - type: rect
+      bounds: [50, 50, 462, 462]
+      color: green
+    - type: clip
+      id: 3
+      complex:
+        - rect: [150, 150, 262, 262]
+          radius: 32
+    - image: checkerboard(2,16,16)
+      bounds: [150, 150, 262, 262]
+      prefer-compositor-surface: true
+      clip-chain: [3]
+    - type: rect
+      bounds: [200, 200, 162, 162]
+      color: [0, 0, 255, 0.5]

gfx/wr/wrench/reftests/compositor-surface/reftest.list

@@ -4,3 +4,5 @@ skip_on(android) fuzzy(2,500) == basic.yaml basic-ref.yaml
 fuzzy(2,1000) == mix-blend.yaml mix-blend-ref.yaml
 != coord-systems.yaml blank.yaml
 fuzzy(2,2500) == filter-overlay.yaml filter-overlay-ref.yaml
+platform(linux,mac) == mask.yaml mask.png
+fuzzy(7,86000) == underlay.yaml underlay.png

gfx/wr/wrench/reftests/compositor-surface/underlay.yaml

@@ -0,0 +1,17 @@
+# Verify that we correctly display an underlay style compositor
+# surface when an overlapping translucent compositor surface
+# exists earlier in the display list.
+# Reference image is public-domain - https://en.wikipedia.org/wiki/File:Barn-yuv.png
+---
+root:
+  items:
+    - image: solid-color(255,0,0,128,299,299)
+      bounds: [100, 100, 299, 299]
+      prefer-compositor-surface: true
+    - type: yuv-image
+      format: planar
+      src-y: barn-y.png
+      src-u: barn-u.png
+      src-v: barn-v.png
+      bounds: [100, 100, 299, 299]
+      prefer-compositor-surface: true