viz: Ensure correct alpha info before buffer allocation.

Buffer allocation for the display compositor is currently done before
overlay processing. This order is incorrect since we need to know
whether buffers used by the compositor need to support alpha and this
is not known until overlay processing. If a quad is promoted to an
underlay then the display compositor buffers will need to support alpha.

R=dcastagna@chromium.org,ccameron@chromium.org

Bug: 946360
Change-Id: Ibb854cdb7764d4f6f6f41e490012416f9f0497ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2026968
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Reviewed-by: Andres Calderon Jaramillo <andrescj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738755}

components/viz/service/display/direct_renderer.cc

@@ -295,33 +295,6 @@ void DirectRenderer::DrawFrame(RenderPassList* render_passes_in_draw_order,
   current_frame()->device_viewport_size = device_viewport_size;
   current_frame()->sdr_white_level = sdr_white_level;
 
-  // Only reshape when we know we are going to draw. Otherwise, the reshape
-  // can leave the window at the wrong size if we never draw and the proper
-  // viewport size is never set.
-  bool frame_has_alpha =
-      current_frame()->root_render_pass->has_transparent_background;
-  bool use_stencil = overdraw_feedback_;
-  bool needs_full_frame_redraw = false;
-  if (device_viewport_size != reshape_surface_size_ ||
-      device_scale_factor != reshape_device_scale_factor_ ||
-      root_render_pass->color_space != reshape_device_color_space_ ||
-      frame_has_alpha != reshape_has_alpha_ ||
-      use_stencil != reshape_use_stencil_) {
-    reshape_surface_size_ = device_viewport_size;
-    reshape_device_scale_factor_ = device_scale_factor;
-    reshape_device_color_space_ = root_render_pass->color_space;
-    reshape_has_alpha_ =
-        current_frame()->root_render_pass->has_transparent_background;
-    reshape_use_stencil_ = overdraw_feedback_;
-    output_surface_->Reshape(
-        reshape_surface_size_, reshape_device_scale_factor_,
-        reshape_device_color_space_, reshape_has_alpha_, reshape_use_stencil_);
-    if (overlay_processor_)
-      overlay_processor_->SetViewportSize(reshape_surface_size_);
-    // The entire surface has to be redrawn if reshape is requested.
-    needs_full_frame_redraw = true;
-  }
-
   BeginDrawingFrame();
 
   // RenderPass owns filters, backdrop_filters, etc., and will outlive this
@@ -341,11 +314,14 @@ void DirectRenderer::DrawFrame(RenderPassList* render_passes_in_draw_order,
     }
   }
 
+  bool frame_has_alpha =
+      current_frame()->root_render_pass->has_transparent_background;
   if (overlay_processor_) {
     // Display transform is needed for overlay validator on Android
     // SurfaceControl. This needs to called before ProcessForOverlays.
     overlay_processor_->SetDisplayTransformHint(
         output_surface_->GetDisplayTransform());
+    overlay_processor_->SetViewportSize(device_viewport_size);
 
     // Before ProcessForOverlay calls into the hardware to ask about whether the
     // overlay setup can be handled, we need to set up the primary plane.
@@ -360,7 +336,7 @@ void DirectRenderer::DrawFrame(RenderPassList* render_passes_in_draw_order,
       current_frame()->output_surface_plane =
           overlay_processor_->ProcessOutputSurfaceAsOverlay(
               device_viewport_size, output_surface_->GetOverlayBufferFormat(),
-              reshape_device_color_space_, reshape_has_alpha_,
+              root_render_pass->color_space, frame_has_alpha,
               output_surface_->GetOverlayMailbox());
       primary_plane = &(current_frame()->output_surface_plane.value());
     }
@@ -374,9 +350,37 @@ void DirectRenderer::DrawFrame(RenderPassList* render_passes_in_draw_order,
         &current_frame()->overlay_list, &current_frame()->root_damage_rect,
         &current_frame()->root_content_bounds);
 
+    // If we promote any quad to an underlay then the main plane must support
+    // alpha.
+    if (current_frame()->output_surface_plane)
+      frame_has_alpha |= current_frame()->output_surface_plane->enable_blending;
+
     overlay_processor_->AdjustOutputSurfaceOverlay(
         &(current_frame()->output_surface_plane));
   }
