SkiaRenderer: Support changing color space

SkiaOutputSurfaceImpl did not handle the color space changing after it
was created previously. The SkSurfaceCharacterization color space was
only set during the first time Reshape() ran when the charactization is
returned from the GPU thread. If the color space was changed later the
SkSurface and SkDDL color spaces no longer matched and draw failed.

Bug: 1009452
Change-Id: Ib6d2083efc7e7eb6f94782342e92a809b69d6fdc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1841811Reviewed-by: Peng Huang <penghuang@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702946}

components/viz/service/display_embedder/skia_output_surface_impl.cc

@@ -193,14 +193,22 @@ void SkiaOutputSurfaceImpl::Reshape(const gfx::Size& size,
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   if (initialize_waitable_event_) {
     initialize_waitable_event_->Wait();
-    initialize_waitable_event_ = nullptr;
+    initialize_waitable_event_.reset();
   }
 
   SkSurfaceCharacterization* characterization = nullptr;
   if (characterization_.isValid()) {
-    // TODO(weiliang): support color space. https://crbug.com/795132
-    characterization_ =
-        characterization_.createResized(size.width(), size.height());
+    sk_sp<SkColorSpace> sk_color_space = color_space.ToSkColorSpace();
+    if (!SkColorSpace::Equals(characterization_.refColorSpace().get(),
+                              sk_color_space.get())) {
+      characterization_ = characterization_.createColorSpace(sk_color_space);
+    }
+    if (size.width() != characterization_.width() ||
+        size.height() != characterization_.height()) {
+      characterization_ =
+          characterization_.createResized(size.width(), size.height());
+    }
+    // TODO(kylechar): Update |characterization_| if |use_alpha| changes.
     RecreateRootRecorder();
   } else {
     characterization = &characterization_;
@@ -211,12 +219,12 @@ void SkiaOutputSurfaceImpl::Reshape(const gfx::Size& size,
 
   // impl_on_gpu_ is released on the GPU thread by a posted task from
   // SkiaOutputSurfaceImpl::dtor. So it is safe to use base::Unretained.
-  auto callback = base::BindOnce(
-      &SkiaOutputSurfaceImplOnGpu::Reshape,
-      base::Unretained(impl_on_gpu_.get()), size, device_scale_factor,
-      std::move(color_space), has_alpha, use_stencil, pre_transform_,
-      characterization, initialize_waitable_event_.get());
-  ScheduleGpuTask(std::move(callback), std::vector<gpu::SyncToken>());
+  auto task = base::BindOnce(&SkiaOutputSurfaceImplOnGpu::Reshape,
+                             base::Unretained(impl_on_gpu_.get()), size,
+                             device_scale_factor, color_space, has_alpha,
+                             use_stencil, pre_transform_, characterization,
+                             initialize_waitable_event_.get());
+  ScheduleGpuTask(std::move(task), {});
 }
 
 void SkiaOutputSurfaceImpl::SetUpdateVSyncParametersCallback(

components/viz/service/display_embedder/skia_output_surface_impl_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/run_loop.h"
 #include "base/test/scoped_feature_list.h"
 #include "cc/test/fake_output_surface_client.h"
 #include "cc/test/pixel_test_utils.h"
@@ -18,6 +19,7 @@
 #include "components/viz/common/frame_sinks/copy_output_request.h"
 #include "components/viz/common/frame_sinks/copy_output_result.h"
 #include "components/viz/common/frame_sinks/copy_output_util.h"
+#include "components/viz/service/display/output_surface_frame.h"
 #include "components/viz/service/display_embedder/skia_output_surface_dependency_impl.h"
 #include "components/viz/service/gl/gpu_service_impl.h"
 #include "components/viz/test/test_gpu_service_holder.h"
@@ -197,12 +199,34 @@ TEST_P(SkiaOutputSurfaceImplTest, SubmitPaint) {
       base::BindOnce(&SkiaOutputSurfaceImplTest::CheckSyncTokenOnGpuThread,
                      base::Unretained(this), sync_token);
 
-  std::vector<gpu::SyncToken> resource_sync_tokens;
-  resource_sync_tokens.push_back(sync_token);
-  output_surface_->ScheduleGpuTaskForTesting(std::move(closure),
-                                             std::move(resource_sync_tokens));
+  output_surface_->ScheduleGpuTaskForTesting(std::move(closure), {sync_token});
   BlockMainThread();
   EXPECT_TRUE(on_finished_called);
 }
 
+// Draws two frames and calls Reshape() between the two frames changing the
+// color space. Verifies draw after color space change is successful.
+TEST_P(SkiaOutputSurfaceImplTest, SupportsColorSpaceChange) {
+  for (auto& color_space : {gfx::ColorSpace(), gfx::ColorSpace::CreateSRGB()}) {
+    output_surface_->Reshape(kSurfaceRect.size(), 1, color_space,
+                             /*has_alpha=*/false, /*use_stencil=*/false);
+
+    // Draw something, it's not important what.
+    SkCanvas* root_canvas = output_surface_->BeginPaintCurrentFrame();
+    SkPaint paint;
+    paint.setColor(SK_ColorRED);
+    root_canvas->drawRect(SkRect::MakeWH(10, 10), paint);
+
+    base::RunLoop run_loop;
+    output_surface_->SubmitPaint(run_loop.QuitClosure());
+
+    OutputSurfaceFrame frame;
+    frame.size = kSurfaceRect.size();
+    output_surface_->SkiaSwapBuffers(std::move(frame),
+                                     /*wants_sync_token=*/false);
+
+    run_loop.Run();
+  }
+}
+
 }  // namespace viz