Bug 1544374 - Make FilterOp not copy. r=gw
authorNicolas Silva <nsilva@mozilla.com>
Tue, 16 Apr 2019 07:43:04 +0000
changeset 469658 f0c87ec1d694
parent 469657 735492db6be1
child 469660 258af4e91151
child 469661 82531c8a2d73
push id35878
push userapavel@mozilla.com
push dateTue, 16 Apr 2019 15:43:40 +0000
treeherdermozilla-central@258af4e91151 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgw
bugs1544374
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1544374 - Make FilterOp not copy. r=gw Differential Revision: https://phabricator.services.mozilla.com/D27480
gfx/webrender_bindings/src/bindings.rs
gfx/wr/webrender/src/batch.rs
gfx/wr/webrender/src/display_list_flattener.rs
gfx/wr/webrender/src/picture.rs
gfx/wr/webrender_api/src/display_item.rs
gfx/wr/webrender_api/src/display_list.rs
--- a/gfx/webrender_bindings/src/bindings.rs
+++ b/gfx/webrender_bindings/src/bindings.rs
@@ -2059,19 +2059,19 @@ pub extern "C" fn wr_dp_push_stacking_co
     filter_count: usize,
     filter_datas: *const WrFilterData,
     filter_datas_count: usize,
     glyph_raster_space: RasterSpace,
 ) -> WrSpatialId {
     debug_assert!(unsafe { !is_in_render_thread() });
 
     let c_filters = unsafe { make_slice(filters, filter_count) };
-    let mut filters : Vec<FilterOp> = c_filters.iter().map(|c_filter| {
-                                                           *c_filter
-    }).collect();
+    let mut filters : Vec<FilterOp> = c_filters.iter()
+        .map(|c_filter| { c_filter.clone() })
+        .collect();
 
     let c_filter_datas = unsafe { make_slice(filter_datas, filter_datas_count) };
     let r_filter_datas : Vec<FilterData> = c_filter_datas.iter().map(|c_filter_data| {
         FilterData {
             func_r_type: c_filter_data.funcR_type,
             r_values: unsafe { make_slice(c_filter_data.R_values, c_filter_data.R_values_count).to_vec() },
             func_g_type: c_filter_data.funcG_type,
             g_values: unsafe { make_slice(c_filter_data.G_values, c_filter_data.G_values_count).to_vec() },
--- a/gfx/wr/webrender/src/batch.rs
+++ b/gfx/wr/webrender/src/batch.rs
@@ -1250,17 +1250,17 @@ impl AlphaBatchBuilder {
 
                                         self.push_new_batch_list(
                                             Vec::new(),
                                             Vec::new(),
                                         );
                                     }
                                 }
                             }
-                            PictureCompositeMode::Filter(filter) => {
+                            PictureCompositeMode::Filter(ref filter) => {
                                 let surface = ctx.surfaces[raster_config.surface_index.0]
                                     .surface
                                     .as_ref()
                                     .expect("bug: surface must be allocated by now");
                                 assert!(filter.is_visible());
                                 match filter {
                                     FilterOp::Blur(..) => {
                                         let kind = BatchKind::Brush(
--- a/gfx/wr/webrender/src/display_list_flattener.rs
+++ b/gfx/wr/webrender/src/display_list_flattener.rs
@@ -1496,17 +1496,17 @@ impl<'a> DisplayListFlattener<'a> {
                 stacking_context.frame_output_pipeline_id
             ),
         };
 
         // Add picture for this actual stacking context contents to render into.
         let leaf_pic_index = PictureIndex(self.prim_store.pictures
             .alloc()
             .init(PicturePrimitive::new_image(
-                leaf_composite_mode,
+                leaf_composite_mode.clone(),
                 leaf_context_3d,
                 stacking_context.pipeline_id,
                 leaf_output_pipeline_id,
                 true,
                 stacking_context.requested_raster_space,
                 PrimitiveList::new(
                     stacking_context.primitives,
                     &self.interners,
@@ -1604,23 +1604,23 @@ impl<'a> DisplayListFlattener<'a> {
                         };
 
                         let handle = self.interners
                             .filter_data
                             .intern(&filter_data_key, || ());
                         PictureCompositeMode::ComponentTransferFilter(handle)
                     }
                 }
-                _ => PictureCompositeMode::Filter(filter),
+                _ => PictureCompositeMode::Filter(filter.clone()),
             });
 
             let filter_pic_index = PictureIndex(self.prim_store.pictures
                 .alloc()
                 .init(PicturePrimitive::new_image(
-                    composite_mode,
+                    composite_mode.clone(),
                     Picture3DContext::Out,
                     stacking_context.pipeline_id,
                     None,
                     true,
                     stacking_context.requested_raster_space,
                     PrimitiveList::new(
                         vec![cur_instance.clone()],
                         &self.interners,
@@ -1664,17 +1664,17 @@ impl<'a> DisplayListFlattener<'a> {
         // backdrop alpha will be 0, and then the blend equation collapses to just
         // Cs = Cs, and the blend mode isn't taken into account at all.
         let has_mix_blend = if let (Some(mix_blend_mode), false) = (stacking_context.composite_ops.mix_blend_mode, parent_is_empty) {
             let composite_mode = Some(PictureCompositeMode::MixBlend(mix_blend_mode));
 
             let blend_pic_index = PictureIndex(self.prim_store.pictures
                 .alloc()
                 .init(PicturePrimitive::new_image(
-                    composite_mode,
+                    composite_mode.clone(),
                     Picture3DContext::Out,
                     stacking_context.pipeline_id,
                     None,
                     true,
                     stacking_context.requested_raster_space,
                     PrimitiveList::new(
                         vec![cur_instance.clone()],
                         &self.interners,
@@ -2021,17 +2021,17 @@ impl<'a> DisplayListFlattener<'a> {
                     // added to the shadow.
                     if !prims.is_empty() {
                         // Create a picture that the shadow primitives will be added to. If the
                         // blur radius is 0, the code in Picture::prepare_for_render will
                         // detect this and mark the picture to be drawn directly into the
                         // parent picture, which avoids an intermediate surface and blur.
                         let blur_filter = FilterOp::Blur(std_deviation).sanitize();
                         let composite_mode = PictureCompositeMode::Filter(blur_filter);
-                        let composite_mode_key = Some(composite_mode).into();
+                        let composite_mode_key = Some(composite_mode.clone()).into();
 
                         // Pass through configuration information about whether WR should
                         // do the bounding rect inflation for text shadows.
                         let options = PictureOptions {
                             inflate_if_required: pending_shadow.shadow.should_inflate,
                         };
 
                         // Create the primitive to draw the shadow picture into the scene.
--- a/gfx/wr/webrender/src/picture.rs
+++ b/gfx/wr/webrender/src/picture.rs
@@ -1872,17 +1872,17 @@ bitflags! {
         /// Preserve-3D requires a surface for plane-splitting.
         const PRESERVE3D = 4;
     }
 }
 
 /// Specifies how this Picture should be composited
 /// onto the target it belongs to.
 #[allow(dead_code)]
-#[derive(Debug, Copy, Clone)]
+#[derive(Debug, Clone)]
 #[cfg_attr(feature = "capture", derive(Serialize))]
 pub enum PictureCompositeMode {
     /// Apply CSS mix-blend-mode effect.
     MixBlend(MixBlendMode),
     /// Apply a CSS filter (except component transfer).
     Filter(FilterOp),
     /// Apply a component transfer filter.
     ComponentTransferFilter(FilterDataHandle),
@@ -2619,17 +2619,17 @@ impl PicturePrimitive {
 
         // Push information about this pic on stack for children to read.
         state.push_picture(PictureInfo {
             spatial_node_index: self.spatial_node_index,
         });
 
         // See if this picture actually needs a surface for compositing.
         let actual_composite_mode = match self.requested_composite_mode {
-            Some(PictureCompositeMode::Filter(filter)) if filter.is_noop() => None,
+            Some(PictureCompositeMode::Filter(ref filter)) if filter.is_noop() => None,
             Some(PictureCompositeMode::Blit(reason)) if reason == BlitReason::CLIP => {
                 // If the only reason a picture has requested a surface is due to the clip
                 // chain node, we might choose to skip drawing a surface, and instead apply
                 // the clips to each individual primitive. The logic below works out which
                 // option to choose.
 
                 // Assume that we will apply clips to individual items
                 let mut apply_clip_to_picture = false;
@@ -2666,17 +2666,17 @@ impl PicturePrimitive {
                 // If we decided not to use a surfce for clipping, then skip and draw straight
                 // into the parent surface.
                 if apply_clip_to_picture {
                     Some(PictureCompositeMode::Blit(reason))
                 } else {
                     None
                 }
             }
-            mode => mode,
+            ref mode => mode.clone(),
         };
 
         if let Some(composite_mode) = actual_composite_mode {
             // Retrieve the positioning node information for the parent surface.
             let parent_raster_node_index = state.current_surface().raster_spatial_node_index;
             let surface_spatial_node_index = self.spatial_node_index;
 
             // This inflation factor is to be applied to all primitives within the surface.
@@ -3115,18 +3115,18 @@ impl PicturePrimitive {
 
                 self.secondary_render_task_id = Some(readback_task_id);
                 frame_state.surfaces[surface_index.0].tasks.push(readback_task_id);
 
                 let render_task_id = frame_state.render_tasks.add(picture_task);
                 frame_state.surfaces[surface_index.0].tasks.push(render_task_id);
                 PictureSurface::RenderTask(render_task_id)
             }
-            PictureCompositeMode::Filter(filter) => {
-                if let FilterOp::ColorMatrix(m) = filter {
+            PictureCompositeMode::Filter(ref filter) => {
+                if let FilterOp::ColorMatrix(m) = *filter {
                     if let Some(mut request) = frame_state.gpu_cache.request(&mut self.extra_gpu_data_handle) {
                         for i in 0..5 {
                             request.push([m[i*4], m[i*4+1], m[i*4+2], m[i*4+3]]);
                         }
                     }
                 }
 
                 let uv_rect_kind = calculate_uv_rect_kind(
--- a/gfx/wr/webrender_api/src/display_item.rs
+++ b/gfx/wr/webrender_api/src/display_item.rs
@@ -621,17 +621,17 @@ pub enum MixBlendMode {
     Exclusion = 11,
     Hue = 12,
     Saturation = 13,
     Color = 14,
     Luminosity = 15,
 }
 
 #[repr(C)]
-#[derive(Clone, Copy, Debug, PartialEq, Deserialize, Serialize)]
+#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)]
 pub enum FilterOp {
     /// Filter that does no transformation of the colors, needed for
     /// debug purposes only.
     Identity,
     Blur(f32),
     Brightness(f32),
     Contrast(f32),
     Grayscale(f32),
@@ -645,27 +645,27 @@ pub enum FilterOp {
     SrgbToLinear,
     LinearToSrgb,
     ComponentTransfer,
 }
 
 impl FilterOp {
     /// Ensure that the parameters for a filter operation
     /// are sensible.
-    pub fn sanitize(self) -> FilterOp {
+    pub fn sanitize(&self) -> FilterOp {
         match self {
             FilterOp::Blur(radius) => {
                 let radius = radius.min(MAX_BLUR_RADIUS);
                 FilterOp::Blur(radius)
             }
             FilterOp::DropShadow(offset, radius, color) => {
                 let radius = radius.min(MAX_BLUR_RADIUS);
-                FilterOp::DropShadow(offset, radius, color)
+                FilterOp::DropShadow(*offset, radius, *color)
             }
-            filter => filter,
+            filter => filter.clone(),
         }
     }
 }
 
 #[repr(u8)]
 #[derive(Clone, Copy, Debug, PartialEq, Deserialize, Serialize)]
 pub enum ComponentTransferFuncType {
   Identity = 0,
--- a/gfx/wr/webrender_api/src/display_list.rs
+++ b/gfx/wr/webrender_api/src/display_list.rs
@@ -32,23 +32,30 @@ pub const MAX_TEXT_RUN_LENGTH: usize = 2
 // TODO(mrobinson): It would be a good idea to eliminate the root scroll frame which is only
 // used by Servo.
 const FIRST_SPATIAL_NODE_INDEX: usize = 2;
 
 // See ROOT_SCROLL_NODE_SPATIAL_ID
 const FIRST_CLIP_NODE_INDEX: usize = 1;
 
 #[repr(C)]
-#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)]
+#[derive(Debug, Deserialize, Eq, Hash, PartialEq, Serialize)]
 pub struct ItemRange<T> {
     start: usize,
     length: usize,
     _boo: PhantomData<T>,
 }
 
+impl<T> Copy for ItemRange<T> {}
+impl<T> Clone for ItemRange<T> {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
 impl<T> Default for ItemRange<T> {
     fn default() -> Self {
         ItemRange {
             start: 0,
             length: 0,
             _boo: PhantomData,
         }
     }