Remove SecondaryCB-only characterization path

Remove SkiaOutputSurfaceDependency change to pass SkSurfaceCharacterization for secondary cb only. Instead use the new skia API to create the characterization directly from passed in info. Needed to add a OutputSurface::Capabilities for secondary cb, which will probably be useful in the future. Also need to fix in webview to ensure the SkSurfaceProps matches viz. Bug: 1144921 Change-Id: I948964a0caa82dba5ef671acafd8c776c403d7e6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2521196 Commit-Queue: Bo <boliu@chromium.org> Reviewed-by: Peng Huang <penghuang@chromium.org> Cr-Commit-Position: refs/heads/master@{#824527}
Denger-Network · Nov 5, 2020 · 1c28c6e · 1c28c6e
1 parent 2fa913a
commit 1c28c6e
Show file tree

Hide file tree

Showing 11 changed files with 26 additions and 52 deletions.
diff --git a/android_webview/browser/gfx/aw_draw_fn_impl.cc b/android_webview/browser/gfx/aw_draw_fn_impl.cc
@@ -13,6 +13,7 @@
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "gpu/config/gpu_switches.h"
+#include "skia/ext/legacy_display_globals.h"
 #include "third_party/skia/include/core/SkRefCnt.h"
 #include "third_party/skia/include/gpu/GrDirectContext.h"
 #include "third_party/skia/src/gpu/vk/GrVkSecondaryCBDrawContext.h"
@@ -90,7 +91,7 @@ sk_sp<GrVkSecondaryCBDrawContext> CreateDrawContext(
       .fFormat = params->format,
       .fDrawBounds = &draw_bounds,
   };
-  SkSurfaceProps props(0, kUnknown_SkPixelGeometry);
+  SkSurfaceProps props = skia::LegacyDisplayGlobals::GetSkSurfaceProps();
   return GrVkSecondaryCBDrawContext::Make(gr_context, info, drawable_info,
                                           &props);
 }

diff --git a/android_webview/browser/gfx/aw_vulkan_context_provider.cc b/android_webview/browser/gfx/aw_vulkan_context_provider.cc
@@ -221,9 +221,6 @@ void AwVulkanContextProvider::SecondaryCBDrawBegin(
   DCHECK(!draw_context_);
   DCHECK(post_submit_tasks_.empty());
   draw_context_ = draw_context;
-  characterization_.emplace();
-  bool result = draw_context_->characterize(&characterization_.value());
-  CHECK(result);
 }
 
 void AwVulkanContextProvider::SecondaryCMBDrawSubmitted() {
@@ -251,8 +248,6 @@ void AwVulkanContextProvider::SecondaryCMBDrawSubmitted() {
 
   fence_helper->EnqueueFence(vk_fence);
   fence_helper->ProcessCleanupTasks();
-
-  characterization_.reset();
 }
 
 }  // namespace android_webview
diff --git a/android_webview/browser/gfx/aw_vulkan_context_provider.h b/android_webview/browser/gfx/aw_vulkan_context_provider.h
@@ -56,10 +56,6 @@ class AwVulkanContextProvider final : public viz::VulkanContextProvider {
       std::vector<VkSemaphore> semaphores) override;
   void EnqueueSecondaryCBPostSubmitTask(base::OnceClosure closure) override;
 
-  base::Optional<SkSurfaceCharacterization> characterization() const {
-    CHECK(characterization_);
-    return characterization_;
-  }
   VkDevice device() { return globals_->device_queue->GetVulkanDevice(); }
   VkQueue queue() { return globals_->device_queue->GetVulkanQueue(); }
 
@@ -95,9 +91,6 @@ class AwVulkanContextProvider final : public viz::VulkanContextProvider {
   std::vector<base::OnceClosure> post_submit_tasks_;
   std::vector<VkSemaphore> post_submit_semaphores_;
 
-  // Accessed from viz thread.
-  base::Optional<SkSurfaceCharacterization> characterization_;
-
   DISALLOW_COPY_AND_ASSIGN(AwVulkanContextProvider);
 };
 

