Backed out changeset ec474d482d90 (bug 1918529) for causing wrench failures.

Author: Sandor Molnar

webrender/src/picture.rs

@@ -4183,8 +4183,6 @@ struct SurfaceAllocInfo {
     // Only used for SVGFEGraph currently, this is the source pixels needed to
     // render the pixels in clipped.
     source: DeviceRect,
-    // Only used for SVGFEGraph, this is the same as clipped before rounding.
-    clipped_notsnapped: DeviceRect,
     clipped_local: PictureRect,
     uv_rect_kind: UvRectKind,
 }
@@ -6380,14 +6378,6 @@ impl PicturePrimitive {
                             )
                         );
 
-                        // Determine the local space to device pixel scaling in the most robust
-                        // way, this accounts for local to device transform and
-                        // device_pixel_scale (if the task is shrunk in get_surface_rects).
-                        let subregion_to_device_scale_x = surface_rects.clipped_notsnapped.width() / surface_rects.clipped_local.width();
-                        let subregion_to_device_scale_y = surface_rects.clipped_notsnapped.height() / surface_rects.clipped_local.height();
-                        let subregion_to_device_offset_x = surface_rects.clipped_notsnapped.min.x - (surface_rects.clipped_local.min.x * subregion_to_device_scale_x).floor();
-                        let subregion_to_device_offset_y = surface_rects.clipped_notsnapped.min.y - (surface_rects.clipped_local.min.y * subregion_to_device_scale_y).floor();
-
                         // Produce the target pixels, this is the result of the
                         // composite op
                         let filter_task_id = request_render_task(
@@ -6406,10 +6396,8 @@ impl PicturePrimitive {
                                     source_subregion.cast_unit(),
                                     target_subregion.cast_unit(),
                                     prim_subregion.cast_unit(),
-                                    subregion_to_device_scale_x,
-                                    subregion_to_device_scale_y,
-                                    subregion_to_device_offset_x,
-                                    subregion_to_device_offset_y,
+                                    surface_rects.clipped.cast_unit(),
+                                    surface_rects.clipped_local.cast_unit(),
                                 )
                             }
                         );
@@ -8062,11 +8050,7 @@ fn get_surface_rects(
         }
     };
 
-    // clipped_local is in LayoutPixel coordinates (technically if the parent
-    // surface has a parent surface this would be PicturePixel), it needs to be
-    // in WorldPixel before we can properly transform it into DevicePixel for
-    // rasterizing.
-    let (clipped, unclipped, source) = if surface.raster_spatial_node_index != surface.surface_spatial_node_index {
+    let (mut clipped, mut unclipped, mut source) = if surface.raster_spatial_node_index != surface.surface_spatial_node_index {
         assert_eq!(surface.device_pixel_scale.0, 1.0);
 
         let local_to_world = SpaceMapper::new_with_target(
@@ -8076,55 +8060,52 @@ fn get_surface_rects(
             spatial_tree,
         );
 
-        let clipped = local_to_world.map(&clipped_local.cast_unit()).unwrap();
-        let unclipped = local_to_world.map(&unclipped_local).unwrap();
-        let source = local_to_world.map(&source_local.cast_unit()).unwrap();
+        let clipped = (local_to_world.map(&clipped_local.cast_unit()).unwrap() * surface.device_pixel_scale).round_out();
+        let unclipped = local_to_world.map(&unclipped_local).unwrap() * surface.device_pixel_scale;
+        let source = (local_to_world.map(&source_local.cast_unit()).unwrap() * surface.device_pixel_scale).round_out();
 
         (clipped, unclipped, source)
     } else {
-        let clipped = clipped_local.cast_unit();
-        let unclipped = unclipped_local.cast_unit();
-        let source = source_local.cast_unit();
+        let clipped = (clipped_local.cast_unit() * surface.device_pixel_scale).round_out();
+        let unclipped = unclipped_local.cast_unit() * surface.device_pixel_scale;
+        let source = (source_local.cast_unit() * surface.device_pixel_scale).round_out();
 
         (clipped, unclipped, source)
     };
 
     // Limit rendering extremely large pictures to something the hardware can
     // handle, considering both clipped (target subregion) and source subregion.
     //
-    // Do this before we apply rounding because the scaling should not vary with
-    // subpixel scrolling.
-    //
-    // Use slightly less than max_surface_size for subpixel rounding, and also
-    // leave room for SVGFEGraph to add a 1 pixel inflate.
-    //
     // If you change this, test with:
     // ./mach crashtest layout/svg/crashtests/387290-1.svg
     let max_dimension =
         clipped.width().max(
             clipped.height().max(
                 source.width().max(
                     source.height()
-                ))) * surface.device_pixel_scale.0;
-    let max_allowed_dimension = max_surface_size - 4.0;
-    if max_allowed_dimension < max_dimension {
+                ))).ceil();
+    if max_dimension > max_surface_size {
+        let max_dimension =
+            clipped_local.width().max(
+                clipped_local.height().max(
+                    source_local.width().max(
+                        source_local.height()
+                    ))).ceil();
         surface.raster_spatial_node_index = surface.surface_spatial_node_index;
-        surface.device_pixel_scale = Scale::new(surface.device_pixel_scale.0 * max_allowed_dimension / max_dimension);
+        surface.device_pixel_scale = Scale::new(max_surface_size / max_dimension);
         surface.local_scale = (1.0, 1.0);
-    }
 
