SkiaOutputDevice: use buffer format passed to Reshape

Bug: 1108406
Change-Id: Iaca79e3b6c1d66e421711c46f75a1cb614b7f961
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2378745Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Peng Huang <penghuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802450}

components/viz/service/display/output_surface.h

@@ -117,13 +117,9 @@ class VIZ_SERVICE_EXPORT OutputSurface {
     // enforced if zero.
     int max_render_target_size = 0;
 
-    // The SkColorType and GrBackendFormat for non-HDR and HDR.
-    // TODO(penghuang): remove SkColorType and GrBackendFormat when
-    // OutputSurface uses the |format| passed to Reshape().
-    SkColorType sk_color_type = kUnknown_SkColorType;
-    GrBackendFormat gr_backend_format;
-    SkColorType sk_color_type_for_hdr = kUnknown_SkColorType;
-    GrBackendFormat gr_backend_format_for_hdr;
+    // SkColorType for all supported buffer formats.
+    SkColorType sk_color_types[static_cast<int>(gfx::BufferFormat::LAST) + 1] =
+        {};
   };
 
   // Constructor for skia-based compositing.

components/viz/service/display_embedder/output_presenter_fuchsia.cc

@@ -188,10 +188,10 @@ void OutputPresenterFuchsia::InitializeCapabilities(
   capabilities->supports_post_sub_buffer = false;
   capabilities->supports_commit_overlay_planes = false;
 
-  capabilities->sk_color_type = kRGBA_8888_SkColorType;
-  capabilities->gr_backend_format =
-      dependency_->GetSharedContextState()->gr_context()->defaultBackendFormat(
-          capabilities->sk_color_type, GrRenderable::kYes);
+  capabilities->sk_color_types[static_cast<int>(gfx::BufferFormat::RGBA_8888)] =
+      kRGBA_8888_SkColorType;
+  capabilities->sk_color_types[static_cast<int>(gfx::BufferFormat::BGRA_8888)] =
+      kRGBA_8888_SkColorType;
 }
 
 bool OutputPresenterFuchsia::Reshape(const gfx::Size& size,

components/viz/service/display_embedder/output_presenter_gl.cc

@@ -214,23 +214,6 @@ OutputPresenterGL::OutputPresenterGL(scoped_refptr<gl::GLSurface> gl_surface,
   // GL is origin is at bottom left normally, all Surfaceless implementations
   // are flipped.
   DCHECK_EQ(gl_surface_->GetOrigin(), gfx::SurfaceOrigin::kTopLeft);
-
-  // TODO(https://crbug.com/958166): The initial |image_format_| should not be
-  // used, and the gfx::BufferFormat specified in Reshape should be used
-  // instead, because it may be updated to reflect changes in the content being
-  // displayed (e.g, HDR content appearing on-screen).
-#if defined(OS_APPLE)
-  image_format_ = BGRA_8888;
-#else
-#if defined(USE_OZONE)
-  if (features::IsUsingOzonePlatform()) {
-    image_format_ =
-        GetResourceFormat(display::DisplaySnapshot::PrimaryFormat());
-    return;
-  }
-#endif
-  image_format_ = RGBA_8888;
-#endif
 }
 
 OutputPresenterGL::~OutputPresenterGL() = default;
@@ -247,13 +230,28 @@ void OutputPresenterGL::InitializeCapabilities(
   // We expect origin of buffers is at top left.
   capabilities->output_surface_origin = gfx::SurfaceOrigin::kTopLeft;
 
-  // TODO(penghuang): Use defaultBackendFormat() in shared image implementation
-  // to make sure backend format is consistent.
-  capabilities->sk_color_type = ResourceFormatToClosestSkColorType(
-      true /* gpu_compositing */, image_format_);
-  capabilities->gr_backend_format =
-      dependency_->GetSharedContextState()->gr_context()->defaultBackendFormat(
-          capabilities->sk_color_type, GrRenderable::kYes);
+  // TODO(https://crbug.com/1108406): only add supported formats base on
+  // platform, driver, etc.
+  capabilities->sk_color_types[static_cast<int>(gfx::BufferFormat::BGR_565)] =
+      kRGB_565_SkColorType;
+  capabilities->sk_color_types[static_cast<int>(gfx::BufferFormat::RGBA_4444)] =
+      kARGB_4444_SkColorType;
+  capabilities->sk_color_types[static_cast<int>(gfx::BufferFormat::RGBX_8888)] =
+      kRGB_888x_SkColorType;
+  capabilities->sk_color_types[static_cast<int>(gfx::BufferFormat::RGBA_8888)] =
+      kRGBA_8888_SkColorType;
+  capabilities->sk_color_types[static_cast<int>(gfx::BufferFormat::BGRX_8888)] =
+      kBGRA_8888_SkColorType;
+  capabilities->sk_color_types[static_cast<int>(gfx::BufferFormat::BGRA_8888)] =
+      kBGRA_8888_SkColorType;
+  capabilities
+      ->sk_color_types[static_cast<int>(gfx::BufferFormat::BGRA_1010102)] =
+      kBGRA_1010102_SkColorType;
+  capabilities
+      ->sk_color_types[static_cast<int>(gfx::BufferFormat::RGBA_1010102)] =
+      kRGBA_1010102_SkColorType;
+  capabilities->sk_color_types[static_cast<int>(gfx::BufferFormat::RGBA_F16)] =
+      kRGBA_F16_SkColorType;
 }
 
 bool OutputPresenterGL::Reshape(const gfx::Size& size,
@@ -261,6 +259,7 @@ bool OutputPresenterGL::Reshape(const gfx::Size& size,
                                 const gfx::ColorSpace& color_space,
                                 gfx::BufferFormat format,
                                 gfx::OverlayTransform transform) {
+  image_format_ = GetResourceFormat(format);
   return gl_surface_->Resize(size, device_scale_factor, color_space,
                              gfx::AlphaBitsForBufferFormat(format));
 }

components/viz/service/display_embedder/output_presenter_gl.h

@@ -65,7 +65,7 @@ class VIZ_SERVICE_EXPORT OutputPresenterGL : public OutputPresenter {
   SkiaOutputSurfaceDependency* dependency_;
   const bool supports_async_swap_;
 
-  ResourceFormat image_format_;
+  ResourceFormat image_format_ = RGBA_8888;
 
   // Shared Image factories
   gpu::SharedImageFactory shared_image_factory_;

components/viz/service/display_embedder/skia_output_device.cc

@@ -120,6 +120,12 @@ void SkiaOutputDevice::ScheduleOverlays(
 void SkiaOutputDevice::EnsureBackbuffer() {}
 void SkiaOutputDevice::DiscardBackbuffer() {}
 
+void SkiaOutputDevice::SetDrawTimings(base::TimeTicks submitted,
+                                      base::TimeTicks started) {
+  viz_scheduled_draw_ = submitted;
+  gpu_started_draw_ = started;
+}
+
 void SkiaOutputDevice::StartSwapBuffers(BufferPresentedCallback feedback) {
   DCHECK_LT(static_cast<int>(pending_swaps_.size()),
             capabilities_.max_frames_pending);
@@ -158,12 +164,6 @@ void SkiaOutputDevice::FinishSwapBuffers(
   pending_swaps_.pop();
 }
 
-void SkiaOutputDevice::SetDrawTimings(base::TimeTicks submitted,
-                                      base::TimeTicks started) {
-  viz_scheduled_draw_ = submitted;
-  gpu_started_draw_ = started;
-}
-
 SkiaOutputDevice::SwapInfo::SwapInfo(
     uint64_t swap_id,
     SkiaOutputDevice::BufferPresentedCallback feedback,

components/viz/service/display_embedder/skia_output_device_dawn.cc

@@ -44,11 +44,10 @@ SkiaOutputDeviceDawn::SkiaOutputDeviceDawn(
   capabilities_.uses_default_gl_framebuffer = false;
   capabilities_.supports_post_sub_buffer = false;
 
-  capabilities_.sk_color_type = kSurfaceColorType;
-  capabilities_.gr_backend_format =
-      context_provider_->GetGrContext()->defaultBackendFormat(
-          kSurfaceColorType, GrRenderable::kYes);
-
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::RGBA_8888)] =
+      kSurfaceColorType;
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::BGRA_8888)] =
+      kSurfaceColorType;
   vsync_provider_ = std::make_unique<gl::VSyncProviderWin>(widget);
   child_window_.Initialize();
 }

components/viz/service/display_embedder/skia_output_device_gl.cc

@@ -105,15 +105,20 @@ SkiaOutputDeviceGL::SkiaOutputDeviceGL(
     glGetIntegerv(GL_ALPHA_BITS, &alpha_bits);
   }
   CHECK_GL_ERROR();
-  supports_alpha_ = alpha_bits > 0;
-
-  capabilities_.sk_color_type =
-      supports_alpha_ ? kRGBA_8888_SkColorType : kRGB_888x_SkColorType;
-  capabilities_.gr_backend_format = gr_context->defaultBackendFormat(
-      capabilities_.sk_color_type, GrRenderable::kYes);
-  capabilities_.sk_color_type_for_hdr = kRGBA_F16_SkColorType;
-  capabilities_.gr_backend_format_for_hdr = gr_context->defaultBackendFormat(
-      capabilities_.sk_color_type_for_hdr, GrRenderable::kYes);
+
+  auto color_type =
+      (alpha_bits > 0) ? kRGBA_8888_SkColorType : kRGB_888x_SkColorType;
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::RGBA_8888)] =
+      color_type;
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::RGBX_8888)] =
+      color_type;
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::BGRA_8888)] =
+      color_type;
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::BGRX_8888)] =
+      color_type;
+
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::RGBA_F16)] =
+      kRGBA_F16_SkColorType;
 }
 
 SkiaOutputDeviceGL::~SkiaOutputDeviceGL() {
@@ -140,22 +145,21 @@ bool SkiaOutputDeviceGL::Reshape(const gfx::Size& size,
   framebuffer_info.fFBOID = 0;
   DCHECK_EQ(gl_surface_->GetBackingFramebufferObject(), 0u);
 
-  SkColorType color_type;
-  // TODO(https://crbug.com/1049334): The pixel format should be determined by
-  // |buffer_format|, not |color_space|, and not |supports_alpha_|.
-  if (color_space.IsHDR()) {
-    color_type = capabilities_.sk_color_type_for_hdr;
-    framebuffer_info.fFormat = GL_RGBA16F;
-    DCHECK_EQ(capabilities_.gr_backend_format_for_hdr.asGLFormat(),
-              GrGLFormat::kRGBA16F);
-  } else if (supports_alpha_) {
-    color_type = capabilities_.sk_color_type;
-    framebuffer_info.fFormat = GL_RGBA8;
-    DCHECK_EQ(capabilities_.gr_backend_format.asGLFormat(), GrGLFormat::kRGBA8);
-  } else {
-    color_type = capabilities_.sk_color_type;
-    framebuffer_info.fFormat = GL_RGB8;
-    DCHECK_EQ(capabilities_.gr_backend_format.asGLFormat(), GrGLFormat::kRGB8);
+  const auto format_index = static_cast<int>(buffer_format);
+  SkColorType color_type = capabilities_.sk_color_types[format_index];
+  switch (color_type) {
+    case kRGBA_8888_SkColorType:
+      framebuffer_info.fFormat = GL_RGBA8;
+      break;
+    case kRGB_888x_SkColorType:
+      framebuffer_info.fFormat = GL_RGB8;
+      break;
+    case kRGBA_F16_SkColorType:
+      framebuffer_info.fFormat = GL_RGBA16F;
+      break;
+    default:
+      NOTREACHED() << "color_type: " << color_type
+                   << " buffer_format: " << format_index;
   }
   // TODO(kylechar): We might need to support RGB10A2 for HDR10. HDR10 was only
   // used with Windows updated RS3 (2017) as a workaround for a DWM bug so it

components/viz/service/display_embedder/skia_output_device_gl.h

@@ -81,7 +81,6 @@ class SkiaOutputDeviceGL final : public SkiaOutputDevice {
 
   sk_sp<SkSurface> sk_surface_;
 
-  bool supports_alpha_ = false;
   uint64_t backbuffer_estimated_size_ = 0;
 
   base::WeakPtrFactory<SkiaOutputDeviceGL> weak_ptr_factory_{this};

components/viz/service/display_embedder/skia_output_device_offscreen.cc

@@ -34,10 +34,16 @@ SkiaOutputDeviceOffscreen::SkiaOutputDeviceOffscreen(
   capabilities_.output_surface_origin = origin;
   capabilities_.supports_post_sub_buffer = true;
 
-  capabilities_.sk_color_type = kSurfaceColorType;
-  capabilities_.gr_backend_format =
-      context_state_->gr_context()->defaultBackendFormat(kSurfaceColorType,
-                                                         GrRenderable::kYes);
+  // TODO(https://crbug.com/1108406): use the right color types base on GPU
+  // capabilities.
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::RGBA_8888)] =
+      kSurfaceColorType;
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::RGBX_8888)] =
+      kSurfaceColorType;
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::BGRA_8888)] =
+      kSurfaceColorType;
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::BGRX_8888)] =
+      kSurfaceColorType;
 }
 
 SkiaOutputDeviceOffscreen::~SkiaOutputDeviceOffscreen() {
@@ -53,6 +59,7 @@ bool SkiaOutputDeviceOffscreen::Reshape(const gfx::Size& size,
 
   DiscardBackbuffer();
   size_ = size;
+  format_ = format;
   sk_color_space_ = color_space.ToSkColorSpace();
   EnsureBackbuffer();
   return true;
@@ -82,15 +89,20 @@ void SkiaOutputDeviceOffscreen::EnsureBackbuffer() {
   if (size_.IsEmpty())
     return;
 
+  auto format_index = static_cast<int>(format_);
+  const auto& sk_color_type = capabilities_.sk_color_types[format_index];
+  DCHECK(sk_color_type != kUnknown_SkColorType)
+      << "SkColorType is invalid for format: " << format_index;
+
   if (has_alpha_) {
     backend_texture_ = context_state_->gr_context()->createBackendTexture(
-        size_.width(), size_.height(), kSurfaceColorType, GrMipMapped::kNo,
+        size_.width(), size_.height(), sk_color_type, GrMipMapped::kNo,
         GrRenderable::kYes);
   } else {
     is_emulated_rgbx_ = true;
     // Initialize alpha channel to opaque.
     backend_texture_ = context_state_->gr_context()->createBackendTexture(
-        size_.width(), size_.height(), kSurfaceColorType, SkColors::kBlack,
+        size_.width(), size_.height(), sk_color_type, SkColors::kBlack,
         GrMipMapped::kNo, GrRenderable::kYes);
   }
   DCHECK(backend_texture_.isValid());

components/viz/service/display_embedder/skia_output_device_offscreen.h

@@ -51,6 +51,7 @@ class SkiaOutputDeviceOffscreen : public SkiaOutputDevice {
 
  private:
   gfx::Size size_;
+  gfx::BufferFormat format_ = gfx::BufferFormat::RGBA_8888;
   uint64_t backbuffer_estimated_size_ = 0;
   sk_sp<SkColorSpace> sk_color_space_;
 

components/viz/service/display_embedder/skia_output_device_vulkan.cc

@@ -318,10 +318,14 @@ bool SkiaOutputDeviceVulkan::Initialize() {
   const auto surface_format = vulkan_surface_->surface_format().format;
   DCHECK(surface_format == VK_FORMAT_B8G8R8A8_UNORM ||
          surface_format == VK_FORMAT_R8G8B8A8_UNORM);
-  capabilities_.sk_color_type = surface_format == VK_FORMAT_R8G8B8A8_UNORM
-                                    ? kRGBA_8888_SkColorType
-                                    : kBGRA_8888_SkColorType;
-  capabilities_.gr_backend_format = GrBackendFormat::MakeVk(surface_format);
+
+  auto sk_color_type = surface_format == VK_FORMAT_R8G8B8A8_UNORM
+                           ? kRGBA_8888_SkColorType
+                           : kBGRA_8888_SkColorType;
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::RGBA_8888)] =
+      sk_color_type;
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::BGRA_8888)] =
+      sk_color_type;
   return true;
 }
 

