From a8354da3c7645fb0126205cd2d3809f3c589d1f3 Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Tue, 10 Mar 2020 03:52:41 +0000 Subject: [PATCH 1/3] Bug 1620904 - Restrict picture cache debug rects to tile valid rect. r=nical This makes the picture cache debug view more represent the amount of pixels that are being rasterized and composited. It's also a bit clearer where picture cache boundaries are on some pages. Differential Revision: https://phabricator.services.mozilla.com/D65932 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/ca720bdc1f1fc3c41a278d10c3c14b9016bd76e1 --- webrender/src/picture.rs | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/webrender/src/picture.rs b/webrender/src/picture.rs index 6331159735..d4e6c75853 100644 --- a/webrender/src/picture.rs +++ b/webrender/src/picture.rs @@ -4847,6 +4847,7 @@ impl PicturePrimitive { tile.root.draw_debug_rects( &map_pic_to_world, tile.is_opaque, + tile.current_descriptor.local_valid_rect, scratch, frame_context.global_device_pixel_scale, ); @@ -6240,6 +6241,7 @@ impl TileNode { &self, pic_to_world_mapper: &SpaceMapper, is_opaque: bool, + local_valid_rect: PictureRect, scratch: &mut PrimitiveScratchBuffer, global_device_pixel_scale: DevicePixelScale, ) { @@ -6253,22 +6255,27 @@ impl TileNode { debug_colors::YELLOW }; - let world_rect = pic_to_world_mapper.map(&self.rect).unwrap(); - let device_rect = world_rect * global_device_pixel_scale; - - let outer_color = color.scale_alpha(0.3); - let inner_color = outer_color.scale_alpha(0.5); - scratch.push_debug_rect( - device_rect.inflate(-3.0, -3.0), - outer_color, - inner_color - ); + if let Some(local_rect) = local_valid_rect.intersection(&self.rect) { + let world_rect = pic_to_world_mapper + .map(&local_rect) + .unwrap(); + let device_rect = world_rect * global_device_pixel_scale; + + let outer_color = color.scale_alpha(0.3); + let inner_color = outer_color.scale_alpha(0.5); + scratch.push_debug_rect( + device_rect.inflate(-3.0, -3.0), + outer_color, + inner_color + ); + } } TileNodeKind::Node { ref children, .. } => { for child in children.iter() { child.draw_debug_rects( pic_to_world_mapper, is_opaque, + local_valid_rect, scratch, global_device_pixel_scale, ); From 43039564d5f859362c150ebb31786e3566a30f69 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Tue, 10 Mar 2020 03:52:51 +0000 Subject: [PATCH 2/3] Bug 1617879 - Rewrite the check for transformed clip intersection with the primitive in WR r=gw Instead of trying to extract an inner rectangle from a transformed inner rectange, which by itself doesn't have an obviously "best" solution, we are going to test if the visibility rect is within the polygon of the projected inner rect. The test is more precise, could be slightly more heavy, but most importantly - it's correct. Differential Revision: https://phabricator.services.mozilla.com/D65836 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/1692426a9549efd97f3241b8fc314eac0786a750 --- webrender/src/clip.rs | 52 ++++++++++++------- webrender/src/prim_store/image.rs | 2 +- .../reftests/clip/clip-thin-rotated-ref.yaml | 13 +++++ wrench/reftests/clip/clip-thin-rotated.yaml | 41 +++++++++++++++ wrench/reftests/clip/reftest.list | 1 + 5 files changed, 88 insertions(+), 21 deletions(-) create mode 100644 wrench/reftests/clip/clip-thin-rotated-ref.yaml create mode 100644 wrench/reftests/clip/clip-thin-rotated.yaml diff --git a/webrender/src/clip.rs b/webrender/src/clip.rs index 59e6f0d326..1e6d0dcde4 100644 --- a/webrender/src/clip.rs +++ b/webrender/src/clip.rs @@ -107,7 +107,7 @@ use crate::prim_store::{ClipData, ImageMaskData, SpaceMapper, VisibleMaskImageTi use crate::prim_store::{PointKey, SizeKey, RectangleKey}; use crate::render_task_cache::to_cache_size; use crate::resource_cache::{ImageRequest, ResourceCache}; -use std::{cmp, ops, u32}; +use std::{iter, ops, u32}; use crate::util::{extract_inner_rect_safe, project_rect, ScaleOffset}; // Type definitions for interning clip nodes. @@ -1371,10 +1371,8 @@ impl ClipItemKind { } }; - if let Some(inner_clip_rect) = inner_rect.and_then(|ref inner_rect| { - project_inner_rect(transform, inner_rect) - }) { - if inner_clip_rect.contains_rect(&visible_rect) { + if let Some(ref inner_clip_rect) = inner_rect { + if let Some(()) = projected_rect_contains(inner_clip_rect, transform, &visible_rect) { return match mode { ClipMode::Clip => ClipResult::Accept, ClipMode::ClipOut => ClipResult::Reject, @@ -1558,25 +1556,39 @@ pub fn rounded_rectangle_contains_point( true } -pub fn project_inner_rect( +pub fn projected_rect_contains( + source_rect: &LayoutRect, transform: &LayoutToWorldTransform, - rect: &LayoutRect, -) -> Option { + target_rect: &WorldRect, +) -> Option<()> { let points = [ - transform.transform_point2d(rect.origin)?, - transform.transform_point2d(rect.top_right())?, - transform.transform_point2d(rect.bottom_left())?, - transform.transform_point2d(rect.bottom_right())?, + transform.transform_point2d(source_rect.origin)?, + transform.transform_point2d(source_rect.top_right())?, + transform.transform_point2d(source_rect.bottom_right())?, + transform.transform_point2d(source_rect.bottom_left())?, + ]; + let target_points = [ + target_rect.origin, + target_rect.top_right(), + target_rect.bottom_right(), + target_rect.bottom_left(), ]; - let mut xs = [points[0].x, points[1].x, points[2].x, points[3].x]; - let mut ys = [points[0].y, points[1].y, points[2].y, points[3].y]; - xs.sort_by(|a, b| a.partial_cmp(b).unwrap_or(cmp::Ordering::Equal)); - ys.sort_by(|a, b| a.partial_cmp(b).unwrap_or(cmp::Ordering::Equal)); - Some(WorldRect::new( - WorldPoint::new(xs[1], ys[1]), - WorldSize::new(xs[2] - xs[1], ys[2] - ys[1]), - )) + // iterate the edges of the transformed polygon + for (a, b) in points + .iter() + .cloned() + .zip(points[1..].iter().cloned().chain(iter::once(points[0]))) + { + // check if every destination point is on the right of the edge + for &c in target_points.iter() { + if (b - a).cross(c - a) < 0.0 { + return None + } + } + } + + Some(()) } // Add a clip node into the list of clips to be processed diff --git a/webrender/src/prim_store/image.rs b/webrender/src/prim_store/image.rs index bab3a037de..2df32455ef 100644 --- a/webrender/src/prim_store/image.rs +++ b/webrender/src/prim_store/image.rs @@ -118,7 +118,7 @@ pub enum ImageSource { #[cfg_attr(feature = "capture", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] -#[derive(MallocSizeOf)] +#[derive(Debug, MallocSizeOf)] pub struct ImageData { pub key: ApiImageKey, pub stretch_size: LayoutSize, diff --git a/wrench/reftests/clip/clip-thin-rotated-ref.yaml b/wrench/reftests/clip/clip-thin-rotated-ref.yaml new file mode 100644 index 0000000000..e09079424d --- /dev/null +++ b/wrench/reftests/clip/clip-thin-rotated-ref.yaml @@ -0,0 +1,13 @@ +# Test checks if a rotated clip with a long and thin rectangle would still +# correctly affect the primitive with regards to the inner bounds. +--- +root: + items: + - + bounds: [0, 0, 0, 0] + type: "stacking-context" + items: + - + bounds: [100, 100, 14, 14] + type: rect + color: blue diff --git a/wrench/reftests/clip/clip-thin-rotated.yaml b/wrench/reftests/clip/clip-thin-rotated.yaml new file mode 100644 index 0000000000..d0e626af91 --- /dev/null +++ b/wrench/reftests/clip/clip-thin-rotated.yaml @@ -0,0 +1,41 @@ +# Test checks if a rotated clip with a long and thin rectangle would still +# correctly affect the primitive with regards to the inner bounds. +--- +root: + items: + - + bounds: [0, 0, 0, 0] + type: "reference-frame" + id: 2 + - + bounds: [0, 0, 0, 0] + type: "stacking-context" + transform: rotate(45) translate(200, 0) + items: + - + bounds: [0, 0, 20, 1000] + type: clip + id: 5 + # uncomment this to see the clip area + #- + # bounds: [0, 0, 20, 1000] + # type: rect + # color: green + - # we aren't supposed to see this one + bounds: [0, 0, 0, 0] + type: "stacking-context" + clip-and-scroll: [2, 5] + items: + - + bounds: [120, 120, 10, 10] + type: rect + color: red + - + bounds: [0, 0, 0, 0] + type: "stacking-context" + clip-and-scroll: [2, 5] + items: + - + bounds: [100, 100, 14, 14] + type: rect + color: blue diff --git a/wrench/reftests/clip/reftest.list b/wrench/reftests/clip/reftest.list index e8907cac46..a436496bb1 100644 --- a/wrench/reftests/clip/reftest.list +++ b/wrench/reftests/clip/reftest.list @@ -14,3 +14,4 @@ skip_on(android,device) == color_targets(3) alpha_targets(1) stacking-context-cl fuzzy(70,2400) == clip-and-filter-with-rotation.yaml clip-and-filter-with-rotation-ref.yaml color_targets(1) alpha_targets(0) == clip-out-rotation.yaml blank.yaml # Unexpected color targets, see bug 1580795 == clipped-occlusion.yaml clipped-occlusion-ref.yaml +== clip-thin-rotated.yaml clip-thin-rotated-ref.yaml From d82e4e67582a9f58ce789710df22c99379352610 Mon Sep 17 00:00:00 2001 From: Bert Peers Date: Tue, 10 Mar 2020 03:53:21 +0000 Subject: [PATCH 3/3] Bug 1620642 - reftest-analyzer improvements for analyzing test failures r=jgilbert Turn the difference checkbox into a radio that adds "heatmap"; it uses WebGL to show both images, their absolute difference, and a color-coded max difference. The quadrants split following the mouse. This helps to separate large variations (red) from small variations (green) and helps to compare the images without losing track of where they are. Differential Revision: https://phabricator.services.mozilla.com/D65841 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/263ce25c220aeab11843445e3634f29b004f44dc --- wrench/script/reftest-analyzer.xhtml | 224 ++++++++++++++++++++++++++- 1 file changed, 217 insertions(+), 7 deletions(-) diff --git a/wrench/script/reftest-analyzer.xhtml b/wrench/script/reftest-analyzer.xhtml index 876bb9583b..9fad3544b3 100644 --- a/wrench/script/reftest-analyzer.xhtml +++ b/wrench/script/reftest-analyzer.xhtml @@ -67,6 +67,161 @@ Features to add: ]]> + +