Surface Synchronization: Use SurfaceRange in SurfaceDrawQuad

Now that we have a SurfaceRange object, this CL replaces the
primary and fallback SurfaceIds in SurfaceDrawQuad with SurfaceRange
and updates all usages appropriately. This results in a modest line
count reduction and improves readability.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I2d5a6e5b173772a44e39f2093d13c9c56d6c6cb1
Bug: 672962
TBR: yfriedman@chromium.org, boliu@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1145044
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576910}

android_webview/browser/surfaces_instance.cc

@@ -142,8 +142,9 @@ void SurfacesInstance::DrawAndSwap(const gfx::Size& viewport,
   viz::SurfaceDrawQuad* surface_quad =
       render_pass->CreateAndAppendDrawQuad<viz::SurfaceDrawQuad>();
   surface_quad->SetNew(quad_state, gfx::Rect(quad_state->quad_layer_rect),
-                       gfx::Rect(quad_state->quad_layer_rect), child_id,
-                       base::nullopt, SK_ColorWHITE, false);
+                       gfx::Rect(quad_state->quad_layer_rect),
+                       viz::SurfaceRange(base::nullopt, child_id),
+                       SK_ColorWHITE, false);
 
   viz::CompositorFrame frame;
   // We draw synchronously, so acknowledge a manual BeginFrame.

cc/layers/surface_layer_impl.cc

