microsoft / DirectX-Graphics-Samples

This repo contains the DirectX Graphics samples that demonstrate how to build graphics intensive applications on Windows.
MIT License
5.89k stars 2k forks source link

HDR Sample treating Min/MaxMasteringLuminance as the same range? #796

Open Joshua-Ashton opened 1 year ago

Joshua-Ashton commented 1 year ago

Hello!

I was looking at the HDR sample here: https://github.com/microsoft/DirectX-Graphics-Samples/blob/master/Samples/Desktop/D3D12HDR/src/D3D12HDR.cpp#L994

And it seems to treat MaxMasteringLuminance and MinMasteringLuminance as the same range/scale.

Whereas the docs say:

MaxMasteringLuminance

The maximum number of nits of the display used to master the content. Values are in whole nits.

MinMasteringLuminance

The minimum number of nits of the display used to master the content. Values are 1/10000th of a nit (0.0001 nit).

Is the sample wrong, are the docs wrong, or am I just misunderstanding?

Thanks for your time!

Joshua-Ashton commented 1 year ago

The D3D12 docs state:

This structure represents the definition of HDR10 metadata used with HEVC, not HDR10 metadata for ST.2086. These are closely related but defined differently.

HEVC spec states:

max_display_mastering_luminance and min_display_mastering_luminance specify the nominal maximum and
minimum display luminance, respectively, of the mastering display in units of 0.0001 candelas per square metre.
min_display_mastering_luminance shall be less than max_display_mastering_luminance.

Which seems to indicate the docs may be wrong here about the range of `MaxMasteringLuminance' and the sample was correct?

sebmerry commented 1 year ago

Thank you for the report! We'll take a look at this and get back to you.

NickoLopez commented 1 year ago

Hi, I was looking at this sample as well and reached the same conclusion than @Joshua-Ashton . It seems MaxMasteringLuminance in DXGI_HDR_METADATA_HDR10 is set to a value 10000x larger than what it should be (it's expressed in Nits everywhere else in the sample): https://github.com/microsoft/DirectX-Graphics-Samples/blob/master/Samples/Desktop/D3D12HDR/src/D3D12HDR.cpp#L994

It doesn't match https://learn.microsoft.com/en-us/windows/win32/api/dxgi1_5/ns-dxgi1_5-dxgi_hdr_metadata_hdr10

@Joshua-Ashton I think MS doc is correct though, and it matches NVApi definitions. The issue only seems to be that the Sample sets MaxMasteringLuminance in 1/10000th of a nit where it should just be in Nits:

NvU16 max_display_mastering_luminance; //!< Maximum display mastering luminance ([0x0000-0xFFFF] = [0.0 - 65535.0] cd/m^2, in units of 1 cd/m^2) NvU16 min_display_mastering_luminance; //!< Minimum display mastering luminance ([0x0000-0xFFFF] = [0.0 - 6.55350] cd/m^2, in units of 0.0001 cd/m^2)

walbourn commented 1 year ago

You can submit a PR for the sample to resolve this.