-    // After deciding if downscaling is necessary, apply scaling and round.
-    let clipped = clipped * surface.device_pixel_scale;
-    let unclipped = unclipped * surface.device_pixel_scale;
-    let source = (source * surface.device_pixel_scale).round_out();
-    let clipped_snapped = clipped.round_out();
+        clipped = (clipped_local.cast_unit() * surface.device_pixel_scale).round();
+        unclipped = unclipped_local.cast_unit() * surface.device_pixel_scale;
+        source = (source_local.cast_unit() * surface.device_pixel_scale).round();
+    }
 
-    let task_size = clipped_snapped.size().to_i32();
+    let task_size = clipped.size().to_i32();
     debug_assert!(task_size.width <= max_surface_size as i32);
     debug_assert!(task_size.height <= max_surface_size as i32);
 
     let uv_rect_kind = calculate_uv_rect_kind(
-        clipped_snapped,
+        clipped,
         unclipped,
     );
 
@@ -8144,10 +8125,9 @@ fn get_surface_rects(
     Some(SurfaceAllocInfo {
         task_size,
         needs_scissor_rect,
-        clipped: clipped_snapped,
+        clipped,
         unclipped,
         source,
-        clipped_notsnapped: clipped,
         clipped_local,
         uv_rect_kind,
     })

webrender/src/render_task.rs

@@ -1649,10 +1649,8 @@ impl RenderTask {
         source_subregion: LayoutRect,
         target_subregion: LayoutRect,
         prim_subregion: LayoutRect,
-        subregion_to_device_scale_x: f32,
-        subregion_to_device_scale_y: f32,
-        subregion_to_device_offset_x: f32,
-        subregion_to_device_offset_y: f32,
+        surface_rects_clipped: LayoutRect,
+        surface_rects_clipped_local: LayoutRect,
     ) -> RenderTaskId {
         const BUFFER_LIMIT: usize = SVGFE_GRAPH_MAX;
         let mut task_by_buffer_id: [RenderTaskId; BUFFER_LIMIT] = [RenderTaskId::INVALID; BUFFER_LIMIT];
@@ -1701,6 +1699,20 @@ impl RenderTask {
             }
         }
 
+        // Determine the local space to device pixel scaling in the most robust
+        // way, this accounts for local to device transform and
+        // device_pixel_scale (if the task is shrunk in get_surface_rects).
+        //
+        // This has some precision issues because surface_rects_clipped was
+        // rounded already, so it's not exactly the same transform that
+        // get_surface_rects performed, but it is very close, since it is not
+        // quite the same we have to round the offset a certain way to avoid
+        // introducing subpixel offsets caused by the slight deviation.
+        let subregion_to_device_scale_x = surface_rects_clipped.width() / surface_rects_clipped_local.width();
+        let subregion_to_device_scale_y = surface_rects_clipped.height() / surface_rects_clipped_local.height();
+        let subregion_to_device_offset_x = surface_rects_clipped.min.x - (surface_rects_clipped_local.min.x * subregion_to_device_scale_x).floor();
+        let subregion_to_device_offset_y = surface_rects_clipped.min.y - (surface_rects_clipped_local.min.y * subregion_to_device_scale_y).floor();
+
         // Iterate the filter nodes and create tasks
         let mut made_dependency_on_source = false;
         for (filter_index, (filter_node, op)) in filter_nodes.iter().enumerate() {

wrench/reftests/transforms/reftest.list

@@ -44,7 +44,7 @@ fuzzy(1,10) == flatten-all-flat.yaml flatten-all-flat-ref.yaml
 == strange-w.yaml strange-w-ref.yaml
 == big-axis-aligned-scale.yaml big-axis-aligned-scale-ref.yaml
 # Compare ~8K raster root (>MAX_SURFACE_SIZE) with ~2K raster root. fuzzy due to lerping on edges.
-skip_on(android) fuzzy(59,804) == raster_root_A_8192.yaml raster_root_A_ref.yaml
+skip_on(android) fuzzy(56,804) == raster_root_A_8192.yaml raster_root_A_ref.yaml
 # Same as large-raster-root.yaml but resulting in a 10302×100 raster root (= >4096) vs 4000x100 in ref:
 skip_on(android) fuzzy(60,917) == raster_root_B_8192.yaml raster_root_B_ref.yaml
 # Make sure we don't panic