Bug 1530242 - Enable some picture caching invalidation tests in wrench. r=emilio
authorGlenn Watson <github@intuitionlibrary.com>
Mon, 25 Feb 2019 22:25:22 +0000
changeset 518876 f0fb6764fdb7cd0bae97e21db7eb1cdab9cd9f9f
parent 518875 f5629bb3e671ad15ede908de3cc0b2b2be342728
child 518877 d744e4b8444357f5507c22ced0da7e6dac2fe441
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1530242
milestone67.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 1530242 - Enable some picture caching invalidation tests in wrench. r=emilio A number of small tweaks to enable the picture caching invalidation tests. With this in place, we can start adding more test coverage for various invalidation scenarios. - Make the reference image render after the test images, so that dirty region tracking is more intuitive. - Instead of replaying the same frame in wrench to ensure frames are caching, try to cache tiles every frame when testing mode is enabled. - Add a basic invalidation test for a rectangle with color that changes each frame. - Make the dirty region index implicit and rename dirty_region to dirty in reftest format. - Fix an underflow error when moving to next frame in wrench. Differential Revision: https://phabricator.services.mozilla.com/D20963
gfx/wr/webrender/src/lib.rs
gfx/wr/webrender/src/picture.rs
gfx/wr/wrench/reftests/invalidation/one-rounded-rect-green.yaml
gfx/wr/wrench/reftests/invalidation/reftest.list
gfx/wr/wrench/src/reftest.rs
gfx/wr/wrench/src/yaml_frame_reader.rs
--- a/gfx/wr/webrender/src/lib.rs
+++ b/gfx/wr/webrender/src/lib.rs
@@ -205,17 +205,16 @@ extern crate rand;
 pub extern crate webrender_api;
 extern crate webrender_build;
 
 #[doc(hidden)]
 pub use device::{build_shader_strings, ReadPixelsFormat, UploadMethod, VertexUsageHint};
 pub use device::{ProgramBinary, ProgramCache, ProgramCacheObserver};
 pub use device::Device;
 pub use frame_builder::ChasePrimitive;
-pub use picture::FRAMES_BEFORE_PICTURE_CACHING;
 pub use profiler::{ProfilerHooks, set_profiler_hooks};
 pub use renderer::{AsyncPropertySampler, CpuProfile, DebugFlags, OutputImageHandler, RendererKind};
 pub use renderer::{ExternalImage, ExternalImageHandler, ExternalImageSource, GpuProfile};
 pub use renderer::{GraphicsApi, GraphicsApiInfo, PipelineInfo, Renderer, RendererOptions};
 pub use renderer::{RenderResults, RendererStats, SceneBuilderHooks, ThreadListener, ShaderPrecacheFlags};
 pub use renderer::MAX_VERTEX_TEXTURE_WIDTH;
 pub use shade::{Shaders, WrShaders};
 pub use webrender_api as api;
--- a/gfx/wr/webrender/src/picture.rs
+++ b/gfx/wr/webrender/src/picture.rs
@@ -99,17 +99,17 @@ pub struct TileIndex(pub usize);
 ///
 /// Note that we use a separate, smaller size during wrench testing, so that
 /// we get tighter dirty rects and can do more meaningful invalidation
 /// tests.
 const TILE_SIZE_WIDTH: i32 = 1024;
 const TILE_SIZE_HEIGHT: i32 = 256;
 const TILE_SIZE_TESTING: i32 = 64;
 
-pub const FRAMES_BEFORE_PICTURE_CACHING: usize = 2;
+const FRAMES_BEFORE_PICTURE_CACHING: usize = 2;
 const MAX_DIRTY_RECTS: usize = 3;
 
 /// The maximum size per axis of a surface,
 ///  in WorldPixel coordinates.
 const MAX_SURFACE_SIZE: f32 = 4096.0;
 
 
 /// The maximum number of primitives to look for in a display
@@ -1530,17 +1530,19 @@ impl TileCache {
                             debug_colors::RED,
                         );
                     }
                 }
 
                 // Only cache tiles that have had the same content for at least two
                 // frames. This skips caching on pages / benchmarks that are changing
                 // every frame, which is wasteful.