@@ -116,8 +116,7 @@ void SurfaceLayerImpl::AppendQuads(viz::RenderPass* render_pass,
   if (!surface_range_.IsValid())
     return;
 
-  auto* primary = CreateSurfaceDrawQuad(render_pass, surface_range_.end(),
-                                        surface_range_.start());
+  auto* primary = CreateSurfaceDrawQuad(render_pass, surface_range_);
   if (primary && surface_range_.end() != surface_range_.start()) {
     // Add the primary surface ID as a dependency.
     append_quads_data->activation_dependencies.push_back(surface_range_.end());
@@ -141,9 +140,8 @@ bool SurfaceLayerImpl::is_surface_layer() const {
 
 viz::SurfaceDrawQuad* SurfaceLayerImpl::CreateSurfaceDrawQuad(
     viz::RenderPass* render_pass,
-    const viz::SurfaceId& primary_surface_id,
-    const base::Optional<viz::SurfaceId>& fallback_surface_id) {
-  DCHECK(primary_surface_id.is_valid());
+    const viz::SurfaceRange& surface_range) {
+  DCHECK(surface_range.end().is_valid());
 
   float device_scale_factor = layer_tree_impl()->device_scale_factor();
 
@@ -168,9 +166,9 @@ viz::SurfaceDrawQuad* SurfaceLayerImpl::CreateSurfaceDrawQuad(
 
   auto* surface_draw_quad =
       render_pass->CreateAndAppendDrawQuad<viz::SurfaceDrawQuad>();
-  surface_draw_quad->SetNew(
-      shared_quad_state, quad_rect, visible_quad_rect, primary_surface_id,
-      fallback_surface_id, background_color(), stretch_content_to_fill_bounds_);
+  surface_draw_quad->SetNew(shared_quad_state, quad_rect, visible_quad_rect,
+                            surface_range, background_color(),
+                            stretch_content_to_fill_bounds_);
 
   return surface_draw_quad;
 }

cc/layers/surface_layer_impl.h

@@ -71,8 +71,7 @@ class CC_EXPORT SurfaceLayerImpl : public LayerImpl {
  private:
   viz::SurfaceDrawQuad* CreateSurfaceDrawQuad(
       viz::RenderPass* render_pass,
-      const viz::SurfaceId& surface_id,
-      const base::Optional<viz::SurfaceId>& fallback_surface_id);
+      const viz::SurfaceRange& surface_range);
 
   void GetDebugBorderProperties(SkColor* color, float* width) const override;
   void AppendRainbowDebugBorder(viz::RenderPass* render_pass);

cc/layers/surface_layer_impl_unittest.cc

@@ -157,17 +157,17 @@ TEST(SurfaceLayerImplTest, SurfaceLayerImplWithTwoDifferentSurfaces) {
       viz::SurfaceDrawQuad::MaterialCast(render_pass->quad_list.ElementAt(2));
   ASSERT_TRUE(surface_draw_quad3);
 
-  EXPECT_EQ(surface_id1, surface_draw_quad1->primary_surface_id);
+  EXPECT_EQ(surface_id1, surface_draw_quad1->surface_range.end());
   EXPECT_EQ(SK_ColorBLUE, surface_draw_quad1->default_background_color);
-  EXPECT_EQ(surface_id2, surface_draw_quad1->fallback_surface_id);
+  EXPECT_EQ(surface_id2, surface_draw_quad1->surface_range.start());
 
-  EXPECT_EQ(surface_id1, surface_draw_quad2->primary_surface_id);
+  EXPECT_EQ(surface_id1, surface_draw_quad2->surface_range.end());
   EXPECT_EQ(SK_ColorBLUE, surface_draw_quad2->default_background_color);
-  EXPECT_EQ(base::nullopt, surface_draw_quad2->fallback_surface_id);
+  EXPECT_EQ(base::nullopt, surface_draw_quad2->surface_range.start());
 
-  EXPECT_EQ(surface_id1, surface_draw_quad3->primary_surface_id);
+  EXPECT_EQ(surface_id1, surface_draw_quad3->surface_range.end());
   EXPECT_EQ(SK_ColorBLUE, surface_draw_quad3->default_background_color);
-  EXPECT_EQ(surface_id2, surface_draw_quad3->fallback_surface_id);
+  EXPECT_EQ(surface_id2, surface_draw_quad3->surface_range.start());
 }
 
 // This test verifies that if one SurfaceLayerImpl has a deadline
@@ -256,8 +256,8 @@ TEST(SurfaceLayerImplTest, SurfaceLayerImplWithMatchingPrimaryAndFallback) {
       viz::SurfaceDrawQuad::MaterialCast(render_pass->quad_list.ElementAt(0));
   ASSERT_TRUE(surface_draw_quad1);
 
-  EXPECT_EQ(surface_id1, surface_draw_quad1->primary_surface_id);
-  EXPECT_EQ(surface_id1, surface_draw_quad1->fallback_surface_id);
+  EXPECT_EQ(surface_id1, surface_draw_quad1->surface_range.end());
+  EXPECT_EQ(surface_id1, surface_draw_quad1->surface_range.start());
   EXPECT_EQ(SK_ColorBLUE, surface_draw_quad1->default_background_color);
 }
 

components/viz/client/hit_test_data_provider_draw_quad.cc

@@ -41,9 +41,9 @@ base::Optional<HitTestRegionList> HitTestDataProviderDrawQuad::GetHitTestData(
         // Skip the quad if the FrameSinkId between fallback and primary is not
         // the same, because we don't know which FrameSinkId would be used to
         // draw this quad.
-        if (surface_quad->fallback_surface_id.has_value() &&
-            surface_quad->fallback_surface_id->frame_sink_id() !=
-                surface_quad->primary_surface_id.frame_sink_id()) {
+        if (surface_quad->surface_range.start() &&
+            surface_quad->surface_range.start()->frame_sink_id() !=
+                surface_quad->surface_range.end().frame_sink_id()) {
           continue;
         }
 
@@ -58,7 +58,7 @@ base::Optional<HitTestRegionList> HitTestDataProviderDrawQuad::GetHitTestData(
         hit_test_region_list->regions.emplace_back();
         HitTestRegion& hit_test_region = hit_test_region_list->regions.back();
         hit_test_region.frame_sink_id =
-            surface_quad->primary_surface_id.frame_sink_id();
+            surface_quad->surface_range.end().frame_sink_id();
         hit_test_region.flags = HitTestRegionFlags::kHitTestMouse |
                                 HitTestRegionFlags::kHitTestTouch |
                                 HitTestRegionFlags::kHitTestChildSurface;

components/viz/client/hit_test_data_provider_draw_quad_unittest.cc

@@ -47,9 +47,10 @@ std::unique_ptr<RenderPass> CreateRenderPassWithChildSurface(
                        1, SkBlendMode::kSrcOver, 0);
 
   auto* surface_quad = pass->CreateAndAppendDrawQuad<SurfaceDrawQuad>();
-  surface_quad->SetNew(pass->shared_quad_state_list.back(), child_rect,
-                       child_rect, child_surface_id, fallback_child_surface_id,
-                       SK_ColorWHITE, false);
+  surface_quad->SetNew(
+      pass->shared_quad_state_list.back(), child_rect, child_rect,
+      SurfaceRange(fallback_child_surface_id, child_surface_id), SK_ColorWHITE,
+      false);
 
   return pass;
 }

components/viz/common/quads/draw_quad_unittest.cc

@@ -283,19 +283,21 @@ TEST(DrawQuadTest, CopySurfaceDrawQuad) {
       LocalSurfaceId(5678, base::UnguessableToken::Create()));
   CREATE_SHARED_STATE();
 
-  CREATE_QUAD_NEW(SurfaceDrawQuad, visible_rect, primary_surface_id,
-                  fallback_surface_id, SK_ColorWHITE, true);
+  CREATE_QUAD_NEW(SurfaceDrawQuad, visible_rect,
+                  SurfaceRange(fallback_surface_id, primary_surface_id),
+                  SK_ColorWHITE, true);
   EXPECT_EQ(DrawQuad::SURFACE_CONTENT, copy_quad->material);
   EXPECT_EQ(visible_rect, copy_quad->visible_rect);
-  EXPECT_EQ(primary_surface_id, copy_quad->primary_surface_id);
-  EXPECT_EQ(fallback_surface_id, copy_quad->fallback_surface_id);
+  EXPECT_EQ(primary_surface_id, copy_quad->surface_range.end());
+  EXPECT_EQ(fallback_surface_id, *copy_quad->surface_range.start());
   EXPECT_TRUE(copy_quad->stretch_content_to_fill_bounds);
 
-  CREATE_QUAD_ALL(SurfaceDrawQuad, primary_surface_id, fallback_surface_id,
+  CREATE_QUAD_ALL(SurfaceDrawQuad,
+                  SurfaceRange(fallback_surface_id, primary_surface_id),
                   SK_ColorWHITE, false);
   EXPECT_EQ(DrawQuad::SURFACE_CONTENT, copy_quad->material);
-  EXPECT_EQ(primary_surface_id, copy_quad->primary_surface_id);
-  EXPECT_EQ(fallback_surface_id, copy_quad->fallback_surface_id);
+  EXPECT_EQ(primary_surface_id, copy_quad->surface_range.end());
+  EXPECT_EQ(fallback_surface_id, *copy_quad->surface_range.start());
   EXPECT_FALSE(copy_quad->stretch_content_to_fill_bounds);
 }
 
@@ -573,8 +575,9 @@ TEST_F(DrawQuadIteratorTest, SurfaceDrawQuad) {
                        LocalSurfaceId(4321, base::UnguessableToken::Create()));
 
   CREATE_SHARED_STATE();
-  CREATE_QUAD_NEW(SurfaceDrawQuad, visible_rect, surface_id, base::nullopt,
-                  SK_ColorWHITE, false);
+  CREATE_QUAD_NEW(SurfaceDrawQuad, visible_rect,
+                  SurfaceRange(base::nullopt, surface_id), SK_ColorWHITE,
+                  false);
   EXPECT_EQ(0, IterateAndCount(quad_new));
 }
 

components/viz/common/quads/surface_draw_quad.cc

@@ -20,36 +20,30 @@ SurfaceDrawQuad::~SurfaceDrawQuad() = default;
 SurfaceDrawQuad& SurfaceDrawQuad::operator=(const SurfaceDrawQuad& other) =
     default;
 
-void SurfaceDrawQuad::SetNew(
-    const SharedQuadState* shared_quad_state,
-    const gfx::Rect& rect,
-    const gfx::Rect& visible_rect,
-    const SurfaceId& primary_surface_id,
-    const base::Optional<SurfaceId>& fallback_surface_id,
-    SkColor default_background_color,
-    bool stretch_content_to_fill_bounds) {
+void SurfaceDrawQuad::SetNew(const SharedQuadState* shared_quad_state,
+                             const gfx::Rect& rect,
+                             const gfx::Rect& visible_rect,
+                             const SurfaceRange& surface_range,
+                             SkColor default_background_color,
+                             bool stretch_content_to_fill_bounds) {
   bool needs_blending = true;
   DrawQuad::SetAll(shared_quad_state, DrawQuad::SURFACE_CONTENT, rect,
                    visible_rect, needs_blending);
-  this->primary_surface_id = primary_surface_id;
-  this->fallback_surface_id = fallback_surface_id;
+  this->surface_range = surface_range;
   this->default_background_color = default_background_color;
   this->stretch_content_to_fill_bounds = stretch_content_to_fill_bounds;
 }
 
-void SurfaceDrawQuad::SetAll(
-    const SharedQuadState* shared_quad_state,
-    const gfx::Rect& rect,
-    const gfx::Rect& visible_rect,
-    bool needs_blending,
-    const SurfaceId& primary_surface_id,
-    const base::Optional<SurfaceId>& fallback_surface_id,
-    SkColor default_background_color,
-    bool stretch_content_to_fill_bounds) {
+void SurfaceDrawQuad::SetAll(const SharedQuadState* shared_quad_state,
+                             const gfx::Rect& rect,
+                             const gfx::Rect& visible_rect,
+                             bool needs_blending,
+                             const SurfaceRange& surface_range,
+                             SkColor default_background_color,
+                             bool stretch_content_to_fill_bounds) {
   DrawQuad::SetAll(shared_quad_state, DrawQuad::SURFACE_CONTENT, rect,
                    visible_rect, needs_blending);
-  this->primary_surface_id = primary_surface_id;
-  this->fallback_surface_id = fallback_surface_id;
+  this->surface_range = surface_range;
   this->default_background_color = default_background_color;
   this->stretch_content_to_fill_bounds = stretch_content_to_fill_bounds;
 }
@@ -60,9 +54,7 @@ const SurfaceDrawQuad* SurfaceDrawQuad::MaterialCast(const DrawQuad* quad) {
 }
 
 void SurfaceDrawQuad::ExtendValue(base::trace_event::TracedValue* value) const {
-  value->SetString("primary_surface_id", primary_surface_id.ToString());
-  if (fallback_surface_id.has_value())
-    value->SetString("fallback_surface_id", fallback_surface_id->ToString());
+  value->SetString("surface_range", surface_range.ToString());
 }
 
 }  // namespace viz

components/viz/common/quads/surface_draw_quad.h

@@ -9,7 +9,7 @@
 
 #include "base/optional.h"
 #include "components/viz/common/quads/draw_quad.h"
-#include "components/viz/common/surfaces/surface_id.h"
+#include "components/viz/common/surfaces/surface_range.h"
 #include "components/viz/common/viz_common_export.h"
 #include "third_party/skia/include/core/SkColor.h"
 
@@ -26,22 +26,19 @@ class VIZ_COMMON_EXPORT SurfaceDrawQuad : public DrawQuad {
   void SetNew(const SharedQuadState* shared_quad_state,
               const gfx::Rect& rect,
               const gfx::Rect& visible_rect,
-              const SurfaceId& primary_surface_id,
-              const base::Optional<SurfaceId>& fallback_surface_id,
+              const SurfaceRange& surface_range,
               SkColor default_background_color,
               bool stretch_content_to_fill_bounds);
 
   void SetAll(const SharedQuadState* shared_quad_state,
               const gfx::Rect& rect,
               const gfx::Rect& visible_rect,
               bool needs_blending,
-              const SurfaceId& primary_surface_id,
-              const base::Optional<SurfaceId>& fallback_surface_id,
+              const SurfaceRange& surface_range,
               SkColor default_background_color,
               bool stretch_content_to_fill_bounds);
 
-  SurfaceId primary_surface_id;
-  base::Optional<SurfaceId> fallback_surface_id;
+  SurfaceRange surface_range;
   SkColor default_background_color = SK_ColorWHITE;
   bool stretch_content_to_fill_bounds = false;
 

components/viz/service/display/display_unittest.cc

@@ -3277,9 +3277,8 @@ TEST_F(DisplayTest, CompositorFrameWithPresentationToken) {
     auto* quad2 = pass->quad_list.AllocateAndConstruct<SurfaceDrawQuad>();
     quad2->SetNew(shared_quad_state2, rect2 /* rect */,
                   rect2 /* visible_rect */,
-                  sub_surface_id /* primary_surface_id */,
-                  base::Optional<SurfaceId>() /* fallback_surface_id */,
-                  SK_ColorBLACK, false /* stretch_content_to_fill_bounds */);
+                  SurfaceRange(base::nullopt, sub_surface_id), SK_ColorBLACK,
+                  false /* stretch_content_to_fill_bounds */);
 
     pass_list.push_back(std::move(pass));
     SubmitCompositorFrame(&pass_list, local_surface_id);