components/viz/service/display_embedder/skia_output_device_webview.cc

@@ -16,6 +16,10 @@
 
 namespace viz {
 
+namespace {
+constexpr auto kSurfaceColorType = kRGBA_8888_SkColorType;
+}
+
 SkiaOutputDeviceWebView::SkiaOutputDeviceWebView(
     gpu::SharedContextState* context_state,
     scoped_refptr<gl::GLSurface> gl_surface,
@@ -36,10 +40,10 @@ SkiaOutputDeviceWebView::SkiaOutputDeviceWebView(
   DCHECK(context_state_->gr_context());
   DCHECK(context_state_->context());
 
-  capabilities_.sk_color_type = kRGBA_8888_SkColorType;
-  capabilities_.gr_backend_format =
-      context_state_->gr_context()->defaultBackendFormat(
-          capabilities_.sk_color_type, GrRenderable::kYes);
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::RGBA_8888)] =
+      kSurfaceColorType;
+  capabilities_.sk_color_types[static_cast<int>(gfx::BufferFormat::BGRA_8888)] =
+      kSurfaceColorType;
 }
 
 SkiaOutputDeviceWebView::~SkiaOutputDeviceWebView() = default;
@@ -100,8 +104,7 @@ void SkiaOutputDeviceWebView::InitSkiaSurface(unsigned int fbo) {
   GrGLFramebufferInfo framebuffer_info;
   framebuffer_info.fFBOID = fbo;
   framebuffer_info.fFormat = GL_RGBA8;
-  DCHECK_EQ(capabilities_.gr_backend_format.asGLFormat(), GrGLFormat::kRGBA8);
-  SkColorType color_type = capabilities_.sk_color_type;
+  SkColorType color_type = kSurfaceColorType;
 
   GrBackendRenderTarget render_target(size_.width(), size_.height(),
                                       /*sampleCnt=*/0,

components/viz/service/display_embedder/skia_output_surface_impl.cc

@@ -236,9 +236,10 @@ void SkiaOutputSurfaceImpl::Reshape(const gfx::Size& size,
   color_space_ = color_space;
   is_hdr_ = color_space_.IsHDR();
   size_ = size;
+  format_ = format;
   characterization_ = CreateSkSurfaceCharacterization(
-      size, GetResourceFormat(format), false /* mipmap */,
-      color_space_.ToSkColorSpace(), true /* is_root_render_pass */);
+      size, format, false /* mipmap */, color_space_.ToSkColorSpace(),
+      true /* is_root_render_pass */);
   RecreateRootRecorder();
 }
 
@@ -289,8 +290,8 @@ SkCanvas* SkiaOutputSurfaceImpl::BeginPaintCurrentFrame() {
   nway_canvas_->addCanvas(current_paint_->recorder()->getCanvas());
 
   SkSurfaceCharacterization characterization = CreateSkSurfaceCharacterization(
-      gfx::Size(characterization_.width(), characterization_.height()),
-      BGRA_8888, false /* mipmap */, characterization_.refColorSpace(),
+      gfx::Size(characterization_.width(), characterization_.height()), format_,
+      false /* mipmap */, characterization_.refColorSpace(),
       false /* is_root_render_pass */);
   if (characterization.isValid()) {
     overdraw_surface_recorder_.emplace(characterization);
@@ -493,7 +494,7 @@ SkCanvas* SkiaOutputSurfaceImpl::BeginPaintRenderPass(
   DCHECK(resource_sync_tokens_.empty());
 
   SkSurfaceCharacterization characterization = CreateSkSurfaceCharacterization(
-      surface_size, format, mipmap, std::move(color_space),
+      surface_size, BufferFormat(format), mipmap, std::move(color_space),
       false /* is_root_render_pass */);
   if (!characterization.isValid())
     return nullptr;
@@ -779,7 +780,7 @@ void SkiaOutputSurfaceImpl::InitializeOnGpuThread(
 SkSurfaceCharacterization
 SkiaOutputSurfaceImpl::CreateSkSurfaceCharacterization(
     const gfx::Size& surface_size,
-    ResourceFormat format,
+    gfx::BufferFormat format,
     bool mipmap,
     sk_sp<SkColorSpace> color_space,
     bool is_root_render_pass) {
@@ -791,15 +792,15 @@ SkiaOutputSurfaceImpl::CreateSkSurfaceCharacterization(
   SkSurfaceProps surface_props(0 /*flags */,
                                SkSurfaceProps::kLegacyFontHost_InitType);
   if (is_root_render_pass) {
-    auto color_type =
-        is_hdr_ && capabilities_.sk_color_type_for_hdr != kUnknown_SkColorType
-            ? capabilities_.sk_color_type_for_hdr
-            : capabilities_.sk_color_type;
-
-    const auto& backend_format =
-        is_hdr_ && capabilities_.gr_backend_format_for_hdr.isValid()
-            ? capabilities_.gr_backend_format_for_hdr
-            : capabilities_.gr_backend_format;
+    const auto format_index = static_cast<int>(format);
+    const auto& color_type = capabilities_.sk_color_types[format_index];
+    const auto backend_format = gr_context_thread_safe_->defaultBackendFormat(
+        color_type, GrRenderable::kYes);
+    DCHECK(color_type != kUnknown_SkColorType)
+        << "SkColorType is invalid for buffer format_index: " << format_index;
+    DCHECK(backend_format.isValid())
+        << "GrBackendFormat is invalid for buffer format_index: "
+        << format_index;
     auto surface_origin =
         capabilities_.output_surface_origin == gfx::SurfaceOrigin::kBottomLeft
             ? kBottomLeft_GrSurfaceOrigin
@@ -838,8 +839,9 @@ SkiaOutputSurfaceImpl::CreateSkSurfaceCharacterization(
     return characterization;
   }
 
-  auto color_type =
-      ResourceFormatToClosestSkColorType(true /* gpu_compositing */, format);
+  auto resource_format = GetResourceFormat(format);
+  auto color_type = ResourceFormatToClosestSkColorType(
+      true /* gpu_compositing */, resource_format);
   auto backend_format = gr_context_thread_safe_->defaultBackendFormat(
       color_type, GrRenderable::kYes);
   DCHECK(backend_format.isValid());

components/viz/service/display_embedder/skia_output_surface_impl.h

@@ -159,7 +159,7 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurfaceImpl : public SkiaOutputSurface {
                              bool* result);
   SkSurfaceCharacterization CreateSkSurfaceCharacterization(
       const gfx::Size& surface_size,
-      ResourceFormat format,
+      gfx::BufferFormat format,
       bool mipmap,
       sk_sp<SkColorSpace> color_space,
       bool is_root_render_pass);
@@ -202,6 +202,7 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurfaceImpl : public SkiaOutputSurface {
 
   gfx::Size size_;
   gfx::ColorSpace color_space_;
+  gfx::BufferFormat format_;
   bool is_hdr_ = false;
   SkSurfaceCharacterization characterization_;
   base::Optional<SkDeferredDisplayListRecorder> root_recorder_;