Bug 1697346 - Remove snapping of inflated surface rects. r=aosmond,gf…

…x-reviewers

When an offscreen surface establishes a raster root, this code
was causing incorrect snapping / rounded (even on non-inflated
surfaces), resulting in test failures in some cases.

In theory, this should not be necessary, since scroll offsets are
snapped, and primitives are already snapped during scene building.
Additionally, the picture surface allocation code expects surfaces
with fractional offsets and handles this case (by rounding out the
allocation size, and creating a UV set that samples from the subpixel
offsets of the surface).

In practice, there may be content that relies on this which isn't
tested by CI, so let's land this as a separate patch and see if it
causes any real-world content regressions, before landing the
changes that rely on this.

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

[ghsync] From https://hg.mozilla.org/mozilla-central/rev/7647c3e90bedc40aca04283df9c74ec9da52ef18

webrender/src/picture.rs

@@ -126,7 +126,7 @@ use crate::render_task::{BlurTask, RenderTask, RenderTaskLocation, BlurTaskCache
 use crate::render_task::{StaticRenderTaskSurface, RenderTaskKind};
 use crate::renderer::BlendMode;
 use crate::resource_cache::{ResourceCache, ImageGeneration, ImageRequest};
-use crate::space::{SpaceMapper, SpaceSnapper};
+use crate::space::SpaceMapper;
 use crate::scene::SceneProperties;
 use smallvec::SmallVec;
 use std::{mem, u8, marker, u32};
@@ -6247,23 +6247,8 @@ impl PicturePrimitive {
         if let Some(ref mut raster_config) = self.raster_config {
             let surface = state.current_surface_mut();
             // Inflate the local bounding rect if required by the filter effect.
-            // This inflaction factor is to be applied to the surface itself.
             if self.options.inflate_if_required {
                 surface.rect = raster_config.composite_mode.inflate_picture_rect(surface.rect, surface.scale_factors);
-
-                // The picture's local rect is calculated as the union of the
-                // snapped primitive rects, which should result in a snapped
-                // local rect, unless it was inflated. This is also done during
-                // update visibility when calculating the picture's precise
-                // local rect.
-                let snap_surface_to_raster = SpaceSnapper::new_with_target(
-                    surface.raster_spatial_node_index,
-                    self.spatial_node_index,
-                    surface.device_pixel_scale,
-                    frame_context.spatial_tree,
-                );
-
-                surface.rect = snap_surface_to_raster.snap_rect(&surface.rect);
             }
 
             let mut surface_rect = surface.rect * Scale::new(1.0);

webrender/src/visibility.rs

@@ -26,7 +26,7 @@ use crate::prim_store::{PrimitiveStore, PrimitiveInstance};
 use crate::render_backend::{DataStores, ScratchBuffer};
 use crate::resource_cache::ResourceCache;
 use crate::scene::SceneProperties;
-use crate::space::{SpaceMapper, SpaceSnapper};
+use crate::space::SpaceMapper;
 use crate::internal_types::Filter;
 use crate::util::{MaxRect};
 
@@ -626,22 +626,8 @@ pub fn update_primitive_visibility(
     //           remove this invalidation here completely.
     if let Some(ref rc) = pic.raster_config {
         // Inflate the local bounding rect if required by the filter effect.
-        // This inflaction factor is to be applied to the surface itstore.
         if pic.options.inflate_if_required {
-            // The picture's local rect is calculated as the union of the
-            // snapped primitive rects, which should result in a snapped
-            // local rect, unless it was inflated. This is also done during
-            // surface configuration when calculating the picture's
-            // estimated local rect.
-            let snap_pic_to_raster = SpaceSnapper::new_with_target(
-                surface.raster_spatial_node_index,
-                pic.spatial_node_index,
-                surface.device_pixel_scale,
-                frame_context.spatial_tree,
-            );
-
             surface_rect = rc.composite_mode.inflate_picture_rect(surface_rect, surface.scale_factors);
-            surface_rect = snap_pic_to_raster.snap_rect(&surface_rect);
         }
 
         // Layout space for the picture is picture space from the

wrench/reftests/snap/reftest.list

@@ -1,3 +1,4 @@
 platform(linux,mac) == snap.yaml snap.png
 == transform.yaml transform.png
 platform(linux,mac) == preserve-3d.yaml preserve-3d.png
+fuzzy(128,200) == subpixel-raster-root.yaml subpixel-raster-root-ref.yaml

wrench/reftests/snap/subpixel-raster-root-ref.yaml

@@ -0,0 +1,7 @@
+---
+root:
+  items:
+    -
+      bounds: [0, 111, 200, 1]
+      type: rect
+      color: green

wrench/reftests/snap/subpixel-raster-root.yaml

@@ -0,0 +1,13 @@
+# Verify that we don't incorrectly snap surface rects with fractional pixel offsets
+---
+root:
+  items:
+    -
+      type: "stacking-context"
+      transform: translate(0, 100.5, 0)
+      transform-style: preserve-3d
+      items:
+        -
+          bounds: [0, 10.5, 200, 1]
+          type: rect
+          color: green