Separate video processor usage by SDR/HDR mode.

This CL makes DCLayerTree to keep a Map for video processor which is
separated by SDR/HDR mode. The purpose is to prevent causing hardware
error when change VP from dealing HDR content to SDR content in some
Intel platform.

This CL also changes the place where setting colorspace of VP to
SwapChainPresenter::VideoProcessorBlt().

Bug: 1121061
Change-Id: I4a669750cb88d68d8655cb2d777c555b43ac3f5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2539559Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Richard Li <richard.li@intel.com>
Cr-Commit-Position: refs/heads/master@{#832608}

gpu/config/gpu_driver_bug_list.json

@@ -3504,18 +3504,6 @@
         "disable_accelerated_vp8_decode"
       ]
     },
-    {
-      "id": 350,
-      "cr_bugs": [1121061],
-      "description": "Intel GPUs need reset VP when colorspace changes",
-      "os": {
-        "type": "win"
-      },
-      "vendor_id": "0x8086",
-      "features": [
-        "reset_vp_when_colorspace_changes"
-      ]
-    },
     {
       "id": 352,
       "cr_bugs": [1038006],

gpu/config/gpu_workaround_list.txt

@@ -102,7 +102,6 @@ remove_invariant_and_centroid_for_essl3
 remove_pow_with_constant_exponent
 reset_base_mipmap_level_before_texstorage
 reset_teximage2d_base_level
-reset_vp_when_colorspace_changes
 restore_scissor_on_fbo_change
 rewrite_do_while_loops
 rewrite_float_unary_minus_operator

gpu/ipc/service/image_transport_surface_win.cc

@@ -31,8 +31,6 @@ CreateDirectCompositionSurfaceSettings(
       workarounds.disable_nv12_dynamic_textures;
   settings.disable_vp_scaling = workarounds.disable_vp_scaling;
   settings.use_angle_texture_offset = features::IsUsingSkiaRenderer();
-  settings.reset_vp_when_colorspace_changes =
-      workarounds.reset_vp_when_colorspace_changes;
   settings.force_root_surface_full_damage =
       features::IsUsingSkiaRenderer() &&
       gl::ShouldForceDirectCompositionRootSurfaceFullDamage();

ui/gl/dc_layer_tree.cc

@@ -19,12 +19,17 @@ bool SizeContains(const gfx::Size& a, const gfx::Size& b) {
 }
 }  // namespace
 
+VideoProcessorWrapper::VideoProcessorWrapper() = default;
+VideoProcessorWrapper::~VideoProcessorWrapper() = default;
+VideoProcessorWrapper::VideoProcessorWrapper(VideoProcessorWrapper&& other) =
+    default;
+VideoProcessorWrapper& VideoProcessorWrapper::operator=(
+    VideoProcessorWrapper&& other) = default;
+
 DCLayerTree::DCLayerTree(bool disable_nv12_dynamic_textures,
-                         bool disable_vp_scaling,
-                         bool reset_vp_when_colorspace_changes)
+                         bool disable_vp_scaling)
     : disable_nv12_dynamic_textures_(disable_nv12_dynamic_textures),
-      disable_vp_scaling_(disable_vp_scaling),
-      reset_vp_when_colorspace_changes_(reset_vp_when_colorspace_changes) {}
+      disable_vp_scaling_(disable_vp_scaling) {}
 
 DCLayerTree::~DCLayerTree() = default;
 
@@ -65,49 +70,42 @@ bool DCLayerTree::Initialize(
   return true;
 }
 
