Bug 1863688 - Fix translucent + opaque RGBA compositor surface overla…

…ys. r=gfx-reviewers,lsalzman Differential Revision: https://phabricator.services.mozilla.com/D193713 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/00534ecc53abe81a11e756c0beee1566f9472600
servo · Nov 17, 2023 · efd7814 · efd7814
1 parent 2481259
commit efd7814
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 26 deletions.
diff --git a/webrender/src/composite.rs b/webrender/src/composite.rs
@@ -27,7 +27,7 @@ use std::{ops, u64, os::raw::c_void};
 /// 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)]
+#[derive(Debug, Copy, Clone, MallocSizeOf, PartialEq)]
 pub enum CompositorSurfaceKind {
     /// Don't create a native compositor surface, blit it as a regular primitive
     Blit,

diff --git a/webrender/src/picture.rs b/webrender/src/picture.rs
@@ -2455,7 +2455,7 @@ impl TileCacheInstance {
         prim_spatial_node_index: SpatialNodeIndex,
         is_root_tile_cache: bool,
         sub_slice_index: usize,
-        is_opaque: bool,
+        surface_kind: CompositorSurfaceKind,
         frame_context: &FrameVisibilityContext,
     ) -> SurfacePromotionResult {
         // Check if this primitive _wants_ to be promoted to a compositor surface.
@@ -2464,7 +2464,7 @@ impl TileCacheInstance {
         }
 
         // For now, only support a small (arbitrary) number of compositor surfaces.
-        if !is_opaque {
+        if surface_kind == CompositorSurfaceKind::Overlay {
             // Non-opaque compositor surfaces require sub-slices, as they are drawn
             // as overlays.
             if sub_slice_index == self.sub_slices.len() - 1 {
@@ -2553,6 +2553,7 @@ impl TileCacheInstance {
             composite_state,
             image_rendering,
             true,
+            CompositorSurfaceKind::Underlay,
         )
     }
 
@@ -2572,6 +2573,7 @@ impl TileCacheInstance {
         gpu_cache: &mut GpuCache,
         image_rendering: ImageRendering,
         is_opaque: bool,
+        surface_kind: CompositorSurfaceKind,
     ) -> bool {
         let mut api_keys = [ImageKey::DUMMY; 3];
         api_keys[0] = api_key;
@@ -2606,6 +2608,7 @@ impl TileCacheInstance {
             composite_state,
             image_rendering,
             is_opaque,
+            surface_kind,
         )
     }
 
@@ -2626,6 +2629,7 @@ impl TileCacheInstance {
         composite_state: &mut CompositeState,
         image_rendering: ImageRendering,
         is_opaque: bool,
+        surface_kind: CompositorSurfaceKind,
     ) -> bool {
         let map_local_to_surface = SpaceMapper::new_with_target(
             self.spatial_node_index,
@@ -2823,20 +2827,24 @@ impl TileCacheInstance {
 
         // 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,
-            });
+        match surface_kind {
+            CompositorSurfaceKind::Underlay => {
+                self.underlays.push(descriptor);
+            }
+            CompositorSurfaceKind::Overlay => {
+                // 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,
+                });
+            }
+            CompositorSurfaceKind::Blit => unreachable!(),
         }
 
         true
@@ -3083,6 +3091,11 @@ impl TileCacheInstance {
             PrimitiveInstanceKind::Image { data_handle, ref mut compositor_surface_kind, .. } => {
                 let image_key = &data_stores.image[data_handle];
                 let image_data = &image_key.kind;
+
+                // For now, assume that for compositor surface purposes, any RGBA image may be
+                // translucent. See the comment in `add_prim` in this source file for more
+                // details. We'll leave the `is_opaque` code branches here, but disabled, as
+                // in future we will want to support this case correctly.
                 let mut is_opaque = false;
 
                 if let Some(image_properties) = resource_cache.get_image_properties(image_data.key) {
@@ -3112,7 +3125,7 @@ impl TileCacheInstance {
                                                   prim_spatial_node_index,
                                                   is_root_tile_cache,
                                                   sub_slice_index,
-                                                  is_opaque,
+                                                  CompositorSurfaceKind::Overlay,
                                                   frame_context) {
                     SurfacePromotionResult::Failed => {
                     }
@@ -3146,17 +3159,14 @@ impl TileCacheInstance {
                         gpu_cache,
                         image_data.image_rendering,
                         is_opaque,
+                        CompositorSurfaceKind::Overlay,
                     );
                 }
 
                 if promote_to_surface {
-                    if is_opaque {
-                        *compositor_surface_kind = CompositorSurfaceKind::Underlay;
-                    } else {
-                        *compositor_surface_kind = CompositorSurfaceKind::Overlay;
-                        prim_instance.vis.state = VisibilityState::Culled;
-                        return;
-                    }
+                    *compositor_surface_kind = CompositorSurfaceKind::Overlay;
+                    prim_instance.vis.state = VisibilityState::Culled;
+                    return;
                 } else {
                     *compositor_surface_kind = CompositorSurfaceKind::Blit;
 
@@ -3174,7 +3184,7 @@ impl TileCacheInstance {
                                             prim_spatial_node_index,
                                             is_root_tile_cache,
                                             sub_slice_index,
-                                            true,
+                                            CompositorSurfaceKind::Underlay,
                                             frame_context) {
                     SurfacePromotionResult::Failed => false,
                     SurfacePromotionResult::Success => true,

diff --git a/wrench/reftests/compositor-surface/reftest.list b/wrench/reftests/compositor-surface/reftest.list
@@ -6,3 +6,4 @@ fuzzy(2,1000) == mix-blend.yaml mix-blend-ref.yaml
 fuzzy(2,2500) == filter-overlay.yaml filter-overlay-ref.yaml
 platform(linux,mac) == mask.yaml mask.png
 platform(linux) fuzzy(7,86000) == underlay.yaml underlay.png
+fuzzy(2,2600) == transparent-opaque-image.yaml transparent-opaque-image-ref.yaml
diff --git a/wrench/reftests/compositor-surface/transparent-opaque-image-ref.yaml b/wrench/reftests/compositor-surface/transparent-opaque-image-ref.yaml
@@ -0,0 +1,8 @@
+---
+root:
+  items:
+    - type: rect
+      bounds: [50, 50, 362, 362]
+      color: green
+    - image: checkerboard(2,16,16)
+      bounds: [100, 100, 262, 262]
diff --git a/wrench/reftests/compositor-surface/transparent-opaque-image.yaml b/wrench/reftests/compositor-surface/transparent-opaque-image.yaml
@@ -0,0 +1,14 @@
+# Verify that an RGBA compositor surface that is translucent, followed
+# by an RGBA compositor surface that is opaque draws correctly.
+---
+root:
+  items:
+    - type: rect
+      bounds: [50, 50, 362, 362]
+      color: green
+    - image: transparent-checkerboard(2,16,16)
+      bounds: [100, 100, 262, 262]
+      prefer-compositor-surface: true
+    - image: checkerboard(2,16,16)
+      bounds: [100, 100, 262, 262]
+      prefer-compositor-surface: true