Skip to content

platform/windows: implement IDXGIOutput5 / Per Monitor v2 #522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
psyke83 wants to merge 4 commits into from
Closed

platform/windows: implement IDXGIOutput5 / Per Monitor v2 #522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5b7c400
platform/windows: implement IDXGIOutput5 / Per Monitor v2
psyke83 Dec 1, 2022
7bad3bf
platform/windows/display_vram: fix Vulkan / real exclusive fullscreen…
psyke83 Dec 22, 2022
62f98b8
platform/windows/display_ram: fix incorrect colours for vulkan/exclus…
psyke83 Dec 22, 2022
d247122
Merge branch 'nightly' into output5
ReenigneArcher Dec 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/platform/common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ struct img_t {
std::int32_t height {};
std::int32_t pixel_pitch {};
std::int32_t row_pitch {};
bool is_bgr = true;

virtual ~img_t() = default;
};
Expand Down
1 change: 1 addition & 0 deletions src/platform/windows/display.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ using device_ctx_t = util::safe_ptr<ID3D11DeviceContext, Release<ID3D11
using adapter_t = util::safe_ptr<IDXGIAdapter1, Release<IDXGIAdapter1>>;
using output_t = util::safe_ptr<IDXGIOutput, Release<IDXGIOutput>>;
using output1_t = util::safe_ptr<IDXGIOutput1, Release<IDXGIOutput1>>;
using output5_t = util::safe_ptr<IDXGIOutput5, Release<IDXGIOutput5>>;
using dup_t = util::safe_ptr<IDXGIOutputDuplication, Release<IDXGIOutputDuplication>>;
using texture2d_t = util::safe_ptr<ID3D11Texture2D, Release<ID3D11Texture2D>>;
using texture1d_t = util::safe_ptr<ID3D11Texture1D, Release<ID3D11Texture1D>>;
Expand Down
46 changes: 35 additions & 11 deletions src/platform/windows/display_base.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <cmath>
#include <codecvt>
#include <initguid.h>

#include "display.h"
#include "misc.h"
Expand Down Expand Up @@ -79,22 +80,21 @@ duplication_t::~duplication_t() {
}