+
+  // Only reshape when we know we are going to draw. Otherwise, the reshape
+  // can leave the window at the wrong size if we never draw and the proper
+  // viewport size is never set.
+  bool use_stencil = overdraw_feedback_;
+  bool needs_full_frame_redraw = false;
+  if (device_viewport_size != reshape_surface_size_ ||
+      device_scale_factor != reshape_device_scale_factor_ ||
+      root_render_pass->color_space != reshape_device_color_space_ ||
+      frame_has_alpha != reshape_has_alpha_ ||
+      use_stencil != reshape_use_stencil_) {
+    reshape_surface_size_ = device_viewport_size;
+    reshape_device_scale_factor_ = device_scale_factor;
+    reshape_device_color_space_ = root_render_pass->color_space;
+    reshape_has_alpha_ = frame_has_alpha;
+    reshape_use_stencil_ = overdraw_feedback_;
+    output_surface_->Reshape(
+        reshape_surface_size_, reshape_device_scale_factor_,
+        reshape_device_color_space_, reshape_has_alpha_, reshape_use_stencil_);
+    // The entire surface has to be redrawn if reshape is requested.
+    needs_full_frame_redraw = true;
+  }
+
 #if defined(OS_WIN)
   bool was_using_dc_layers = using_dc_layers_;
   if (!current_frame()->overlay_list.empty()) {

components/viz/service/display/display_unittest.cc

@@ -273,7 +273,8 @@ TEST_F(DisplayTest, DisplayDamaged) {
             software_output_device_->viewport_pixel_size());
   EXPECT_EQ(gfx::Rect(0, 0, 100, 100), software_output_device_->damage_rect());
 
-  // Only damaged portion should be swapped.
+  // Only a small area is damaged but the color space changes which should
+  // result in full damage.
   {
     pass = RenderPass::Create();
     pass->output_rect = gfx::Rect(0, 0, 100, 100);
@@ -294,6 +295,36 @@ TEST_F(DisplayTest, DisplayDamaged) {
     EXPECT_EQ(color_space_2, output_surface_->last_reshape_color_space());
     EXPECT_TRUE(scheduler_->swapped());
     EXPECT_EQ(2u, output_surface_->num_sent_frames());
+    EXPECT_EQ(gfx::Size(100, 100),
+              software_output_device_->viewport_pixel_size());
+    EXPECT_EQ(gfx::Rect(0, 0, 100, 100),
+              software_output_device_->damage_rect());
+
+    EXPECT_EQ(0u, output_surface_->last_sent_frame()->latency_info.size());
+  }
+
+  // Same frame as above but no color space change. Only partial area should be
+  // drawn.
+  {
+    pass = RenderPass::Create();
+    pass->output_rect = gfx::Rect(0, 0, 100, 100);
+    pass->damage_rect = gfx::Rect(10, 10, 1, 1);
+    pass->id = 1u;
+
+    pass_list.push_back(std::move(pass));
+    ResetDamageForTest();
+    SubmitCompositorFrame(
+        &pass_list,
+        id_allocator_.GetCurrentLocalSurfaceIdAllocation().local_surface_id());
+    EXPECT_TRUE(scheduler_->damaged());
+
+    scheduler_->reset_swapped_for_test();
+    EXPECT_EQ(color_space_2, output_surface_->last_reshape_color_space());
+    display_->SetDisplayColorSpaces(color_spaces_2);
+    display_->DrawAndSwap(base::TimeTicks::Now());
+    EXPECT_EQ(color_space_2, output_surface_->last_reshape_color_space());
+    EXPECT_TRUE(scheduler_->swapped());
+    EXPECT_EQ(3u, output_surface_->num_sent_frames());
     EXPECT_EQ(gfx::Size(100, 100),
               software_output_device_->viewport_pixel_size());
     EXPECT_EQ(gfx::Rect(10, 10, 1, 1), software_output_device_->damage_rect());
@@ -318,7 +349,7 @@ TEST_F(DisplayTest, DisplayDamaged) {
     scheduler_->reset_swapped_for_test();
     display_->DrawAndSwap(base::TimeTicks::Now());
     EXPECT_TRUE(scheduler_->swapped());
-    EXPECT_EQ(2u, output_surface_->num_sent_frames());
+    EXPECT_EQ(3u, output_surface_->num_sent_frames());
   }
 
   // Pass is wrong size so shouldn't be swapped. However, damage should
@@ -346,7 +377,7 @@ TEST_F(DisplayTest, DisplayDamaged) {
     scheduler_->reset_swapped_for_test();
     display_->DrawAndSwap(base::TimeTicks::Now());
     EXPECT_TRUE(scheduler_->swapped());
-    EXPECT_EQ(2u, output_surface_->num_sent_frames());
+    EXPECT_EQ(3u, output_surface_->num_sent_frames());
   }
 
   // Previous frame wasn't swapped, so next swap should have full damage.
@@ -371,7 +402,7 @@ TEST_F(DisplayTest, DisplayDamaged) {
     scheduler_->reset_swapped_for_test();
     display_->DrawAndSwap(base::TimeTicks::Now());
     EXPECT_TRUE(scheduler_->swapped());
-    EXPECT_EQ(3u, output_surface_->num_sent_frames());
+    EXPECT_EQ(4u, output_surface_->num_sent_frames());
     EXPECT_EQ(gfx::Rect(0, 0, 100, 100),
               software_output_device_->damage_rect());
 
@@ -399,7 +430,7 @@ TEST_F(DisplayTest, DisplayDamaged) {
     scheduler_->reset_swapped_for_test();
     display_->DrawAndSwap(base::TimeTicks::Now());
     EXPECT_TRUE(scheduler_->swapped());
-    EXPECT_EQ(4u, output_surface_->num_sent_frames());
+    EXPECT_EQ(5u, output_surface_->num_sent_frames());
     EXPECT_TRUE(copy_called);
   }
 
@@ -424,7 +455,7 @@ TEST_F(DisplayTest, DisplayDamaged) {
     scheduler_->reset_swapped_for_test();
     display_->DrawAndSwap(base::TimeTicks::Now());
     EXPECT_TRUE(scheduler_->swapped());
-    EXPECT_EQ(4u, output_surface_->num_sent_frames());
+    EXPECT_EQ(5u, output_surface_->num_sent_frames());
   }
 
   // DisableSwapUntilResize() should cause a swap if no frame was swapped at the
@@ -437,7 +468,7 @@ TEST_F(DisplayTest, DisplayDamaged) {
     scheduler_->reset_swapped_for_test();
     display_->Resize(gfx::Size(200, 200));
     EXPECT_FALSE(scheduler_->swapped());
-    EXPECT_EQ(4u, output_surface_->num_sent_frames());
+    EXPECT_EQ(5u, output_surface_->num_sent_frames());
     ResetDamageForTest();
 
     constexpr gfx::Rect kOutputRect(0, 0, 200, 200);
@@ -455,7 +486,7 @@ TEST_F(DisplayTest, DisplayDamaged) {
     display_->DisableSwapUntilResize(base::OnceClosure());
     display_->Resize(gfx::Size(100, 100));
     EXPECT_TRUE(scheduler_->swapped());
-    EXPECT_EQ(5u, output_surface_->num_sent_frames());
+    EXPECT_EQ(6u, output_surface_->num_sent_frames());
     EXPECT_EQ(0u, output_surface_->last_sent_frame()->latency_info.size());
   }
 
@@ -480,7 +511,7 @@ TEST_F(DisplayTest, DisplayDamaged) {
     scheduler_->reset_swapped_for_test();
     display_->DrawAndSwap(base::TimeTicks::Now());
     EXPECT_TRUE(scheduler_->swapped());
-    EXPECT_EQ(6u, output_surface_->num_sent_frames());
+    EXPECT_EQ(7u, output_surface_->num_sent_frames());
     EXPECT_EQ(gfx::Size(100, 100),
               software_output_device_->viewport_pixel_size());
     EXPECT_EQ(gfx::Rect(0, 0, 100, 100),

components/viz/service/display/software_renderer.cc

@@ -88,7 +88,6 @@ bool SoftwareRenderer::CanPartialSwap() {
 
 void SoftwareRenderer::BeginDrawingFrame() {
   TRACE_EVENT0("viz", "SoftwareRenderer::BeginDrawingFrame");
-  root_canvas_ = output_device_->BeginPaint(current_frame()->root_damage_rect);
 }
 
 void SoftwareRenderer::FinishDrawingFrame() {
@@ -125,6 +124,7 @@ void SoftwareRenderer::EnsureScissorTestDisabled() {
 void SoftwareRenderer::BindFramebufferToOutputSurface() {
   DCHECK(!output_surface_->HasExternalStencilTest());
   current_framebuffer_canvas_.reset();
+  root_canvas_ = output_device_->BeginPaint(current_frame()->root_damage_rect);
   current_canvas_ = root_canvas_;
 }
 

components/viz/service/display_embedder/software_output_device_mac.cc

@@ -170,6 +170,9 @@ SkCanvas* SoftwareOutputDeviceMac::BeginPaint(
 
 void SoftwareOutputDeviceMac::EndPaint() {
   SoftwareOutputDevice::EndPaint();
+  if (!current_paint_buffer_)
+    return;
+
   {
     TRACE_EVENT0("browser", "IOSurfaceUnlock");
     IOReturn io_result = IOSurfaceUnlock(current_paint_buffer_->io_surface,

gpu/command_buffer/service/ahardwarebuffer_utils.cc

@@ -14,6 +14,7 @@ bool AHardwareBufferSupportedFormat(viz::ResourceFormat format) {
   switch (format) {
     case viz::RGBA_8888:
     case viz::RGB_565:
+    case viz::BGR_565:
     case viz::RGBA_F16:
     case viz::RGBX_8888:
     case viz::RGBA_1010102:
@@ -30,6 +31,8 @@ unsigned int AHardwareBufferFormat(viz::ResourceFormat format) {
       return AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM;
     case viz::RGB_565:
       return AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM;
+    case viz::BGR_565:
+      return AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM;
     case viz::RGBA_F16:
       return AHARDWAREBUFFER_FORMAT_R16G16B16A16_FLOAT;
     case viz::RGBX_8888: