Bug 1851508 - Fix rendering artifacts on clip masks with nested perspective r=gfx-reviewers,lsalzman

Add support for drawing clip masks in the same reference frame as
the raster space without the perspective transform, which fixes
some complex edge cases with nested complex perspective transforms.

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

gfx/wr/webrender/res/ps_quad_mask.glsl

@@ -4,7 +4,7 @@
 
 #include ps_quad,ellipse
 
-varying highp vec2 vClipLocalPos;
+varying highp vec4 vClipLocalPos;
 
 #ifdef WR_FEATURE_FAST_PATH
 flat varying highp vec3 v_clip_params;      // xy = box size, z = radius
@@ -24,6 +24,9 @@ flat varying highp vec2 vClipMode;
 
 PER_INSTANCE in ivec4 aClipData;
 
+#define CLIP_SPACE_RASTER       0
+#define CLIP_SPACE_PRIMITIVE    1
+
 struct Clip {
     RectWithEndpoint rect;
 #ifdef WR_FEATURE_FAST_PATH
@@ -33,11 +36,14 @@ struct Clip {
     vec4 radii_bottom;
 #endif
     float mode;
+    int space;
 };
 
 Clip fetch_clip(int index) {
     Clip clip;
 
+    clip.space = aClipData.z;
+
 #ifdef WR_FEATURE_FAST_PATH
     vec4 texels[3] = fetch_from_gpu_buffer_3(index);
     clip.rect = RectWithEndpoint(texels[0].xy, texels[0].zw);
@@ -58,16 +64,22 @@ void main(void) {
     PrimitiveInfo prim_info = ps_quad_main();
 
     Clip clip = fetch_clip(aClipData.y);
+    Transform clip_transform = fetch_transform(aClipData.x);
 
+    vClipLocalPos = clip_transform.m * vec4(prim_info.local_pos, 0.0, 1.0);
+
+#ifndef WR_FEATURE_FAST_PATH
+    if (clip.space == CLIP_SPACE_RASTER) {
+        vTransformBounds = vec4(clip.rect.p0, clip.rect.p1);
+    } else {
         RectWithEndpoint xf_bounds = RectWithEndpoint(
             max(clip.rect.p0, prim_info.local_clip_rect.p0),
             min(clip.rect.p1, prim_info.local_clip_rect.p1)
         );
         vTransformBounds = vec4(xf_bounds.p0, xf_bounds.p1);
+    }
+#endif
 
-    Transform clip_transform = fetch_transform(aClipData.x);
-
-    vClipLocalPos = (clip_transform.m * vec4(prim_info.local_pos, 0.0, 1.0)).xy;
     vClipMode.x = clip.mode;
 
 #ifdef WR_FEATURE_FAST_PATH
@@ -75,7 +87,7 @@ void main(void) {
     // signed distance function to get a rounded rect clip.
     vec2 half_size = 0.5 * (clip.rect.p1 - clip.rect.p0);
     float radius = clip.radii.x;
-    vClipLocalPos -= (half_size + clip.rect.p0);
+    vClipLocalPos.xy -= (half_size + clip.rect.p0) * vClipLocalPos.w;
     v_clip_params = vec3(half_size - vec2(radius), radius);
 #else
     vec2 r_tl = clip.radii_top.xy;
@@ -135,13 +147,14 @@ float sd_rounded_box(in vec2 pos, in vec2 box_size, in float radius) {
 #endif
 
 void main(void) {
-    float aa_range = compute_aa_range(vClipLocalPos);
+    vec2 clip_local_pos = vClipLocalPos.xy / vClipLocalPos.w;
+    float aa_range = compute_aa_range(clip_local_pos);
 
 #ifdef WR_FEATURE_FAST_PATH
-    float dist = sd_rounded_box(vClipLocalPos, v_clip_params.xy, v_clip_params.z);
+    float dist = sd_rounded_box(clip_local_pos, v_clip_params.xy, v_clip_params.z);
 #else
     float dist = distance_to_rounded_rect(
-        vClipLocalPos,
+        clip_local_pos,
         vClipPlane_TL,
         vClipCenter_Radius_TL,
         vClipPlane_TR,

gfx/wr/webrender/src/gpu_types.rs

@@ -583,6 +583,15 @@ pub struct QuadSegment {
     pub task_id: RenderTaskId,
 }
 
+#[derive(Copy, Debug, Clone, PartialEq)]
+#[cfg_attr(feature = "capture", derive(Serialize))]
+#[cfg_attr(feature = "replay", derive(Deserialize))]
+#[repr(u32)]
+pub enum ClipSpace {
+    Raster = 0,
+    Primitive = 1,
+}
+
 #[repr(C)]
 #[derive(Clone)]
 #[cfg_attr(feature = "capture", derive(Serialize))]
@@ -591,7 +600,8 @@ pub struct MaskInstance {
     pub prim: PrimitiveInstanceData,
     pub clip_transform_id: TransformPaletteId,
     pub clip_address: i32,
-    pub info: [i32; 2],
+    pub clip_space: ClipSpace,
+    pub unused: i32,
 }
 
 

gfx/wr/webrender/src/prepare.rs

@@ -1299,8 +1299,24 @@ fn prepare_interned_prim_for_render(
 
                 let content_origin = DevicePoint::new(x0, y0);
 
+                let pic_surface_index = pic.raster_config.as_ref().unwrap().surface_index;
+                let prim_local_rect = frame_state
+                    .surfaces[pic_surface_index.0]
+                    .clipped_local_rect
+                    .cast_unit();
+
+                let main_prim_address = write_prim_blocks(
+                    frame_state.frame_gpu_data,
+                    prim_local_rect,
+                    prim_instance.vis.clip_chain.local_clip_rect,
+                    PremultipliedColorF::WHITE,
+                    &[],
+                );
+
                 let masks = MaskSubPass {
                     clip_node_range: prim_instance.vis.clip_chain.clips_range,
+                    prim_spatial_node_index,
+                    main_prim_address,
                 };
 
                 let parent_task_id = if in_place_mask {
@@ -2121,6 +2137,8 @@ fn add_segment(
 
         let masks = MaskSubPass {
             clip_node_range: prim_instance.vis.clip_chain.clips_range,
+            prim_spatial_node_index,
+            main_prim_address,
         };
 
         let task = frame_state.rg_builder.get_task_mut(task_id);

gfx/wr/webrender/src/render_target.rs

@@ -10,13 +10,12 @@ use crate::batch::{ClipBatcher, BatchBuilder, INVALID_SEGMENT_INDEX, ClipMaskIns
 use crate::command_buffer::{CommandBufferList, QuadFlags};
 use crate::segment::EdgeAaSegmentMask;
 use crate::spatial_tree::SpatialTree;
-use crate::space::SpaceMapper;
 use crate::clip::{ClipStore, ClipItemKind};
 use crate::frame_builder::{FrameGlobalResources};
 use crate::gpu_cache::{GpuCache, GpuCacheAddress};
 use crate::gpu_types::{BorderInstance, SvgFilterInstance, BlurDirection, BlurInstance, PrimitiveHeaders, ScalingInstance};
-use crate::gpu_types::{TransformPalette, ZBufferIdGenerator, TransformPaletteId, MaskInstance};
+use crate::gpu_types::{TransformPalette, ZBufferIdGenerator, MaskInstance, ClipSpace};
-use crate::gpu_types::{ZBufferId, QuadSegment, PrimitiveInstanceData};
+use crate::gpu_types::{ZBufferId, QuadSegment, PrimitiveInstanceData, TransformPaletteId};
 use crate::internal_types::{FastHashMap, TextureSource, CacheTextureId};
 use crate::picture::{SliceId, SurfaceInfo, ResolvedSurfaceTexture, TileCacheInstance};
 use crate::prepare::write_prim_blocks;
@@ -25,7 +24,7 @@ use crate::prim_store::gradient::{
     FastLinearGradientInstance, LinearGradientInstance, RadialGradientInstance,
     ConicGradientInstance,
 };
-use crate::renderer::{GpuBufferBuilder};
+use crate::renderer::{GpuBufferBuilder, GpuBufferAddress};
 use crate::render_backend::DataStores;
 use crate::render_task::{RenderTaskKind, RenderTaskAddress, SubPass};
 use crate::render_task::{RenderTask, ScalingTask, SvgFilterInfo, MaskSubPass};
@@ -959,9 +958,10 @@ pub struct LineDecorationJob {
 fn build_mask_tasks(
     info: &MaskSubPass,
     render_task_address: RenderTaskAddress,
+    task_world_rect: WorldRect,
     target_rect: DeviceIntRect,
-    content_rect: DeviceRect,
+    main_prim_address: GpuBufferAddress,
-    device_pixel_scale: DevicePixelScale,
+    prim_spatial_node_index: SpatialNodeIndex,
     raster_spatial_node_index: SpatialNodeIndex,
     clip_store: &ClipStore,
     data_stores: &DataStores,
@@ -971,41 +971,11 @@ fn build_mask_tasks(
     render_tasks: &RenderTaskGraph,
     results: &mut ClipMaskInstanceList,
 ) {
-    let task_world_rect = content_rect / device_pixel_scale;
-
     for i in 0 .. info.clip_node_range.count {
         let clip_instance = clip_store.get_instance_from_range(&info.clip_node_range, i);
         let clip_node = &data_stores.clip[clip_instance.handle];
 
-        let is_same_coord_system = spatial_tree.is_matching_coord_system(
+        let (clip_address, fast_path) = match clip_node.item.kind {
-            clip_node.item.spatial_node_index,
-            raster_spatial_node_index,
-        );
-
-        let clip_needs_scissor_rect = !is_same_coord_system;
-        let quad_flags = if is_same_coord_system {
-            QuadFlags::APPLY_DEVICE_CLIP
-        } else {
-            QuadFlags::empty()
-        };
-
-        let clip_transform_id = transforms.get_id(
-            clip_node.item.spatial_node_index,
-            raster_spatial_node_index,
-            spatial_tree,
-        );
-
-        // Work out a local space rect for the clip that will ensure we write to every
-        // pixel covered by the primitive. For 2d cases, this will be exact. For complex
-        // perspective cases, it will be a conservative estimate.
-        let mapper = SpaceMapper::new_with_target(
-            raster_spatial_node_index,
-            clip_node.item.spatial_node_index,
-            task_world_rect,
-            spatial_tree,
-        );
-
-        let (clip_address, clip_rect, fast_path) = match clip_node.item.kind {
             ClipItemKind::RoundedRectangle { rect, radius, mode } => {
                 let (fast_path, clip_address) = if radius.is_uniform().is_some() {
                     let mut writer = gpu_buffer_builder.write_blocks(3);
@@ -1036,7 +1006,7 @@ fn build_mask_tasks(
                     (false, clip_address)
                 };
 
-                (clip_address, rect, fast_path)
+                (clip_address, fast_path)
             }
             ClipItemKind::Rectangle { rect, mode, .. } => {
                 assert_eq!(mode, ClipMode::Clip);
@@ -1047,12 +1017,30 @@ fn build_mask_tasks(
                 writer.push_one([mode as i32 as f32, 0.0, 0.0, 0.0]);
                 let clip_address = writer.finish();
 
-                (clip_address, rect, true)
+                (clip_address, true)
             }
             ClipItemKind::BoxShadow { .. } => {
                 panic!("bug: box-shadow clips not expected on non-legacy rect/quads");
             }
             ClipItemKind::Image { rect, .. } => {
+                let clip_transform_id = transforms.get_id(
+                    clip_node.item.spatial_node_index,
+                    raster_spatial_node_index,
+                    spatial_tree,
+                );
+
+                let is_same_coord_system = spatial_tree.is_matching_coord_system(
+                    prim_spatial_node_index,
+                    raster_spatial_node_index,
+                );
+
+                let clip_needs_scissor_rect = !is_same_coord_system;
+                let mut quad_flags = QuadFlags::SAMPLE_AS_MASK;
+
+                if is_same_coord_system {
+                    quad_flags |= QuadFlags::APPLY_DEVICE_CLIP;
+                }
+
                 for tile in clip_store.visible_mask_tiles(&clip_instance) {
                     let clip_prim_address = write_prim_blocks(
                         gpu_buffer_builder,
@@ -1073,7 +1061,7 @@ fn build_mask_tasks(
                         render_task_address,
                         clip_transform_id,
                         clip_prim_address,
-                        quad_flags | QuadFlags::SAMPLE_AS_MASK,
+                        quad_flags,
                         EdgeAaSegmentMask::empty(),
                         0,
                         tile.task_id,
@@ -1104,32 +1092,72 @@ fn build_mask_tasks(
             }
         };
 
-        let bounding_rect = match mapper.unmap(&task_world_rect) {
+        let prim_spatial_node = spatial_tree.get_spatial_node(prim_spatial_node_index);
-            Some(rect) => rect,
+        let clip_spatial_node = spatial_tree.get_spatial_node(clip_node.item.spatial_node_index);
-            None => {
+        let raster_spatial_node = spatial_tree.get_spatial_node(raster_spatial_node_index);
-                // TODO(gw): This doesn't seem right - it may need to be expanded to cover
+        let raster_clip = raster_spatial_node.coordinate_system_id == clip_spatial_node.coordinate_system_id;
-                //           the primitive region. However, I cannot get any test cases to
-                //           fail - let's see if we get any regressions here and work out
-                //           the correct way to handle any cases that arise.
-                //           Should also assert on coordinate system, perhaps?
-                clip_rect
-            }
-        };
 
-        let clip_prim_address = write_prim_blocks(
+        let (clip_space, clip_transform_id, main_prim_address, prim_transform_id, is_same_coord_system) = if raster_clip {
+            let prim_transform_id = TransformPaletteId::IDENTITY;
+
+            let clip_transform_id = transforms.get_id(
+                raster_spatial_node_index,
+                clip_node.item.spatial_node_index,
+                spatial_tree,
+            );
+
+            let main_prim_address = write_prim_blocks(
                 gpu_buffer_builder,
-            bounding_rect,
+                task_world_rect.cast_unit(),
-            bounding_rect,
+                task_world_rect.cast_unit(),
                 PremultipliedColorF::WHITE,
                 &[],
             );
 
+            (ClipSpace::Raster, clip_transform_id, main_prim_address, prim_transform_id, true)
+        } else {
+            let prim_transform_id = transforms.get_id(
+                prim_spatial_node_index,
+                raster_spatial_node_index,
+                spatial_tree,
+            );
+
+            let clip_transform_id = if prim_spatial_node.coordinate_system_id < clip_spatial_node.coordinate_system_id {
+                transforms.get_id(
+                    clip_node.item.spatial_node_index,
+                    prim_spatial_node_index,
+                    spatial_tree,
+                )
+            } else {
+                transforms.get_id(
+                    prim_spatial_node_index,
+                    clip_node.item.spatial_node_index,
+                    spatial_tree,
+                )
+            };
+
+            let is_same_coord_system = spatial_tree.is_matching_coord_system(
+                prim_spatial_node_index,
+                raster_spatial_node_index,
+            );
+
+            (ClipSpace::Primitive, clip_transform_id, main_prim_address, prim_transform_id, is_same_coord_system)
+        };
+
+        let clip_needs_scissor_rect = !is_same_coord_system;
+
+        let quad_flags = if is_same_coord_system {
+            QuadFlags::APPLY_DEVICE_CLIP
+        } else {
+            QuadFlags::empty()
+        };
+
         add_quad_to_batch(
             render_task_address,
-            clip_transform_id,
+            prim_transform_id,
-            clip_prim_address,
+            main_prim_address,
             quad_flags,
-            EdgeAaSegmentMask::empty(),
+            EdgeAaSegmentMask::all(),
             INVALID_SEGMENT_INDEX as u8,
             RenderTaskId::INVALID,
             ZBufferId(0),
@@ -1137,9 +1165,10 @@ fn build_mask_tasks(
             |_, prim| {
                 let instance = MaskInstance {
                     prim,
-                    clip_transform_id: TransformPaletteId::IDENTITY,
+                    clip_transform_id,
                     clip_address: clip_address.as_int(),
-                    info: [0; 2],
+                    clip_space,
+                    unused: 0,
                 };
 
                 if clip_needs_scissor_rect {
@@ -1204,9 +1233,10 @@ fn build_sub_pass(
                 build_mask_tasks(
                     masks,
                     render_task_address,
+                    content_rect / device_pixel_scale,
                     target_rect,
-                    content_rect,
+                    masks.main_prim_address,
-                    device_pixel_scale,
+                    masks.prim_spatial_node_index,
                     raster_spatial_node_index,
                     ctx.clip_store,
                     ctx.data_stores,

gfx/wr/webrender/src/render_task.rs

@@ -882,6 +882,8 @@ pub type TaskDependencies = SmallVec<[RenderTaskId;2]>;
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 pub struct MaskSubPass {
     pub clip_node_range: ClipNodeRange,
+    pub prim_spatial_node_index: SpatialNodeIndex,
+    pub main_prim_address: GpuBufferAddress,
 }
 
 #[cfg_attr(feature = "capture", derive(Serialize))]

gfx/wr/webrender/src/spatial_tree.rs

@@ -24,7 +24,7 @@ use peek_poke::PeekPoke;
 /// coordinate system has an id and those ids will be shared when the coordinates
 /// system are the same or are in the same axis-aligned space. This allows
 /// for optimizing mask generation.
-#[derive(Debug, Copy, Clone, PartialEq)]
+#[derive(Debug, Copy, Clone, PartialEq, PartialOrd)]
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 pub struct CoordinateSystemId(pub u32);

testing/web-platform/meta/css/motion/offset-path-shape-circle-001.html.ini

@@ -0,0 +1,3 @@
+[offset-path-shape-circle-001.html]
+  fuzzy:
+    if swgl: maxDifference=255-255;totalPixels=117-117

testing/web-platform/meta/css/motion/offset-path-shape-circle-004.html.ini

@@ -0,0 +1,3 @@
+[offset-path-shape-circle-004.html]
+  fuzzy:
+    if swgl: maxDifference=255-255;totalPixels=303-303

testing/web-platform/meta/css/motion/offset-path-shape-ellipse-001.html.ini

@@ -0,0 +1,3 @@
+[offset-path-shape-ellipse-001.html]
+  fuzzy:
+    if swgl: maxDifference=255-255;totalPixels=109-109

testing/web-platform/meta/css/motion/offset-path-url-003.html.ini

@@ -0,0 +1,3 @@
+[offset-path-url-003.html]
+  fuzzy:
+    if swgl: maxDifference=255-255;totalPixels=70-70

testing/web-platform/meta/css/motion/offset-path-url-004.html.ini

@@ -0,0 +1,3 @@
+[offset-path-url-004.html]
+  fuzzy:
+    if swgl: maxDifference=255-255;totalPixels=60-60