int display_base_t::init(int framerate, const std::string &display_name) {
/* Uncomment when use of IDXGIOutput5 is implemented
std::once_flag windows_cpp_once_flag;

std::call_once(windows_cpp_once_flag, []() {
DECLARE_HANDLE(DPI_AWARENESS_CONTEXT);
const auto DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2 = ((DPI_AWARENESS_CONTEXT)-4);

typedef BOOL (*User32_SetProcessDpiAwarenessContext)(DPI_AWARENESS_CONTEXT value);

auto user32 = LoadLibraryA("user32.dll");
auto f = (User32_SetProcessDpiAwarenessContext)GetProcAddress(user32, "SetProcessDpiAwarenessContext");
auto f = (User32_SetProcessDpiAwarenessContext)GetProcAddress(user32, "SetProcessDpiAwarenessContext");
if(f) {
f(DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2);
}

FreeLibrary(user32);
});
*/

// Ensure we can duplicate the current display
syncThreadDesktop();
Expand Down Expand Up @@ -291,27 +291,51 @@ int display_base_t::init(int framerate, const std::string &display_name) {
}

//FIXME: Duplicate output on RX580 in combination with DOOM (2016) --> BSOD
Copy link
Contributor Author

@psyke83 psyke83 Dec 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this comment from loki. My host GPU is a newer AMD GPU (RX 6600), but perhaps this is related to my problem; DOOM 2016 runs in Vulkan, so it could be using DXGI_FORMAT_R8G8B8A8_UNORM just like DX11 titles in true fullscreen exclusive mode.

I've been testing a game (Dying Light) with "disable fullscreen optimizations" checked. Before this PR, it wasn't possible to run in true exclusive fullscreen; now it works. However, if you toggle between resolutions in the settings menu, eventually it will either cause a driver timeout or complete BSOD. I was able to reproduce the crash on both libx264 and amdvce, and it looks like #605 doesn't solve my issue (but I realize that it wasn't the focus).

//TODO: Use IDXGIOutput5 for improved performance
{
const DXGI_FORMAT DesktopFormats[] = {
DXGI_FORMAT_R8G8B8A8_UNORM,
DXGI_FORMAT_B8G8R8A8_UNORM,
DXGI_FORMAT_R16G16B16A16_FLOAT,
DXGI_FORMAT_R10G10B10A2_UNORM,
Copy link
Collaborator

@cgutman cgutman Dec 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can fix #376 here by specifying the appropriate formats (DXGI_FORMAT_B8G8R8A8_UNORM for SDR and DXGI_FORMAT_R10G10B10A2_UNORM for HDR, when that is implemented).

As I understand the docs, DWM should convert the format to match what we've asked for (for example, doing HDR -> SDR conversion if requested).

Copy link
Contributor Author

@psyke83 psyke83 Dec 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately I don't have any HDR capable monitors to test HDR or HDR -> SDR conversion; I even tried a virtual monitor driver for Windows, but it refuses to enable HDR in the virtual monitor.

Specifying the expanded list of formats doesn't seem to automatically convert formats, but it has allowed me to fix some other issues (added as separate commits).

Since my hands are tied in diagnosing/fixing HDR support due to lack of hardware, I would be more than happy to let you take over this PR (doesn't matter if you recommend this to be merged and then you continue, you cherry-pick some changes, or start over entirely, I don't mind).

In its present form, this PR will at minimum fix: #553

Copy link
Collaborator

@cgutman cgutman Dec 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifying the expanded list of formats doesn't seem to automatically convert formats, but it has allowed me to fix some other issues (added as separate commits).

It actually does, but we just made an mistake in our interpretation of the MSDN docs on it. DXGI_OUTDUPL_DESC will always give us the actual desktop mode, not the format that our frames are being converted to. See #609

For IDXGIOutput5, I think we have to actually acquire a frame and call GetDesc() on it to determine the actual format that was chosen from our supported list (or maybe just provide a list with only one SDR or HDR format to ensure we'll get that one).

};
const unsigned DesktopFormatsCounts = sizeof(DesktopFormats) / sizeof(DesktopFormats[0]);
dxgi::output1_t output1 {};
status = output->QueryInterface(IID_IDXGIOutput1, (void **)&output1);
dxgi::output5_t output5 {};

status = output->QueryInterface(IID_IDXGIOutput5, (void **)&output5);
if(FAILED(status)) {
BOOST_LOG(error) << "Failed to query IDXGIOutput1 from the output"sv;
return -1;
BOOST_LOG(warning) << "Failed to query IDXGIOutput5 from the output"sv;

status = output->QueryInterface(IID_IDXGIOutput1, (void **)&output1);
if(FAILED(status)) {
BOOST_LOG(error) << "Failed to query IDXGIOutput1 from the output"sv;
return -1;
}
}

// We try this twice, in case we still get an error on reinitialization
for(int x = 0; x < 2; ++x) {
status = output1->DuplicateOutput((IUnknown *)device.get(), &dup.dup);
status = output5->DuplicateOutput1((IUnknown *)device.get(), 0, DesktopFormatsCounts, DesktopFormats, &dup.dup);
if(SUCCEEDED(status)) {
break;
}
std::this_thread::sleep_for(200ms);
}

if(FAILED(status)) {
BOOST_LOG(error) << "DuplicateOutput Failed [0x"sv << util::hex(status).to_string_view() << ']';
return -1;
BOOST_LOG(warning) << "DuplicateOutput1 Failed [0x"sv << util::hex(status).to_string_view() << ']';
for(int x = 0; x < 2; ++x) {
status = output1->DuplicateOutput((IUnknown *)device.get(), &dup.dup);
if(SUCCEEDED(status)) {
break;
}
std::this_thread::sleep_for(200ms);
}

if(FAILED(status)) {
BOOST_LOG(error) << "DuplicateOutput Failed [0x"sv << util::hex(status).to_string_view() << ']';
return -1;
}
}
}

Expand Down
1 change: 1 addition & 0 deletions src/platform/windows/display_ram.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ std::shared_ptr<platf::img_t> display_ram_t::alloc_img() {
img->width = width;
img->height = height;
img->data = new std::uint8_t[img->row_pitch * height];
img->is_bgr = (format == DXGI_FORMAT_B8G8R8A8_UNORM ? true : false);

return img;
}
Expand Down
5 changes: 3 additions & 2 deletions src/platform/windows/display_vram.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ class hwdevice_t : public platf::hwdevice_t {

this->device_ctx_p = device_ctx_p;

DXGI_FORMAT src_format = format;
Copy link
Contributor Author

@psyke83 psyke83 Dec 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cgutman

Related to #609: I actually made an additional mistake here: I assumed that format was initialized with the correct value passed from display_base_t::init, but in fact it is just an uninitialized variable that resolves to DXGI_FORMAT_UNKNOWN, which coincidentally seems to handle both DXGI_FORMAT_R8G8B8A8_UNORM and DXGI_FORMAT_B8G8R8A8_UNORM. This is obviously not ideal and is supposed to select the proper format based on a similar img->is_bgr check as done here:

Sunshine/src/platform/windows/display_ram.cpp

Line 288 in 62f98b8

img->is_bgr = (format == DXGI_FORMAT_B8G8R8A8_UNORM ? true : false);

However, looking at #609, instead of fixing this, it looks like I should just remove both the vram and ram commits entirely in place of #609? I will not be able to edit the PR and verify in depth for a few hours, but I will be happy to back out those commits ASAP if you can verify my suspicion.

Copy link
Collaborator

@cgutman cgutman Dec 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think DXGI_FORMAT_UNKNOWN tells D3D11 to use the actual format of the texture (which is what we want).

The base IDXGIOutput5 part needs more work to handle the proper surface format selection. I think we want to avoid the HDR formats if we're not in HDR mode (and the SDR formats in HDR mode). I'll take a stab at it after the holidays.

format = (pix_fmt == pix_fmt_e::nv12 ? DXGI_FORMAT_NV12 : DXGI_FORMAT_P010);
status = device_p->CreateVertexShader(scene_vs_hlsl->GetBufferPointer(), scene_vs_hlsl->GetBufferSize(), nullptr, &scene_vs);
if(status) {
Expand Down Expand Up @@ -490,7 +491,7 @@ class hwdevice_t : public platf::hwdevice_t {
}

D3D11_SHADER_RESOURCE_VIEW_DESC desc {
DXGI_FORMAT_B8G8R8A8_UNORM,
src_format,
D3D11_SRV_DIMENSION_TEXTURE2D
};
desc.Texture2D.MipLevels = 1;
Expand Down Expand Up @@ -885,4 +886,4 @@ int init() {

return 0;
}
} // namespace platf::dxgi
} // namespace platf::dxgi
6 changes: 5 additions & 1 deletion src/video.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,14 @@ class swdevice_t : public platf::hwdevice_t {
data[1] = sw_frame->data[1] + offsetUV * 2;
data[2] = nullptr;
}
else {
else if (img.is_bgr) { // BGR0
data[1] = sw_frame->data[1] + offsetUV;
data[2] = sw_frame->data[2] + offsetUV;
data[3] = nullptr;
} else { // RGB0
data[1] = sw_frame->data[2] + offsetUV;
data[2] = sw_frame->data[1] + offsetUV;
data[3] = nullptr;
}

int ret = sws_scale(sws.get(), (std::uint8_t *const *)&img.data, linesizes, 0, img.height, data, sw_frame->linesize);
Expand Down