-bool DCLayerTree::InitializeVideoProcessor(
+VideoProcessorWrapper* DCLayerTree::InitializeVideoProcessor(
     const gfx::Size& input_size,
     const gfx::Size& output_size,
-    const gfx::ColorSpace& input_color_space,
-    const gfx::ColorSpace& output_color_space,
-    Microsoft::WRL::ComPtr<IDXGISwapChain1> swap_chain,
-    bool is_yuv_swapchain) {
-  if (!video_device_) {
+    bool is_hdr_output) {
+  VideoProcessorWrapper& video_processor_wrapper =
+      GetOrCreateVideoProcessor(is_hdr_output);
+
+  if (!video_processor_wrapper.video_device) {
     // This can fail if the D3D device is "Microsoft Basic Display Adapter".
-    if (FAILED(d3d11_device_.As(&video_device_))) {
+    if (FAILED(d3d11_device_.As(&video_processor_wrapper.video_device))) {
       DLOG(ERROR) << "Failed to retrieve video device from D3D11 device";
       DCHECK(false);
       DirectCompositionSurfaceWin::DisableOverlays();
-      return false;
+      return nullptr;
     }
-    DCHECK(video_device_);
+    DCHECK(video_processor_wrapper.video_device);
 
     Microsoft::WRL::ComPtr<ID3D11DeviceContext> context;
     d3d11_device_->GetImmediateContext(&context);
     DCHECK(context);
-    context.As(&video_context_);
-    DCHECK(video_context_);
+    context.As(&video_processor_wrapper.video_context);
+    DCHECK(video_processor_wrapper.video_context);
   }
 
-  bool colorspace_changed = !(input_color_space == video_input_color_space_ &&
-                              output_color_space == video_output_color_space_ &&
-                              is_yuv_video_output_ == is_yuv_swapchain);
-  if (video_processor_ && SizeContains(video_input_size_, input_size) &&
-      SizeContains(video_output_size_, output_size) &&
-      !(colorspace_changed && reset_vp_when_colorspace_changes_)) {
-    if (colorspace_changed) {
-      SetColorSpaceForVideoProcessor(input_color_space, output_color_space,
-                                     std::move(swap_chain), is_yuv_swapchain);
-    }
-    return true;
-  }
+  if (video_processor_wrapper.video_processor &&
+      SizeContains(video_processor_wrapper.video_input_size, input_size) &&
+      SizeContains(video_processor_wrapper.video_output_size, output_size))
+    return &video_processor_wrapper;
+
   TRACE_EVENT2("gpu", "DCLayerTree::InitializeVideoProcessor", "input_size",
                input_size.ToString(), "output_size", output_size.ToString());
-  video_input_size_ = input_size;
-  video_output_size_ = output_size;
+  video_processor_wrapper.video_input_size = input_size;
+  video_processor_wrapper.video_output_size = output_size;
 
-  video_processor_.Reset();
-  video_processor_enumerator_.Reset();
+  video_processor_wrapper.video_processor.Reset();
+  video_processor_wrapper.video_processor_enumerator.Reset();
   D3D11_VIDEO_PROCESSOR_CONTENT_DESC desc = {};
   desc.InputFrameFormat = D3D11_VIDEO_FRAME_FORMAT_PROGRESSIVE;
   desc.InputFrameRate.Numerator = 60;
@@ -119,8 +117,9 @@ bool DCLayerTree::InitializeVideoProcessor(
   desc.OutputWidth = output_size.width();
   desc.OutputHeight = output_size.height();
   desc.Usage = D3D11_VIDEO_USAGE_PLAYBACK_NORMAL;
-  HRESULT hr = video_device_->CreateVideoProcessorEnumerator(
-      &desc, &video_processor_enumerator_);
+  HRESULT hr =
+      video_processor_wrapper.video_device->CreateVideoProcessorEnumerator(
+          &desc, &video_processor_wrapper.video_processor_enumerator);
   base::UmaHistogramSparse(
       "GPU.DirectComposition.CreateVideoProcessorEnumerator", hr);
   if (FAILED(hr)) {
@@ -129,11 +128,11 @@ bool DCLayerTree::InitializeVideoProcessor(
     // It might fail again next time. Disable overlay support so
     // overlay processor will stop sending down overlay frames.
     DirectCompositionSurfaceWin::DisableOverlays();
-    return false;
+    return nullptr;
   }
-
-  hr = video_device_->CreateVideoProcessor(video_processor_enumerator_.Get(), 0,
-                                           &video_processor_);
+  hr = video_processor_wrapper.video_device->CreateVideoProcessor(
+      video_processor_wrapper.video_processor_enumerator.Get(), 0,
+      &video_processor_wrapper.video_processor);
   base::UmaHistogramSparse(
       "GPU.DirectComposition.VideoDeviceCreateVideoProcessor", hr);
   if (FAILED(hr)) {
@@ -142,55 +141,21 @@ bool DCLayerTree::InitializeVideoProcessor(
     // It might fail again next time. Disable overlay support so
     // overlay processor will stop sending down overlay frames.
     DirectCompositionSurfaceWin::DisableOverlays();
-    return false;
+    return nullptr;
   }
   // Auto stream processing (the default) can hurt power consumption.
-  video_context_->VideoProcessorSetStreamAutoProcessingMode(
-      video_processor_.Get(), 0, FALSE);
-  SetColorSpaceForVideoProcessor(input_color_space, output_color_space,
-                                 std::move(swap_chain), is_yuv_swapchain);
-  return true;
+  video_processor_wrapper.video_context
+      ->VideoProcessorSetStreamAutoProcessingMode(
+          video_processor_wrapper.video_processor.Get(), 0, FALSE);
+  return &video_processor_wrapper;
 }
 
-void DCLayerTree::SetColorSpaceForVideoProcessor(
-    const gfx::ColorSpace& input_color_space,
-    const gfx::ColorSpace& output_color_space,
-    Microsoft::WRL::ComPtr<IDXGISwapChain1> swap_chain,
-    bool is_yuv_swapchain) {
-  Microsoft::WRL::ComPtr<IDXGISwapChain3> swap_chain3;
-  Microsoft::WRL::ComPtr<ID3D11VideoContext1> context1;
-  if (SUCCEEDED(swap_chain.As(&swap_chain3)) &&
-      SUCCEEDED(video_context_.As(&context1))) {
-    DCHECK(swap_chain3);
-    DCHECK(context1);
-    // Set input color space.
-    context1->VideoProcessorSetStreamColorSpace1(
-        video_processor_.Get(), 0,
-        gfx::ColorSpaceWin::GetDXGIColorSpace(input_color_space));
-    // Set output color space.
-    DXGI_COLOR_SPACE_TYPE output_dxgi_color_space =
-        gfx::ColorSpaceWin::GetDXGIColorSpace(output_color_space,
-                                              /*force_yuv=*/is_yuv_swapchain);
-
-    if (SUCCEEDED(swap_chain3->SetColorSpace1(output_dxgi_color_space))) {
-      context1->VideoProcessorSetOutputColorSpace1(video_processor_.Get(),
-                                                   output_dxgi_color_space);
-    }
-  } else {
-    // This can't handle as many different types of color spaces, so use it
-    // only if ID3D11VideoContext1 isn't available.
-    D3D11_VIDEO_PROCESSOR_COLOR_SPACE src_d3d11_color_space =
-        gfx::ColorSpaceWin::GetD3D11ColorSpace(input_color_space);
-    video_context_->VideoProcessorSetStreamColorSpace(video_processor_.Get(), 0,
-                                                      &src_d3d11_color_space);
-    D3D11_VIDEO_PROCESSOR_COLOR_SPACE output_d3d11_color_space =
-        gfx::ColorSpaceWin::GetD3D11ColorSpace(output_color_space);
-    video_context_->VideoProcessorSetOutputColorSpace(
-        video_processor_.Get(), &output_d3d11_color_space);
-  }
-  video_input_color_space_ = input_color_space;
-  video_output_color_space_ = output_color_space;
-  is_yuv_video_output_ = is_yuv_swapchain;
+VideoProcessorWrapper& DCLayerTree::GetOrCreateVideoProcessor(bool is_hdr) {
+  VideoProcessorType video_processor_type =
+      is_hdr ? VideoProcessorType::kHDR : VideoProcessorType::kSDR;
+  return video_processor_map_
+      .try_emplace(video_processor_type, VideoProcessorWrapper())
+      .first->second;
 }
 
 Microsoft::WRL::ComPtr<IDXGISwapChain1>

ui/gl/dc_layer_tree.h

@@ -12,6 +12,7 @@
 
 #include <memory>
 
+#include "base/containers/flat_map.h"
 #include "ui/gfx/color_space_win.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gl/dc_renderer_layer_params.h"
@@ -22,15 +23,41 @@ namespace gl {
 class DirectCompositionChildSurfaceWin;
 class SwapChainPresenter;
 
+enum class VideoProcessorType { kSDR, kHDR };
+
+// Cache video processor and its size.
+struct VideoProcessorWrapper {
+  VideoProcessorWrapper();
+  ~VideoProcessorWrapper();
+  VideoProcessorWrapper(VideoProcessorWrapper&& other);
+  VideoProcessorWrapper& operator=(VideoProcessorWrapper&& other);
+  VideoProcessorWrapper(const VideoProcessorWrapper&) = delete;
+  VideoProcessorWrapper& operator=(VideoProcessorWrapper& other) = delete;
+
+  // Input and output size of video processor .
+  gfx::Size video_input_size;
+  gfx::Size video_output_size;
+
+  // The video processor is cached so SwapChains don't have to recreate it
+  // whenever they're created.
+  Microsoft::WRL::ComPtr<ID3D11VideoDevice> video_device;
+  Microsoft::WRL::ComPtr<ID3D11VideoContext> video_context;
+  Microsoft::WRL::ComPtr<ID3D11VideoProcessor> video_processor;
+  Microsoft::WRL::ComPtr<ID3D11VideoProcessorEnumerator>
+      video_processor_enumerator;
+};
+
 // DCLayerTree manages a tree of direct composition visuals, and associated
 // swap chains for given overlay layers.  It maintains a list of pending layers
 // submitted using ScheduleDCLayer() that are presented and committed in
 // CommitAndClearPendingOverlays().
 class DCLayerTree {
  public:
-  DCLayerTree(bool disable_nv12_dynamic_textures,
-              bool disable_vp_scaling,
-              bool reset_vp_when_colorspace_changes);
+  using VideoProcessorMap =
+      base::flat_map<VideoProcessorType, VideoProcessorWrapper>;
+
+  DCLayerTree(bool disable_nv12_dynamic_textures, bool disable_vp_scaling);
+
   ~DCLayerTree();
 
   // Returns true on success.
@@ -50,13 +77,9 @@ class DCLayerTree {
   // at least given input and output size.  The video processor is shared across
   // layers so the same one can be reused if it's large enough.  Returns true on
   // success.
-  bool InitializeVideoProcessor(
-      const gfx::Size& input_size,
-      const gfx::Size& output_size,
-      const gfx::ColorSpace& input_color_space,
-      const gfx::ColorSpace& output_color_space,
-      Microsoft::WRL::ComPtr<IDXGISwapChain1> swap_chain,
-      bool is_yuv_swapchain);
+  VideoProcessorWrapper* InitializeVideoProcessor(const gfx::Size& input_size,
+                                                  const gfx::Size& output_size,
+                                                  bool is_hdr_output);
 
   void SetNeedsRebuildVisualTree() { needs_rebuild_visual_tree_ = true; }
 
@@ -66,22 +89,7 @@ class DCLayerTree {
 
   bool disable_vp_scaling() const { return disable_vp_scaling_; }
 
-  const Microsoft::WRL::ComPtr<ID3D11VideoDevice>& video_device() const {
-    return video_device_;
-  }
-
-  const Microsoft::WRL::ComPtr<ID3D11VideoContext>& video_context() const {
-    return video_context_;
-  }
-
-  const Microsoft::WRL::ComPtr<ID3D11VideoProcessor>& video_processor() const {
-    return video_processor_;
-  }
-
-  const Microsoft::WRL::ComPtr<ID3D11VideoProcessorEnumerator>&
-  video_processor_enumerator() const {
-    return video_processor_enumerator_;
-  }
+  VideoProcessorWrapper& GetOrCreateVideoProcessor(bool is_hdr);
 
   Microsoft::WRL::ComPtr<IDXGISwapChain1> GetLayerSwapChainForTesting(
       size_t index) const;
@@ -100,37 +108,21 @@ class DCLayerTree {
   HWND window() const { return window_; }
 
  private:
-  void SetColorSpaceForVideoProcessor(
-      const gfx::ColorSpace& input_color_space,
-      const gfx::ColorSpace& output_color_space,
-      Microsoft::WRL::ComPtr<IDXGISwapChain1> swap_chain,
-      bool is_yuv_swapchain);
-
   const bool disable_nv12_dynamic_textures_;
   const bool disable_vp_scaling_;
-  const bool reset_vp_when_colorspace_changes_;
 
   HWND window_;
   Microsoft::WRL::ComPtr<ID3D11Device> d3d11_device_;
   Microsoft::WRL::ComPtr<IDCompositionDevice2> dcomp_device_;
   Microsoft::WRL::ComPtr<IDCompositionTarget> dcomp_target_;
 
-  // The video processor is cached so SwapChains don't have to recreate it
-  // whenever they're created.
-  Microsoft::WRL::ComPtr<ID3D11VideoDevice> video_device_;
-  Microsoft::WRL::ComPtr<ID3D11VideoContext> video_context_;
-  Microsoft::WRL::ComPtr<ID3D11VideoProcessor> video_processor_;
-  Microsoft::WRL::ComPtr<ID3D11VideoProcessorEnumerator>
-      video_processor_enumerator_;
-
-  // Current video processor input and output size.
-  gfx::Size video_input_size_;
-  gfx::Size video_output_size_;
+  // Store video processor for SDR/HDR mode separately, which could avoid
+  // problem in (http://crbug.com/1121061).
+  VideoProcessorMap video_processor_map_;
 
   // Current video processor input and output colorspace.
   gfx::ColorSpace video_input_color_space_;
   gfx::ColorSpace video_output_color_space_;
-  bool is_yuv_video_output_ = false;
 
   // Set to true if a direct composition visual tree needs rebuild.
   bool needs_rebuild_visual_tree_ = false;

ui/gl/direct_composition_surface_win.cc

@@ -359,10 +359,9 @@ DirectCompositionSurfaceWin::DirectCompositionSurfaceWin(
           settings.use_angle_texture_offset,
           settings.max_pending_frames,
           settings.force_root_surface_full_damage)),
-      layer_tree_(std::make_unique<DCLayerTree>(
-          settings.disable_nv12_dynamic_textures,
-          settings.disable_vp_scaling,
-          settings.reset_vp_when_colorspace_changes)) {
+      layer_tree_(
+          std::make_unique<DCLayerTree>(settings.disable_nv12_dynamic_textures,
+                                        settings.disable_vp_scaling)) {
   ui::GpuSwitchingManager::GetInstance()->AddObserver(this);
 }
 

ui/gl/direct_composition_surface_win.h

@@ -35,7 +35,6 @@ class GL_EXPORT DirectCompositionSurfaceWin : public GLSurfaceEGL,
     bool disable_vp_scaling = false;
     size_t max_pending_frames = 2;
     bool use_angle_texture_offset = false;
-    bool reset_vp_when_colorspace_changes = false;
     bool force_root_surface_full_damage = false;
   };
 

ui/gl/swap_chain_presenter.cc

@@ -1087,9 +1087,9 @@ bool SwapChainPresenter::VideoProcessorBlt(
   // TODO(sunnyps): Ensure output color space for YUV swap chains is Rec709 or
   // Rec601 so that the conversion from gfx::ColorSpace to DXGI_COLOR_SPACE
   // doesn't need a |force_yuv| parameter (and the associated plumbing).
-  gfx::ColorSpace output_color_space = IsYUVSwapChainFormat(swap_chain_format_)
-                                           ? src_color_space
-                                           : gfx::ColorSpace::CreateSRGB();
+  bool is_yuv_swapchain = IsYUVSwapChainFormat(swap_chain_format_);
+  gfx::ColorSpace output_color_space =
+      is_yuv_swapchain ? src_color_space : gfx::ColorSpace::CreateSRGB();
   if (base::FeatureList::IsEnabled(kFallbackBT709VideoToBT601) &&
       (output_color_space == gfx::ColorSpace::CreateREC709())) {
     output_color_space = gfx::ColorSpace::CreateREC601();
@@ -1097,17 +1097,48 @@ bool SwapChainPresenter::VideoProcessorBlt(
   if (content_is_hdr)
     output_color_space = gfx::ColorSpace::CreateHDR10();
 
-  if (!layer_tree_->InitializeVideoProcessor(
-          content_rect.size(), swap_chain_size_, src_color_space,
-          output_color_space, swap_chain_,
-          IsYUVSwapChainFormat(swap_chain_format_))) {
+  VideoProcessorWrapper* video_processor_wrapper =
+      layer_tree_->InitializeVideoProcessor(
+          content_rect.size(), swap_chain_size_, output_color_space.IsHDR());
+  if (!video_processor_wrapper)
     return false;
-  }
 
   Microsoft::WRL::ComPtr<ID3D11VideoContext> video_context =
-      layer_tree_->video_context();
+      video_processor_wrapper->video_context;
   Microsoft::WRL::ComPtr<ID3D11VideoProcessor> video_processor =
-      layer_tree_->video_processor();
+      video_processor_wrapper->video_processor;
+
+  Microsoft::WRL::ComPtr<IDXGISwapChain3> swap_chain3;
+  Microsoft::WRL::ComPtr<ID3D11VideoContext1> context1;
+  if (SUCCEEDED(swap_chain_.As(&swap_chain3)) &&
+      SUCCEEDED(video_context.As(&context1))) {
+    DCHECK(swap_chain3);
+    DCHECK(context1);
+    // Set input color space.
+    context1->VideoProcessorSetStreamColorSpace1(
+        video_processor.Get(), 0,
+        gfx::ColorSpaceWin::GetDXGIColorSpace(src_color_space));
+    // Set output color space.
+    DXGI_COLOR_SPACE_TYPE output_dxgi_color_space =
+        gfx::ColorSpaceWin::GetDXGIColorSpace(output_color_space,
+                                              /*force_yuv=*/is_yuv_swapchain);
+
+    if (SUCCEEDED(swap_chain3->SetColorSpace1(output_dxgi_color_space))) {
+      context1->VideoProcessorSetOutputColorSpace1(video_processor.Get(),
+                                                   output_dxgi_color_space);
+    }
+  } else {
+    // This can't handle as many different types of color spaces, so use it
+    // only if ID3D11VideoContext1 isn't available.
+    D3D11_VIDEO_PROCESSOR_COLOR_SPACE src_d3d11_color_space =
+        gfx::ColorSpaceWin::GetD3D11ColorSpace(src_color_space);
+    video_context->VideoProcessorSetStreamColorSpace(video_processor.Get(), 0,
+                                                     &src_d3d11_color_space);
+    D3D11_VIDEO_PROCESSOR_COLOR_SPACE output_d3d11_color_space =
+        gfx::ColorSpaceWin::GetD3D11ColorSpace(output_color_space);
+    video_context->VideoProcessorSetOutputColorSpace(video_processor.Get(),
+                                                     &output_d3d11_color_space);
+  }
   Microsoft::WRL::ComPtr<ID3D11VideoContext2> context2;
   base::Optional<DXGI_HDR_METADATA_HDR10> display_metadata =
       layer_tree_->GetHDRMetadataHelper()->GetDisplayMetadata();
@@ -1141,9 +1172,10 @@ bool SwapChainPresenter::VideoProcessorBlt(
     }
 
     Microsoft::WRL::ComPtr<ID3D11VideoDevice> video_device =
-        layer_tree_->video_device();
+        video_processor_wrapper->video_device;
     Microsoft::WRL::ComPtr<ID3D11VideoProcessorEnumerator>
-        video_processor_enumerator = layer_tree_->video_processor_enumerator();
+        video_processor_enumerator =
+            video_processor_wrapper->video_processor_enumerator;
 
     D3D11_VIDEO_PROCESSOR_INPUT_VIEW_DESC input_desc = {};
     input_desc.ViewDimension = D3D11_VPIV_DIMENSION_TEXTURE2D;