diff --git a/android_webview/browser/gfx/skia_output_surface_dependency_webview.cc b/android_webview/browser/gfx/skia_output_surface_dependency_webview.cc
@@ -148,13 +148,6 @@ SkiaOutputSurfaceDependencyWebView::GetGpuBlockedTimeSinceLastSwap() {
   return base::TimeDelta();
 }
 
-base::Optional<SkSurfaceCharacterization>
-SkiaOutputSurfaceDependencyWebView::GetRootSurfaceCharacterization() {
-  if (!vulkan_context_provider_)
-    return base::nullopt;
-  return vulkan_context_provider_->characterization();
-}
-
 void SkiaOutputSurfaceDependencyWebView::ScheduleDelayedGPUTaskFromGPUThread(
     base::OnceClosure task) {
   task_queue_->ScheduleIdleTask(std::move(task));

diff --git a/android_webview/browser/gfx/skia_output_surface_dependency_webview.h b/android_webview/browser/gfx/skia_output_surface_dependency_webview.h
@@ -53,8 +53,6 @@ class SkiaOutputSurfaceDependencyWebView
   void DidLoseContext(gpu::error::ContextLostReason reason,
                       const GURL& active_url) override;
 
-  base::Optional<SkSurfaceCharacterization> GetRootSurfaceCharacterization()
-      override;
   base::TimeDelta GetGpuBlockedTimeSinceLastSwap() override;
   bool NeedsSupportForExternalStencil() override;
 

diff --git a/components/viz/service/display/output_surface.h b/components/viz/service/display/output_surface.h
@@ -116,6 +116,8 @@ class VIZ_SERVICE_EXPORT OutputSurface {
     // This is the maximum size for RenderPass textures. No maximum size is
     // enforced if zero.
     int max_render_target_size = 0;
+    // The root surface is rendered using vulkan secondary command buffer.
+    bool root_is_vulkan_secondary_command_buffer = false;
 
     // SkColorType for all supported buffer formats.
     SkColorType sk_color_types[static_cast<int>(gfx::BufferFormat::LAST) + 1] =

diff --git a/components/viz/service/display_embedder/skia_output_device_vulkan_secondary_cb.cc b/components/viz/service/display_embedder/skia_output_device_vulkan_secondary_cb.cc
@@ -30,12 +30,22 @@ SkiaOutputDeviceVulkanSecondaryCB::SkiaOutputDeviceVulkanSecondaryCB(
   capabilities_.output_surface_origin = gfx::SurfaceOrigin::kTopLeft;
   capabilities_.supports_post_sub_buffer = false;
   capabilities_.orientation_mode = OutputSurface::OrientationMode::kLogic;
-
-  // Do not set any |capabilities_.sk_color_types|. This requires the
-  // SkSurfaceCharacterization to be overridden with the one obtained from
-  // GrVkSecondaryCBDrawContext instead of being created by SkiaRenderer
-  // from the passed in information. So |sk_color_types| should not be used.
-  // TODO(crbug.com/1144921): Remove this override.
+  capabilities_.root_is_vulkan_secondary_command_buffer = true;
+
+  GrVkSecondaryCBDrawContext* secondary_cb_draw_context =
+      context_provider_->GetGrSecondaryCBDrawContext();
+  SkSurfaceCharacterization characterization;
+  VkFormat vkFormat = VK_FORMAT_UNDEFINED;
+  bool result = secondary_cb_draw_context->characterize(&characterization);
+  CHECK(result);
+  characterization.backendFormat().asVkFormat(&vkFormat);
+  auto sk_color_type = vkFormat == 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;
 }
 
 void SkiaOutputDeviceVulkanSecondaryCB::Submit(base::OnceClosure callback) {

diff --git a/components/viz/service/display_embedder/skia_output_surface_dependency.h b/components/viz/service/display_embedder/skia_output_surface_dependency.h
@@ -110,10 +110,6 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurfaceDependency {
   virtual void DidLoseContext(gpu::error::ContextLostReason reason,
                               const GURL& active_url) = 0;
 
-  // Called on client thread.
-  virtual base::Optional<SkSurfaceCharacterization>
-  GetRootSurfaceCharacterization() = 0;
-
   virtual base::TimeDelta GetGpuBlockedTimeSinceLastSwap() = 0;
   virtual bool NeedsSupportForExternalStencil() = 0;
 

diff --git a/components/viz/service/display_embedder/skia_output_surface_dependency_impl.cc b/components/viz/service/display_embedder/skia_output_surface_dependency_impl.cc
@@ -164,11 +164,6 @@ void SkiaOutputSurfaceDependencyImpl::DidLoseContext(
   gpu_service_impl_->DidLoseContext(/*offscreen=*/false, reason, active_url);
 }
 
-base::Optional<SkSurfaceCharacterization>
-SkiaOutputSurfaceDependencyImpl::GetRootSurfaceCharacterization() {
-  return base::nullopt;
-}
-
 base::TimeDelta
 SkiaOutputSurfaceDependencyImpl::GetGpuBlockedTimeSinceLastSwap() {
   return gpu_service_impl_->GetGpuScheduler()->TakeTotalBlockingTime();

diff --git a/components/viz/service/display_embedder/skia_output_surface_dependency_impl.h b/components/viz/service/display_embedder/skia_output_surface_dependency_impl.h
@@ -59,8 +59,6 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurfaceDependencyImpl
   void DidLoseContext(gpu::error::ContextLostReason reason,
                       const GURL& active_url) override;
 
-  base::Optional<SkSurfaceCharacterization> GetRootSurfaceCharacterization()
-      override;
   base::TimeDelta GetGpuBlockedTimeSinceLastSwap() override;
   bool NeedsSupportForExternalStencil() override;
 

diff --git a/components/viz/service/display_embedder/skia_output_surface_impl.cc b/components/viz/service/display_embedder/skia_output_surface_impl.cc
@@ -246,18 +246,9 @@ void SkiaOutputSurfaceImpl::Reshape(const gfx::Size& size,
   is_hdr_ = color_space_.IsHDR();
   size_ = size;
   format_ = format;
-  base::Optional<SkSurfaceCharacterization> characterization_opt =
-      dependency_->GetRootSurfaceCharacterization();
-  if (characterization_opt) {
-    characterization_ = characterization_opt.value();
-    DCHECK(characterization_.isValid());
-    DCHECK_EQ(size.width(), characterization_.width());
-    DCHECK_EQ(size.height(), characterization_.height());
-  } else {
-    characterization_ = CreateSkSurfaceCharacterization(
-        size, format, false /* mipmap */, color_space_.ToSkColorSpace(),
-        true /* is_root_render_pass */);
-  }
+  characterization_ = CreateSkSurfaceCharacterization(
+      size, format, false /* mipmap */, color_space_.ToSkColorSpace(),
+      true /* is_root_render_pass */);
   RecreateRootRecorder();
 }
 
@@ -851,7 +842,9 @@ SkiaOutputSurfaceImpl::CreateSkSurfaceCharacterization(
         capabilities_.uses_default_gl_framebuffer, false /* isTextureable */,
         impl_on_gpu_->GetGpuPreferences().enforce_vulkan_protected_memory
             ? GrProtected::kYes
-            : GrProtected::kNo);
+            : GrProtected::kNo,
+        false /* vkRTSupportsInputAttachment */,
+        capabilities_.root_is_vulkan_secondary_command_buffer);
     VkFormat vk_format = VK_FORMAT_UNDEFINED;
     LOG_IF(DFATAL, !characterization.isValid())
         << "\n  surface_size=" << surface_size.ToString()