-                if tile.same_frames >= FRAMES_BEFORE_PICTURE_CACHING {
+                // When we are testing invalidation, we want WR to try to cache tiles each
+                // frame, to make it simpler to define the expected dirty rects.
+                if tile.same_frames >= FRAMES_BEFORE_PICTURE_CACHING || frame_context.config.testing {
                     // Ensure that this texture is allocated.
                     if !resource_cache.texture_cache.is_allocated(&tile.handle) {
                         resource_cache.texture_cache.update_picture_cache(
                             &mut tile.handle,
                             gpu_cache,
                         );
                     }
 
new file mode 100644
--- /dev/null
+++ b/gfx/wr/wrench/reftests/invalidation/one-rounded-rect-green.yaml
@@ -0,0 +1,17 @@
+---
+root:
+  items:
+    -
+      bounds: 0 0 1000 1000
+      type: stacking-context
+      cache: true
+      items:
+        - type: clip
+          bounds: [50, 50, 200, 200]
+          complex:
+            - rect: [50, 50, 200, 200]
+              radius: 8
+          items:
+          - type: rect
+            bounds: 50 50 200 200
+            color: green
--- a/gfx/wr/wrench/reftests/invalidation/reftest.list
+++ b/gfx/wr/wrench/reftests/invalidation/reftest.list
@@ -1,1 +1,2 @@
-== one-rounded-rect.yaml two-rounded-rects.yaml rounded-rects.yaml rounded-rects.yaml
+# Test that a red -> green -> red rect correctly invalidates each frame
+dirty([(50,50):256x256]) one-rounded-rect.yaml dirty([(50,50):256x256]) one-rounded-rect-green.yaml dirty([(50,50):256x256]) one-rounded-rect.yaml == one-rounded-rect.yaml
--- a/gfx/wr/wrench/src/reftest.rs
+++ b/gfx/wr/wrench/src/reftest.rs
@@ -81,18 +81,19 @@ impl ExtraCheck {
     fn run(&self, results: &[RenderResults]) -> bool {
         match *self {
             ExtraCheck::DrawCalls(x) =>
                 x == results.last().unwrap().stats.total_draw_calls,
             ExtraCheck::AlphaTargets(x) =>
                 x == results.last().unwrap().stats.alpha_target_count,
             ExtraCheck::ColorTargets(x) =>
                 x == results.last().unwrap().stats.color_target_count,
-            ExtraCheck::DirtyRegion { index, ref region } =>
-                *region == format!("{}", results[index].recorded_dirty_regions[0]),
+            ExtraCheck::DirtyRegion { index, ref region } => {
+                *region == format!("{}", results[index].recorded_dirty_regions[0])
+            }
         }
     }
 }
 
 pub struct Reftest {
     op: ReftestOp,
     test: Vec<PathBuf>,
     reference: PathBuf,
@@ -216,16 +217,17 @@ impl ReftestManifest {
             let mut max_difference = 0;
             let mut max_count = 0;
             let mut op = ReftestOp::Equal;
             let mut font_render_mode = None;
             let mut extra_checks = vec![];
             let mut disable_dual_source_blending = false;
             let mut zoom_factor = 1.0;
             let mut allow_mipmaps = false;
+            let mut dirty_region_index = 0;
 
             let mut paths = vec![];
             for (i, token) in tokens.iter().enumerate() {
                 match *token {
                     "include" => {
                         assert!(i == 0, "include must be by itself");
                         let include = dir.join(tokens[1]);
 
@@ -258,21 +260,24 @@ impl ReftestManifest {
                     function if function.starts_with("alpha_targets") => {
                         let (_, args, _) = parse_function(function);
                         extra_checks.push(ExtraCheck::AlphaTargets(args[0].parse().unwrap()));
                     }
                     function if function.starts_with("color_targets") => {
                         let (_, args, _) = parse_function(function);
                         extra_checks.push(ExtraCheck::ColorTargets(args[0].parse().unwrap()));
                     }
-                    function if function.starts_with("dirty_region") => {
+                    function if function.starts_with("dirty") => {
                         let (_, args, _) = parse_function(function);
-                        let index: usize = args[0].parse().unwrap();
-                        let region: String = args[1].parse().unwrap();
-                        extra_checks.push(ExtraCheck::DirtyRegion { index, region });
+                        let region: String = args[0].parse().unwrap();
+                        extra_checks.push(ExtraCheck::DirtyRegion {
+                            index: dirty_region_index,
+                            region,
+                        });
+                        dirty_region_index += 1;
                     }
                     options if options.starts_with("options") => {
                         let (_, args, _) = parse_function(options);
                         if args.iter().any(|arg| arg == &OPTION_DISABLE_SUBPX) {
                             font_render_mode = Some(FontRenderMode::Alpha);
                         }
                         if args.iter().any(|arg| arg == &OPTION_DISABLE_AA) {
                             font_render_mode = Some(FontRenderMode::Mono);
@@ -398,61 +403,57 @@ impl<'a> ReftestHarness<'a> {
             self.wrench
                 .api
                 .send_debug_cmd(
                     DebugCommand::EnableDualSourceBlending(false)
                 );
         }
 
         let window_size = self.window.get_inner_size();
-        let reference = match t.reference.extension().unwrap().to_str().unwrap() {
-            "yaml" => {
-                let output = self.render_yaml(
-                    &t.reference,
-                    window_size,
-                    t.font_render_mode,
-                    t.allow_mipmaps,
-                );
-                output.image
-            }
-            "png" => {
-                self.load_image(t.reference.as_path(), ImageFormat::PNG)
-            }
+        let reference_image = match t.reference.extension().unwrap().to_str().unwrap() {
+            "yaml" => None,
+            "png" => Some(self.load_image(t.reference.as_path(), ImageFormat::PNG)),
             other => panic!("Unknown reftest extension: {}", other),
         };
+        let test_size = reference_image.as_ref().map_or(window_size, |img| img.size);
 
         // The reference can be smaller than the window size, in which case
         // we only compare the intersection.
         //
         // Note also that, when we have multiple test scenes in sequence, we
         // want to test the picture caching machinery. But since picture caching
         // only takes effect after the result has been the same several frames in
         // a row, we need to render the scene multiple times.
         let mut images = vec![];
         let mut results = vec![];
-        let num_iterations = if t.test.len() > 1 {
-            webrender::FRAMES_BEFORE_PICTURE_CACHING
-        } else {
-            1
-        };
+
         for filename in t.test.iter() {
-            let mut output = None;
-            for _ in 0..num_iterations {
-                output = Some(self.render_yaml(
-                    &filename,
-                    reference.size,
-                    t.font_render_mode,
-                    t.allow_mipmaps,
-                ));
-            }
-            let output = output.unwrap();
+            let output = self.render_yaml(
+                &filename,
+                test_size,
+                t.font_render_mode,
+                t.allow_mipmaps,
+            );
             images.push(output.image);
             results.push(output.results);
         }
 
+        let reference = match reference_image {
+            Some(image) => image,
+            None => {
+                let output = self.render_yaml(
+                    &t.reference,
+                    test_size,
+                    t.font_render_mode,
+                    t.allow_mipmaps,
+                );
+                output.image
+            }
+        };
+
         if t.disable_dual_source_blending {
             self.wrench
                 .api
                 .send_debug_cmd(
                     DebugCommand::EnableDualSourceBlending(true)
                 );
         }
 
--- a/gfx/wr/wrench/src/yaml_frame_reader.rs
+++ b/gfx/wr/wrench/src/yaml_frame_reader.rs
@@ -1894,17 +1894,17 @@ impl WrenchThing for YamlFrameReader {
 
     fn next_frame(&mut self) {
         let mut max_frame_count = 0;
         if let Some(ref keyframes) = self.keyframes {
             for (_, values) in keyframes.as_hash().unwrap() {
                 max_frame_count = max_frame_count.max(values.as_vec().unwrap().len());
             }
         }
-        if self.requested_frame < max_frame_count - 1 {
+        if self.requested_frame + 1 < max_frame_count {
             self.requested_frame += 1;
         }
     }
 
     fn prev_frame(&mut self) {
         if self.requested_frame > 0 {
             self.requested_frame -= 1;
         }