From 40205f1b55f1ba8a26fcbd362c20d843e8718a87 Mon Sep 17 00:00:00 2001 From: Csoregi Natalia Date: Thu, 4 Feb 2021 21:21:32 +0200 Subject: [PATCH] Backed out changeset 0788e77d2d62 (bug 1690396) for causing crashes in Bug 1690846. a=backout --- gfx/wr/webrender/res/brush_mix_blend.glsl | 75 ++++++----------------- gfx/wr/webrender/src/batch.rs | 10 +-- gfx/wr/webrender/src/frame_builder.rs | 6 -- gfx/wr/webrender/src/picture.rs | 71 +-------------------- gfx/wr/webrender/src/render_target.rs | 6 +- gfx/wr/webrender/src/render_task.rs | 45 +++----------- gfx/wr/webrender/src/renderer/mod.rs | 39 ++++++------ gfx/wr/wrench/src/wrench.rs | 1 + gfx/wr/wrench/src/yaml_helper.rs | 2 - 9 files changed, 63 insertions(+), 192 deletions(-) diff --git a/gfx/wr/webrender/res/brush_mix_blend.glsl b/gfx/wr/webrender/res/brush_mix_blend.glsl index f576a23764d6..ff86e780f45c 100644 --- a/gfx/wr/webrender/res/brush_mix_blend.glsl +++ b/gfx/wr/webrender/res/brush_mix_blend.glsl @@ -7,40 +7,16 @@ #include shared,prim_shared,brush -// UV and bounds for the source image +// xy: uv coorinates. varying vec2 v_src_uv; -flat varying vec4 v_src_uv_sample_bounds; -// UV and bounds for the backdrop image +// xy: uv coorinates. varying vec2 v_backdrop_uv; -flat varying vec4 v_backdrop_uv_sample_bounds; -// mix-blend op, and perspective interpolation control -flat varying float v_perspective; flat varying int v_op; #ifdef WR_VERTEX_SHADER -void get_uv( - int res_address, - vec2 f, - ivec2 texture_size, - float perspective_f, - out vec2 out_uv, - out vec4 out_uv_sample_bounds -) { - ImageResource res = fetch_image_resource(res_address); - vec2 uv0 = res.uv_rect.p0; - vec2 uv1 = res.uv_rect.p1; - - vec2 inv_texture_size = vec2(1.0) / vec2(texture_size); - f = get_image_quad_uv(res_address, f); - vec2 uv = mix(uv0, uv1, f); - - out_uv = uv * inv_texture_size * perspective_f; - out_uv_sample_bounds = vec4(uv0 + vec2(0.5), uv1 - vec2(0.5)) * inv_texture_size.xyxy; -} - void brush_vs( VertexInfo vi, int prim_address, @@ -53,29 +29,25 @@ void brush_vs( int brush_flags, vec4 unused ) { - vec2 f = (vi.local_pos - local_rect.p0) / local_rect.size; - float perspective_interpolate = (brush_flags & BRUSH_FLAG_PERSPECTIVE_INTERPOLATION) != 0 ? 1.0 : 0.0; - float perspective_f = mix(vi.world_pos.w, 1.0, perspective_interpolate); - v_perspective = perspective_interpolate; + //Note: this is unsafe for `vi.world_pos.w <= 0.0` + vec2 device_pos = vi.world_pos.xy * pic_task.device_pixel_scale / max(0.0, vi.world_pos.w); + vec2 backdrop_texture_size = vec2(textureSize(sColor0, 0)); + vec2 src_texture_size = vec2(textureSize(sColor1, 0)); v_op = prim_user_data.x; - get_uv( - prim_user_data.y, - f, - textureSize(sColor0, 0).xy, - 1.0, - v_backdrop_uv, - v_backdrop_uv_sample_bounds - ); + PictureTask src_task = fetch_picture_task(prim_user_data.z); + vec2 src_device_pos = vi.world_pos.xy * (src_task.device_pixel_scale / max(0.0, vi.world_pos.w)); + vec2 src_uv = src_device_pos + + src_task.common_data.task_rect.p0 - + src_task.content_origin; + v_src_uv = src_uv / src_texture_size; - get_uv( - prim_user_data.z, - f, - textureSize(sColor1, 0).xy, - perspective_f, - v_src_uv, - v_src_uv_sample_bounds - ); + RenderTaskCommonData backdrop_task = fetch_render_task_common_data(prim_user_data.y); + float src_to_backdrop_scale = pic_task.device_pixel_scale / src_task.device_pixel_scale; + vec2 backdrop_uv = device_pos + + backdrop_task.task_rect.p0 - + src_task.content_origin * src_to_backdrop_scale; + v_backdrop_uv = backdrop_uv / backdrop_texture_size; } #endif @@ -238,15 +210,8 @@ const int MixBlendMode_Color = 14; const int MixBlendMode_Luminosity = 15; Fragment brush_fs() { - float perspective_divisor = mix(gl_FragCoord.w, 1.0, v_perspective); - - vec2 src_uv = v_src_uv * perspective_divisor; - src_uv = clamp(src_uv, v_src_uv_sample_bounds.xy, v_src_uv_sample_bounds.zw); - - vec2 backdrop_uv = clamp(v_backdrop_uv, v_backdrop_uv_sample_bounds.xy, v_backdrop_uv_sample_bounds.zw); - - vec4 Cb = texture(sColor0, backdrop_uv); - vec4 Cs = texture(sColor1, src_uv); + vec4 Cb = texture(sColor0, v_backdrop_uv); + vec4 Cs = texture(sColor1, v_src_uv); // The mix-blend-mode functions assume no premultiplied alpha if (Cb.a != 0.0) { diff --git a/gfx/wr/webrender/src/batch.rs b/gfx/wr/webrender/src/batch.rs index 871e0107c8b2..27f2b8f65e76 100644 --- a/gfx/wr/webrender/src/batch.rs +++ b/gfx/wr/webrender/src/batch.rs @@ -59,6 +59,7 @@ pub enum BrushBatchKind { Blend, MixBlend { task_id: RenderTaskId, + source_id: RenderTaskId, backdrop_id: RenderTaskId, }, YuvImage(ImageBufferKind, YuvFormat, ColorDepth, YuvColorSpace, ColorRange), @@ -1978,6 +1979,7 @@ impl BatchBuilder { BatchKind::Brush( BrushBatchKind::MixBlend { task_id: self.batchers[0].render_task_id, + source_id: cache_task_id, backdrop_id, }, ), @@ -1999,12 +2001,12 @@ impl BatchBuilder { clip_mask: clip_mask_texture_id, }, ); - let src_uv_address = render_tasks[cache_task_id].get_texture_address(gpu_cache); - let readback_uv_address = render_tasks[backdrop_id].get_texture_address(gpu_cache); + let backdrop_task_address: RenderTaskAddress = backdrop_id.into(); + let source_task_address: RenderTaskAddress = cache_task_id.into(); let prim_header_index = prim_headers.push(&prim_header, z_id, [ mode as u32 as i32, - readback_uv_address.as_int(), - src_uv_address.as_int(), + backdrop_task_address.0 as i32, + source_task_address.0 as i32, 0, ]); diff --git a/gfx/wr/webrender/src/frame_builder.rs b/gfx/wr/webrender/src/frame_builder.rs index 98882e15ca86..24a64a49c92b 100644 --- a/gfx/wr/webrender/src/frame_builder.rs +++ b/gfx/wr/webrender/src/frame_builder.rs @@ -210,12 +210,10 @@ impl<'a> FrameBuildingState<'a> { &mut self, surface_index: SurfaceIndex, tasks: Vec, - device_rect: DeviceRect, ) { let surface = &mut self.surfaces[surface_index.0]; assert!(surface.render_tasks.is_none()); surface.render_tasks = Some(SurfaceRenderTasks::Tiled(tasks)); - surface.device_rect = Some(device_rect); } /// Initialize render tasks for a simple surface, that contains only a @@ -225,12 +223,10 @@ impl<'a> FrameBuildingState<'a> { surface_index: SurfaceIndex, task_id: RenderTaskId, parent_surface_index: SurfaceIndex, - device_rect: DeviceRect, ) { let surface = &mut self.surfaces[surface_index.0]; assert!(surface.render_tasks.is_none()); surface.render_tasks = Some(SurfaceRenderTasks::Simple(task_id)); - surface.device_rect = Some(device_rect); self.add_child_render_task( parent_surface_index, @@ -247,12 +243,10 @@ impl<'a> FrameBuildingState<'a> { root_task_id: RenderTaskId, port_task_id: RenderTaskId, parent_surface_index: SurfaceIndex, - device_rect: DeviceRect, ) { let surface = &mut self.surfaces[surface_index.0]; assert!(surface.render_tasks.is_none()); surface.render_tasks = Some(SurfaceRenderTasks::Chained { root_task_id, port_task_id }); - surface.device_rect = Some(device_rect); self.add_child_render_task( parent_surface_index, diff --git a/gfx/wr/webrender/src/picture.rs b/gfx/wr/webrender/src/picture.rs index 8238f9adfc44..da777f424e0e 100644 --- a/gfx/wr/webrender/src/picture.rs +++ b/gfx/wr/webrender/src/picture.rs @@ -4067,8 +4067,6 @@ pub struct SurfaceInfo { pub device_pixel_scale: DevicePixelScale, /// The scale factors of the surface to raster transform. pub scale_factors: (f32, f32), - /// The allocated device rect for this surface - pub device_rect: Option, } impl SurfaceInfo { @@ -4107,13 +4105,8 @@ impl SurfaceInfo { inflation_factor, device_pixel_scale, scale_factors, - device_rect: None, } } - - pub fn get_device_rect(&self) -> DeviceRect { - self.device_rect.expect("bug: queried before surface was initialized") - } } #[derive(Debug)] @@ -5099,7 +5092,6 @@ impl PicturePrimitive { frame_state.init_surface_tiled( surface_index, surface_tasks, - device_clip_rect, ); } Some(ref mut raster_config) => { @@ -5307,7 +5299,6 @@ impl PicturePrimitive { blur_render_task_id, picture_task_id, parent_surface_index, - device_rect, ); } PictureCompositeMode::Filter(Filter::DropShadows(ref shadows)) => { @@ -5405,7 +5396,6 @@ impl PicturePrimitive { blur_render_task_id, picture_task_id, parent_surface_index, - device_rect, ); } PictureCompositeMode::MixBlend(..) if !frame_context.fb_config.gpu_supports_advanced_blend => { @@ -5425,55 +5415,10 @@ impl PicturePrimitive { device_pixel_scale, ); - let parent_surface = &frame_state.surfaces[parent_surface_index.0]; - let parent_raster_spatial_node_index = parent_surface.raster_spatial_node_index; - let parent_device_pixel_scale = parent_surface.device_pixel_scale; - - // Create a space mapper that will allow mapping from the local rect - // of the mix-blend primitive into the space of the surface that we - // need to read back from. Note that we use the parent's raster spatial - // node here, so that we are in the correct device space of the parent - // surface, whether it establishes a raster root or not. - let map_pic_to_parent = SpaceMapper::new_with_target( - parent_raster_spatial_node_index, - self.spatial_node_index, - RasterRect::max_rect(), // TODO(gw): May need a conservative estimate? - frame_context.spatial_tree, - ); - let pic_in_raster_space = map_pic_to_parent - .map(&pic_rect) - .expect("bug: unable to map mix-blend content into parent"); - - // Apply device pixel ratio for parent surface to get into device - // pixels for that surface. - let backdrop_rect = raster_rect_to_device_pixels( - pic_in_raster_space, - parent_device_pixel_scale, - ); - - let parent_surface_rect = parent_surface.get_device_rect(); - let backdrop_rect = backdrop_rect - .intersection(&parent_surface_rect) - .expect("bug: readback rect outside parent rect clip"); - - // Calculate the UV coords necessary for the shader to sampler - // from the primitive rect within the readback region. This is - // 0..1 for aligned surfaces, but doing it this way allows - // accurate sampling if the primitive bounds have fractional values. - let backdrop_uv = calculate_uv_rect_kind( - &pic_rect, - &map_pic_to_parent.get_transform(), - &backdrop_rect, - parent_device_pixel_scale, - ); - let readback_task_id = frame_state.rg_builder.add().init( RenderTask::new_dynamic( - backdrop_rect.size.to_i32(), - RenderTaskKind::new_readback( - backdrop_rect.origin, - backdrop_uv, - ), + clipped.size.to_i32(), + RenderTaskKind::new_readback(), ) ); @@ -5508,7 +5453,6 @@ impl PicturePrimitive { raster_config.surface_index, render_task_id, parent_surface_index, - clipped, ); } PictureCompositeMode::Filter(..) => { @@ -5553,7 +5497,6 @@ impl PicturePrimitive { raster_config.surface_index, render_task_id, parent_surface_index, - clipped, ); } PictureCompositeMode::ComponentTransferFilter(..) => { @@ -5597,7 +5540,6 @@ impl PicturePrimitive { raster_config.surface_index, render_task_id, parent_surface_index, - clipped, ); } PictureCompositeMode::MixBlend(..) | @@ -5642,7 +5584,6 @@ impl PicturePrimitive { raster_config.surface_index, render_task_id, parent_surface_index, - clipped, ); } PictureCompositeMode::SvgFilter(ref primitives, ref filter_datas) => { @@ -5698,17 +5639,9 @@ impl PicturePrimitive { filter_task_id, picture_task_id, parent_surface_index, - clipped, ); } } - - // Update the device pixel ratio in the surface, in case it was adjusted due - // to the surface being too large. This ensures the correct scale is available - // in case it's used as input to a parent mix-blend-mode readback. - frame_state - .surfaces[raster_config.surface_index.0] - .device_pixel_scale = device_pixel_scale; } None => {} }; diff --git a/gfx/wr/webrender/src/render_target.rs b/gfx/wr/webrender/src/render_target.rs index c8e486f5e8fe..9e67179d5af7 100644 --- a/gfx/wr/webrender/src/render_target.rs +++ b/gfx/wr/webrender/src/render_target.rs @@ -408,7 +408,7 @@ impl RenderTarget for ColorRenderTarget { RenderTaskKind::LineDecoration(..) => { panic!("Should not be added to color target!"); } - RenderTaskKind::Readback(..) => {} + RenderTaskKind::Readback => {} RenderTaskKind::Scaling(ref info) => { add_scaling_instances( info, @@ -530,7 +530,7 @@ impl RenderTarget for AlphaRenderTarget { let (target_rect, _) = task.get_target_rect(); match task.kind { - RenderTaskKind::Readback(..) | + RenderTaskKind::Readback | RenderTaskKind::Picture(..) | RenderTaskKind::Blit(..) | RenderTaskKind::Border(..) | @@ -752,7 +752,7 @@ impl TextureCacheRenderTarget { RenderTaskKind::Picture(..) | RenderTaskKind::ClipRegion(..) | RenderTaskKind::CacheMask(..) | - RenderTaskKind::Readback(..) | + RenderTaskKind::Readback | RenderTaskKind::Scaling(..) | RenderTaskKind::SvgFilter(..) => { panic!("BUG: unexpected task kind for texture cache target"); diff --git a/gfx/wr/webrender/src/render_task.rs b/gfx/wr/webrender/src/render_task.rs index a9e569f855e8..445f045acae1 100644 --- a/gfx/wr/webrender/src/render_task.rs +++ b/gfx/wr/webrender/src/render_task.rs @@ -297,18 +297,6 @@ pub struct SvgFilterTask { pub uv_rect_kind: UvRectKind, } -#[cfg_attr(feature = "capture", derive(Serialize))] -#[cfg_attr(feature = "replay", derive(Deserialize))] -pub struct ReadbackTask { - // The offset of the rect that needs to be read back, in the - // device space of the surface that will be read back from - pub readback_origin: DevicePoint, - // UV rect of the readback rect within the readback task area - pub uv_rect_kind: UvRectKind, - // GPU cache handle that provides uv_rect_kind to shaders - pub uv_rect_handle: GpuCacheHandle, -} - #[derive(Debug)] #[cfg_attr(feature = "capture", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] @@ -324,7 +312,7 @@ pub enum RenderTaskKind { ClipRegion(ClipRegionTask), VerticalBlur(BlurTask), HorizontalBlur(BlurTask), - Readback(ReadbackTask), + Readback, Scaling(ScalingTask), Blit(BlitTask), Border(BorderTask), @@ -343,7 +331,7 @@ impl RenderTaskKind { RenderTaskKind::ClipRegion(..) => "ClipRegion", RenderTaskKind::VerticalBlur(..) => "VerticalBlur", RenderTaskKind::HorizontalBlur(..) => "HorizontalBlur", - RenderTaskKind::Readback(..) => "Readback", + RenderTaskKind::Readback => "Readback", RenderTaskKind::Scaling(..) => "Scaling", RenderTaskKind::Blit(..) => "Blit", RenderTaskKind::Border(..) => "Border", @@ -358,7 +346,7 @@ impl RenderTaskKind { pub fn target_kind(&self) -> RenderTargetKind { match *self { RenderTaskKind::LineDecoration(..) | - RenderTaskKind::Readback(..) | + RenderTaskKind::Readback | RenderTaskKind::Border(..) | RenderTaskKind::Gradient(..) | RenderTaskKind::Picture(..) | @@ -431,17 +419,8 @@ impl RenderTaskKind { }) } - pub fn new_readback( - readback_origin: DevicePoint, - uv_rect_kind: UvRectKind, - ) -> Self { - RenderTaskKind::Readback( - ReadbackTask { - readback_origin, - uv_rect_kind, - uv_rect_handle: GpuCacheHandle::new(), - } - ) + pub fn new_readback() -> Self { + RenderTaskKind::Readback } pub fn new_line_decoration( @@ -649,7 +628,7 @@ impl RenderTaskKind { task.blur_region.height as f32, ] } - RenderTaskKind::Readback(..) | + RenderTaskKind::Readback | RenderTaskKind::Scaling(..) | RenderTaskKind::Border(..) | RenderTaskKind::LineDecoration(..) | @@ -706,9 +685,7 @@ impl RenderTaskKind { RenderTaskKind::SvgFilter(ref mut info) => { (&mut info.uv_rect_handle, info.uv_rect_kind) } - RenderTaskKind::Readback(ref mut info) => { - (&mut info.uv_rect_handle, info.uv_rect_kind) - } + RenderTaskKind::Readback | RenderTaskKind::Scaling(..) | RenderTaskKind::Blit(..) | RenderTaskKind::ClipRegion(..) | @@ -1382,7 +1359,7 @@ impl RenderTask { pub fn uv_rect_kind(&self) -> UvRectKind { match self.kind { RenderTaskKind::CacheMask(..) | - RenderTaskKind::Readback(..) => { + RenderTaskKind::Readback => { unreachable!("bug: unexpected render task"); } @@ -1430,10 +1407,8 @@ impl RenderTask { RenderTaskKind::SvgFilter(ref info) => { gpu_cache.get_address(&info.uv_rect_handle) } - RenderTaskKind::Readback(ref info) => { - gpu_cache.get_address(&info.uv_rect_handle) - } RenderTaskKind::ClipRegion(..) | + RenderTaskKind::Readback | RenderTaskKind::Scaling(..) | RenderTaskKind::Blit(..) | RenderTaskKind::Border(..) | @@ -1533,7 +1508,7 @@ impl RenderTask { pt.new_level("HorizontalBlur".to_owned()); task.print_with(pt); } - RenderTaskKind::Readback(..) => { + RenderTaskKind::Readback => { pt.new_level("Readback".to_owned()); } RenderTaskKind::Scaling(ref kind) => { diff --git a/gfx/wr/webrender/src/renderer/mod.rs b/gfx/wr/webrender/src/renderer/mod.rs index 8f0cea4caf65..302b833652a5 100644 --- a/gfx/wr/webrender/src/renderer/mod.rs +++ b/gfx/wr/webrender/src/renderer/mod.rs @@ -2489,6 +2489,7 @@ impl Renderer { &mut self, draw_target: DrawTarget, uses_scissor: bool, + source: &RenderTask, backdrop: &RenderTask, readback: &RenderTask, ) { @@ -2503,19 +2504,16 @@ impl Renderer { let (cache_texture, _) = self.texture_resolver .resolve(&texture_source).expect("bug: no source texture"); - // Extract the rectangle in the backdrop surface's device space of where - // we need to read from. - let readback_origin = match readback.kind { - RenderTaskKind::Readback(ref info) => info.readback_origin, - _ => unreachable!(), - }; - // Before submitting the composite batch, do the // framebuffer readbacks that are needed for each // composite operation in this batch. let (readback_rect, readback_layer) = readback.get_target_rect(); let (backdrop_rect, _) = backdrop.get_target_rect(); - let (backdrop_screen_origin, _) = match backdrop.kind { + let (backdrop_screen_origin, backdrop_scale) = match backdrop.kind { + RenderTaskKind::Picture(ref task_info) => (task_info.content_origin, task_info.device_pixel_scale), + _ => panic!("bug: composite on non-picture?"), + }; + let (source_screen_origin, source_scale) = match source.kind { RenderTaskKind::Picture(ref task_info) => (task_info.content_origin, task_info.device_pixel_scale), _ => panic!("bug: composite on non-picture?"), }; @@ -2530,24 +2528,28 @@ impl Renderer { false, ); - let src_origin = readback_origin + - backdrop_rect.origin.to_f32().to_vector() - - backdrop_screen_origin.to_vector(); + let source_in_backdrop_space = source_screen_origin * (backdrop_scale.0 / source_scale.0); - let src = DeviceIntRect::new( - src_origin.to_i32(), + let mut src = DeviceIntRect::new( + (source_in_backdrop_space + (backdrop_rect.origin.to_f32() - backdrop_screen_origin)).to_i32(), readback_rect.size, ); - - // Should always be drawing to picture cache tiles or off-screen surface! - debug_assert!(!draw_target.is_default()); + let mut dest = readback_rect.to_i32(); let device_to_framebuffer = Scale::new(1i32); + // Need to invert the y coordinates and flip the image vertically when + // reading back from the framebuffer. + if draw_target.is_default() { + src.origin.y = draw_target.dimensions().height as i32 - src.size.height - src.origin.y; + dest.origin.y += dest.size.height; + dest.size.height = -dest.size.height; + } + self.device.blit_render_target( draw_target.into(), src * device_to_framebuffer, cache_draw_target, - readback_rect * device_to_framebuffer, + dest * device_to_framebuffer, TextureFilter::Linear, ); @@ -2897,13 +2899,14 @@ impl Renderer { } // Handle special case readback for composites. - if let BatchKind::Brush(BrushBatchKind::MixBlend { task_id, backdrop_id }) = batch.key.kind { + if let BatchKind::Brush(BrushBatchKind::MixBlend { task_id, source_id, backdrop_id }) = batch.key.kind { // composites can't be grouped together because // they may overlap and affect each other. debug_assert_eq!(batch.instances.len(), 1); self.handle_readback_composite( draw_target, uses_scissor, + &render_tasks[source_id], &render_tasks[task_id], &render_tasks[backdrop_id], ); diff --git a/gfx/wr/wrench/src/wrench.rs b/gfx/wr/wrench/src/wrench.rs index 52c67473a631..058aefbca3f0 100644 --- a/gfx/wr/wrench/src/wrench.rs +++ b/gfx/wr/wrench/src/wrench.rs @@ -266,6 +266,7 @@ impl Wrench { testing: true, max_texture_size: Some(8196), // Needed for rawtest::test_resize_image. allow_dual_source_blending: !disable_dual_source_blending, + allow_advanced_blend_equation: true, dump_shader_source, // SWGL doesn't support the GL_ALWAYS depth comparison function used by // `clear_caches_with_quads`, but scissored clears work well. diff --git a/gfx/wr/wrench/src/yaml_helper.rs b/gfx/wr/wrench/src/yaml_helper.rs index f9974efdc751..dc55b1e860ce 100644 --- a/gfx/wr/wrench/src/yaml_helper.rs +++ b/gfx/wr/wrench/src/yaml_helper.rs @@ -52,8 +52,6 @@ fn string_to_color(color: &str) -> Option { "white" => Some(ColorF::new(1.0, 1.0, 1.0, 1.0)), "black" => Some(ColorF::new(0.0, 0.0, 0.0, 1.0)), "yellow" => Some(ColorF::new(1.0, 1.0, 0.0, 1.0)), - "cyan" => Some(ColorF::new(0.0, 1.0, 1.0, 1.0)), - "magenta" => Some(ColorF::new(1.0, 0.0, 1.0, 1.0)), "transparent" => Some(ColorF::new(1.0, 1.0, 1.0, 0.0)), s => { let items: Vec = s.split_whitespace()