color_space_helper: Allow 8- and 10-bit fmts for the sRGB color space #175
Conversation
|
Can one of the admins verify this patch? |
Test summary for commit 8dbca14CTS tests (Failed: 267/207843)
Rhel 8.9, Gfx10Ubuntu navi3x, SrdcvkUbuntu navi2x, Srdcvk |
|
That commit will make “R16G16B16A16_UNORM and R16G16B16A16_SFLOAT formats become available for the sRGB color space”, but currently Linux display only supports 32bits formats. Need to define a “new Fmt_8bpc_10bpc” for format “VK_FORMAT_B8G8R8A8_UNORM”, “VK_FORMAT_B8G8R8A8_SRGB” and “VK_FORMAT_A2R10G10B10_UNORM_PACK32”. Please don’t use “Fmt_All” directly. |
|
@JoeWangAMD these would be the plane formats supported by EDIT: For future reference some of this info might come from https://drmdb.emersion.fr/snapshots/e0ffff941ce7. Let's introduce the suggested |
MarijnS95
force-pushed
the
10-bit-srgb
branch
from
8dbca14
to
df3a754
Compare
MarijnS95
changed the title
|
I don't have a concrete list currently, but at least we have proved the following formats are supported. Suggest adding the format restrictions for the 10bits. Thanks. Sample code below.
|
|
Hi @JoeWangAMD, while related to color spaces that should not be too important for this PR (the I have however pushed the split in a separate commit, and use it in non-sRGB 10-bit formats. |
icd/api/include/color_space_helper.h
Outdated
| Fmt_8bpc = Fmt_8bpc_srgb | Fmt_8bpc_unorm, | ||
| Fmt_10bpc = Fmt_10bpc_srgb | Fmt_10bpc_unorm, | ||
| Fmt_16bpc = Fmt_16bpc_unorm | Fmt_16bpc_sfloat, | ||
| Fmt_KnownHDR = Fmt_10bpc | Fmt_11bpc | Fmt_12bpc | Fmt_16bpc, |
There was a problem hiding this comment.
Note that 10-bit sRGB, while technically invalid for TfHlg (where this KnownHDR format is used), is still HDR. Should we change TfHlg to use a new Fmt, without the srgb variant?
There was a problem hiding this comment.
Thanks for your information @MarijnS95 . Could you help change "Fmt_KnownHDR = Fmt_10bpc_unorm| Fmt_11bpc | Fmt_12bpc | Fmt_16bpc" in this commit? Thanks.
There was a problem hiding this comment.
@JoeWangAMD sounds good! We can also decide to rename it to Fmt_KnownHLG, is that more clear or would you prefer to stick with KnownHDR?
There was a problem hiding this comment.
@JoeWangAMD I've updated this and also added Fmt_10bpc_srgb to Fmt_All to complement this change.
I haven't renamed it to HLG as this flag is also used in the IsFmtHdr() function.
Most texture formats should be presentable on the default sRGB color space, in particular 10-bit formats for higher bit depth and detail than the typical 8-bit. A couple notes: - This currently only affects `VK_KHR_display` as there is a `needsWorkaround` case in `PhysicalDevice::GetSurfaceFormats()` that returns exclusively RGBA8 and BGRA8 in the windowed/compositor case; - Before this patch, `A2B10G10R10` wasn't in the list returned by `vkGetPhysicalDeviceSurfaceFormatsKHR()` but worked anyway on `VK_KHR_display` _without_ validation errors; - Using the `A2B10G10R10` format inside a 10-bit compositor works anyway, albeit with validation layer errors; - RADV also allows 10-bit sRGB swapchains, but only in a compositor, not in `VK_KHR_display`; - Windows also does not report support for this 10-bit sRGB format combination, but it works anyway (also with validation layer errors); - This was previously `Fmt_All` already before commit 02e867e ("Update xgl from commit 2aeb0b25") but it is assumed that sub-commit "Fix ColorspaceHelper's lookup table so it only reports formats that are legal" changed this behaviour. At least 10-bit sRGB should be retained? (When saying "it works", I mean that it works as intended, i.e. any 8-bit banding previously observable is gone when using a 10-bit swapchain)
MarijnS95
force-pushed
the
10-bit-srgb
branch
from
f0bd5a0
to
03b1852
Compare
Test summary for commit 03b1852CTS tests (Failed: 0/142491)
Ubuntu navi3x, SrdcvkUbuntu navi2x, Srdcvk |
Most texture formats should be presentable on the default sRGB color space, in particular 10-bit formats for higher bit depth and detail than the typical 8-bit.
A couple notes:
VK_KHR_displayas there is aneedsWorkaroundcase inPhysicalDevice::GetSurfaceFormats()that returns exclusively RGBA8 and BGRA8 in the windowed/compositor case;A2B10G10R10wasn't in the list returned byvkGetPhysicalDeviceSurfaceFormatsKHR()but worked anyway onVK_KHR_displaywithout validation errors;A2B10G10R10format inside a 10-bit compositor works anyway, albeit with validation layer errors;VK_KHR_display;By usingFmt_All, theR16G16B16A16_UNORMandR16G16B16A16_SFLOATformats become available for the sRGB color space, but creating a swapchain with them leaves a black screen. If this is truly unsupported, perhaps there should be a newFmt_8bpc_10bpc?Fmt_Allalready before commit 02e867e ("Update xgl from commit 2aeb0b25") but it is assumed that sub-commit "Fix ColorspaceHelper's lookup table so it only reports formats that are legal" changed this behaviour. At least 10-bit sRGB should be retained?(When saying "it works", I mean that it works as intended, i.e. any 8-bit banding previously observable is gone when using a 10-bit swapchain)