Commit d6fa4999 authored by Sunny Sachanandani's avatar Sunny Sachanandani Committed by Commit Bot

gpu: Do not hold on to swap chain in DCLayerTree

The CL to fix HDR color banding issues (https://crrev.com/c/2411731)
introduced a bug by holding on to the last swap chain which asked for
the video processor in DCLayerTree. This CL attempts to fix that in the
least impactful way so that it can be safely merged to M87.

Bug: 1126365
Change-Id: I2cbae929738598782b59e5d3390fcc4dddfc5673
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2444189
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: default avatarZhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814483}
parent c8ad736a
......@@ -69,7 +69,9 @@ bool DCLayerTree::InitializeVideoProcessor(
const gfx::Size& input_size,
const gfx::Size& output_size,
const gfx::ColorSpace& input_color_space,
const gfx::ColorSpace& output_color_space) {
const gfx::ColorSpace& output_color_space,
Microsoft::WRL::ComPtr<IDXGISwapChain1> swap_chain,
bool is_yuv_swapchain) {
if (!video_device_) {
// This can fail if the D3D device is "Microsoft Basic Display Adapter".
if (FAILED(d3d11_device_.As(&video_device_))) {
......@@ -88,10 +90,15 @@ bool DCLayerTree::InitializeVideoProcessor(
}
bool colorspace_changed = !(input_color_space == video_input_color_space_ &&
output_color_space == video_output_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;
}
TRACE_EVENT2("gpu", "DCLayerTree::InitializeVideoProcessor", "input_size",
......@@ -137,34 +144,22 @@ bool DCLayerTree::InitializeVideoProcessor(
DirectCompositionSurfaceWin::DisableOverlays();
return false;
}
// Color spaces should be updated later through
// SetColorspaceForVideoProcessor() for the new VP.
video_input_color_space_ = gfx::ColorSpace();
video_output_color_space_ = gfx::ColorSpace();
// 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;
}
void DCLayerTree::SetColorspaceForVideoProcessor(
void DCLayerTree::SetColorSpaceForVideoProcessor(
const gfx::ColorSpace& input_color_space,
const gfx::ColorSpace& output_color_space,
Microsoft::WRL::ComPtr<IDXGISwapChain1> swapchain,
Microsoft::WRL::ComPtr<IDXGISwapChain1> swap_chain,
bool is_yuv_swapchain) {
// Reset colorspace of video processor in two circumstances:
// 1. Input/output colorspaces change.
// 2. Swapchain changes.
// Others just return.
if ((input_color_space == video_input_color_space_) &&
(output_color_space == video_output_color_space_) &&
swapchain == last_swapchain_setting_colorspace_)
return;
Microsoft::WRL::ComPtr<IDXGISwapChain3> swap_chain3;
Microsoft::WRL::ComPtr<ID3D11VideoContext1> context1;
if (SUCCEEDED(swapchain.As(&swap_chain3)) &&
if (SUCCEEDED(swap_chain.As(&swap_chain3)) &&
SUCCEEDED(video_context_.As(&context1))) {
DCHECK(swap_chain3);
DCHECK(context1);
......@@ -175,13 +170,12 @@ void DCLayerTree::SetColorspaceForVideoProcessor(
// Set output color space.
DXGI_COLOR_SPACE_TYPE output_dxgi_color_space =
gfx::ColorSpaceWin::GetDXGIColorSpace(output_color_space,
is_yuv_swapchain /* force_yuv */);
/*force_yuv=*/is_yuv_swapchain);
if (SUCCEEDED(swap_chain3->SetColorSpace1(output_dxgi_color_space))) {
context1->VideoProcessorSetOutputColorSpace1(video_processor_.Get(),
output_dxgi_color_space);
}
last_swapchain_setting_colorspace_ = swapchain;
} else {
// This can't handle as many different types of color spaces, so use it
// only if ID3D11VideoContext1 isn't available.
......@@ -196,6 +190,7 @@ void DCLayerTree::SetColorspaceForVideoProcessor(
}
video_input_color_space_ = input_color_space;
video_output_color_space_ = output_color_space;
is_yuv_video_output_ = is_yuv_swapchain;
}
Microsoft::WRL::ComPtr<IDXGISwapChain1>
......
......@@ -51,15 +51,12 @@ 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);
void SetColorspaceForVideoProcessor(
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> swapchain,
Microsoft::WRL::ComPtr<IDXGISwapChain1> swap_chain,
bool is_yuv_swapchain);
void SetNeedsRebuildVisualTree() { needs_rebuild_visual_tree_ = true; }
......@@ -101,6 +98,12 @@ class DCLayerTree {
}
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_larger_than_screen_overlays_;
const bool disable_vp_scaling_;
......@@ -125,9 +128,7 @@ class DCLayerTree {
// Current video processor input and output colorspace.
gfx::ColorSpace video_input_color_space_;
gfx::ColorSpace video_output_color_space_;
// Cache the last swapchain that has been set output colorspace.
Microsoft::WRL::ComPtr<IDXGISwapChain1> last_swapchain_setting_colorspace_;
bool is_yuv_video_output_ = false;
// Set to true if a direct composition visual tree needs rebuild.
bool needs_rebuild_visual_tree_ = false;
......
......@@ -983,6 +983,9 @@ bool SwapChainPresenter::VideoProcessorBlt(
content_rect.ToString(), "swap_chain_size",
swap_chain_size_.ToString());
// 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();
......@@ -993,14 +996,12 @@ 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)) {
if (!layer_tree_->InitializeVideoProcessor(
content_rect.size(), swap_chain_size_, src_color_space,
output_color_space, swap_chain_,
IsYUVSwapChainFormat(swap_chain_format_))) {
return false;
}
layer_tree_->SetColorspaceForVideoProcessor(
src_color_space, output_color_space, swap_chain_,
IsYUVSwapChainFormat(swap_chain_format_));
Microsoft::WRL::ComPtr<ID3D11VideoContext> video_context =
layer_tree_->video_context();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment