Force NV12 swap chain to have even width and height

NV12 uses 4:2:0 subsampling which means there's one U and V sample per
2x2 block of Y samples.  This means to have an integral number of U and
V samples, both width and height of the surface (or Y plane) must be
even.

Scrolling on a page with video causes swap chain to be resized when the
video's visible rect changes.  Without forcing even width and height for
NV12 swap chains, creating the swap chain often fails with fallback to
BGRA swap chain.  With this fix, creating the swap chain never fails.

Includes a unit test for testing that swap chain is created with even
dimensions after rounding, if necessary, and that the format of the swap
chain is indeed NV12.

Also, includes some minor cosmetic changes to logging to make clear that
the errors codes are hexadecimal.

Bug: 869677
Change-Id: Ief5999149d438e4883fc9c2bb397ab8a7d2b294e
Reviewed-on: https://chromium-review.googlesource.com/c/1308906Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604112}

gpu/ipc/service/direct_composition_surface_win.cc

@@ -348,7 +348,7 @@ bool CreateSurfaceHandleHelper(HANDLE* handle) {
   HRESULT hr = create_surface_handle_function(COMPOSITIONOBJECT_ALL_ACCESS,
                                               nullptr, handle);
   if (FAILED(hr)) {
-    DLOG(ERROR) << "DCompositionCreateSurfaceHandle failed with error "
+    DLOG(ERROR) << "DCompositionCreateSurfaceHandle failed with error 0x"
                 << std::hex << hr;
     return false;
   }
@@ -635,7 +635,7 @@ bool DCLayerTree::Initialize(
   HRESULT hr = desktop_device->CreateTargetForHwnd(
       window, TRUE, dcomp_target_.GetAddressOf());
   if (FAILED(hr)) {
-    DLOG(ERROR) << "CreateTargetForHwnd failed with error " << std::hex << hr;
+    DLOG(ERROR) << "CreateTargetForHwnd failed with error 0x" << std::hex << hr;
     return false;
   }
 
@@ -670,7 +670,7 @@ bool DCLayerTree::InitializeVideoProcessor(const gfx::Size& input_size,
   HRESULT hr = video_device_->CreateVideoProcessorEnumerator(
       &desc, video_processor_enumerator_.GetAddressOf());
   if (FAILED(hr)) {
-    DLOG(ERROR) << "CreateVideoProcessorEnumerator failed with error "
+    DLOG(ERROR) << "CreateVideoProcessorEnumerator failed with error 0x"
                 << std::hex << hr;
     return false;
   }
@@ -678,7 +678,8 @@ bool DCLayerTree::InitializeVideoProcessor(const gfx::Size& input_size,
   hr = video_device_->CreateVideoProcessor(video_processor_enumerator_.Get(), 0,
                                            video_processor_.GetAddressOf());
   if (FAILED(hr)) {
-    DLOG(ERROR) << "CreateVideoProcessor failed with error " << std::hex << hr;
+    DLOG(ERROR) << "CreateVideoProcessor failed with error 0x" << std::hex
+                << hr;
     return false;
   }
 
@@ -896,9 +897,12 @@ gfx::Size DCLayerTree::SwapChainPresenter::CalculateSwapChainSize(
     swap_chain_size = GetStableSwapChainSize(swap_chain_size);
   }
 
-  // YUV surfaces must have an even width.
+  // 4:2:2 subsampled formats like YUY2 must have an even width, and 4:2:0
+  // subsampled formats like NV12 must have an even width and height.
   if (swap_chain_size.width() % 2 == 1)
     swap_chain_size.set_width(swap_chain_size.width() + 1);
+  if (swap_chain_size.height() % 2 == 1)
+    swap_chain_size.set_height(swap_chain_size.height() + 1);
 
   return swap_chain_size;
 }
@@ -1079,7 +1083,7 @@ bool DCLayerTree::SwapChainPresenter::PresentToSwapChain(
   if (first_present) {
     HRESULT hr = swap_chain_->Present(0, 0);
     if (FAILED(hr)) {
-      DLOG(ERROR) << "Present failed with error " << std::hex << hr;
+      DLOG(ERROR) << "Present failed with error 0x" << std::hex << hr;
       return false;
     }
 
@@ -1115,7 +1119,7 @@ bool DCLayerTree::SwapChainPresenter::PresentToSwapChain(
 
   HRESULT hr = swap_chain_->Present(1, 0);
   if (FAILED(hr)) {
-    DLOG(ERROR) << "Present failed with error " << std::hex << hr;
+    DLOG(ERROR) << "Present failed with error 0x" << std::hex << hr;
     return false;
   }
 
@@ -1177,7 +1181,7 @@ bool DCLayerTree::SwapChainPresenter::VideoProcessorBlt(
         texture.Get(), video_processor_enumerator_.Get(), &out_desc,
         out_view_.GetAddressOf());
     if (FAILED(hr)) {
-      DLOG(ERROR) << "CreateVideoProcessorOutputView failed with error "
+      DLOG(ERROR) << "CreateVideoProcessorOutputView failed with error 0x"
                   << std::hex << hr;
       return false;
     }
@@ -1247,7 +1251,7 @@ bool DCLayerTree::SwapChainPresenter::VideoProcessorBlt(
         input_texture.Get(), video_processor_enumerator_.Get(), &in_desc,
         in_view.GetAddressOf());
     if (FAILED(hr)) {
-      DLOG(ERROR) << "CreateVideoProcessorInputView failed with error "
+      DLOG(ERROR) << "CreateVideoProcessorInputView failed with error 0x"
                   << std::hex << hr;
       return false;
     }
@@ -1271,7 +1275,7 @@ bool DCLayerTree::SwapChainPresenter::VideoProcessorBlt(
     hr = video_context_->VideoProcessorBlt(video_processor_.Get(),
                                            out_view_.Get(), 0, 1, &stream);
     if (FAILED(hr)) {
-      DLOG(ERROR) << "VideoProcessorBlt failed with error " << std::hex << hr;
+      DLOG(ERROR) << "VideoProcessorBlt failed with error 0x" << std::hex << hr;
       return false;
     }
   }
@@ -1350,8 +1354,9 @@ bool DCLayerTree::SwapChainPresenter::ReallocateSwapChain(
     if (FAILED(hr)) {
       DLOG(ERROR) << "Failed to create "
                   << OverlayFormatToString(g_overlay_format_used)
-                  << " swap chain with error " << std::hex << hr
-                  << ". Falling back to BGRA";
+                  << " swap chain of size " << swap_chain_size.ToString()
+                  << " with error 0x" << std::hex << hr
+                  << "\nFalling back to BGRA";
     }
   }
 
@@ -1369,7 +1374,8 @@ bool DCLayerTree::SwapChainPresenter::ReallocateSwapChain(
                                   OverlayFormatToString(OverlayFormat::kBGRA),
                               SUCCEEDED(hr));
     if (FAILED(hr)) {
-      DLOG(ERROR) << "Failed to create BGRA swap chain with error " << std::hex
+      DLOG(ERROR) << "Failed to create BGRA swap chain of size "
+                  << swap_chain_size.ToString() << " with error 0x" << std::hex
                   << hr;
       return false;
     }
@@ -1471,7 +1477,7 @@ bool DCLayerTree::CommitAndClearPendingOverlays(
 
     HRESULT hr = dcomp_device_->Commit();
     if (FAILED(hr)) {
-      DLOG(ERROR) << "Commit failed with error " << std::hex << hr;
+      DLOG(ERROR) << "Commit failed with error 0x" << std::hex << hr;
       return false;
     }
   }
@@ -1548,6 +1554,12 @@ int DirectCompositionSurfaceWin::GetNumFramesBeforeSwapChainResizeForTesting() {
   return kNumFramesBeforeSwapChainResize;
 }
 
+// static
+void DirectCompositionSurfaceWin::SetPreferNV12OverlaysForTesting() {
+  g_overlay_format_used = OverlayFormat::kNV12;
+  g_overlay_dxgi_format_used = DXGI_FORMAT_NV12;
+}
+
 // static
 bool DirectCompositionSurfaceWin::IsHDRSupported() {
   // HDR support was introduced in Windows 10 Creators Update.

gpu/ipc/service/direct_composition_surface_win.h

@@ -56,6 +56,8 @@ class GPU_IPC_SERVICE_EXPORT DirectCompositionSurfaceWin
 
   static int GetNumFramesBeforeSwapChainResizeForTesting();
 
+  static void SetPreferNV12OverlaysForTesting();
+
   bool InitializeNativeWindow();
 
   // GLSurfaceEGL implementation.

gpu/ipc/service/direct_composition_surface_win_unittest.cc

@@ -1049,5 +1049,69 @@ TEST_F(DirectCompositionPixelTest, SkipVideoLayerEmptyContentsRect) {
       << actual_color;
 }
 
+TEST_F(DirectCompositionPixelTest, NV12SwapChain) {
+  if (!CheckIfDCSupported())
+    return;
+  DirectCompositionSurfaceWin::SetPreferNV12OverlaysForTesting();
+  InitializeSurface();
+
+  surface_->SetEnableDCLayers(true);
+
+  gfx::Size window_size(100, 100);
+  EXPECT_TRUE(surface_->Resize(window_size, 1.0,
+                               gl::GLSurface::ColorSpace::UNSPECIFIED, true));
+  EXPECT_TRUE(surface_->SetDrawRectangle(gfx::Rect(window_size)));
+
+  glClearColor(0.0, 0.0, 0.0, 1.0);
+  glClear(GL_COLOR_BUFFER_BIT);
+
+  Microsoft::WRL::ComPtr<ID3D11Device> d3d11_device =
+      gl::QueryD3D11DeviceObjectFromANGLE();
+
+  gfx::Size texture_size(50, 50);
+  Microsoft::WRL::ComPtr<ID3D11Texture2D> texture =
+      CreateNV12Texture(d3d11_device, texture_size, true);
+  Microsoft::WRL::ComPtr<IDXGIResource1> resource;
+  texture.CopyTo(resource.GetAddressOf());
+  HANDLE handle = 0;
+  resource->CreateSharedHandle(nullptr, DXGI_SHARED_RESOURCE_READ, nullptr,
+                               &handle);
+  // The format doesn't matter, since we aren't binding.
+  scoped_refptr<gl::GLImageDXGIHandle> image_dxgi(
+      new gl::GLImageDXGIHandle(texture_size, 0, gfx::BufferFormat::RGBA_8888));
+  ASSERT_TRUE(image_dxgi->Initialize(base::win::ScopedHandle(handle)));
+
+  // Pass content rect with odd with and height.  Surface should round up width
+  // and height when creating swap chain.
+  gfx::RectF contents_rect(0, 0, 49, 49);
+  ui::DCRendererLayerParams params(
+      false, gfx::Rect(), 1, gfx::Transform(),
+      std::vector<scoped_refptr<gl::GLImage>>{image_dxgi}, contents_rect,
+      gfx::Rect(window_size), 0, 0, 1.0, 0, false);
+  surface_->ScheduleDCLayer(params);
+
+  EXPECT_EQ(gfx::SwapResult::SWAP_ACK,
+            surface_->SwapBuffers(base::DoNothing()));
+
+  Sleep(1000);
+
+  Microsoft::WRL::ComPtr<IDXGISwapChain1> swap_chain =
+      surface_->GetLayerSwapChainForTesting(0);
+  ASSERT_TRUE(swap_chain);
+
+  DXGI_SWAP_CHAIN_DESC1 desc;
+  EXPECT_TRUE(SUCCEEDED(swap_chain->GetDesc1(&desc)));
+  EXPECT_EQ(desc.Format, DXGI_FORMAT_NV12);
+  EXPECT_EQ(desc.Width, 50u);
+  EXPECT_EQ(desc.Height, 50u);
+
+  SkColor expected_color = SkColorSetRGB(0xe1, 0x90, 0xeb);
+  SkColor actual_color =
+      ReadBackWindowPixel(window_.hwnd(), gfx::Point(75, 75));
+  EXPECT_TRUE(AreColorsSimilar(expected_color, actual_color))
+      << std::hex << "Expected " << expected_color << " Actual "
+      << actual_color;
+}
+
 }  // namespace
 }  // namespace gpu