Skip to content

Commit 5e6a1c7

Browse files
igl | vulkan | Add sRGB/UNORM VkFormat pair helpers
Reviewed By: corporateshark, ChristianK275 Differential Revision: D115048965 fbshipit-source-id: fa1ca1586ca9dcf92158fdb6df0ef3fa63ece3b1
1 parent 1dd20a2 commit 5e6a1c7

3 files changed

Lines changed: 216 additions & 0 deletions

File tree

src/igl/tests/vulkan/CommonTest.cpp

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
#include <cstdint>
1313
#include <cstring>
14+
#include <utility>
15+
#include <igl/TextureFormat.h>
1416
#include <igl/tests/util/device/TestDevice.h>
1517
#include <igl/vulkan/CommandBuffer.h>
1618
#include <igl/vulkan/Device.h>
@@ -198,6 +200,134 @@ TEST(CommonTest, IsTextureFormatBGRTest) {
198200
EXPECT_FALSE(igl::vulkan::isTextureFormatBGR(VK_FORMAT_R16G16B16A16_SFLOAT));
199201
}
200202

203+
// isSrgbFormat / srgbToUnorm / unormToSrgb ****************************************************
204+
205+
namespace {
206+
// Independent restatement of the IGL_FOR_EACH_SRGB_UNORM_FORMAT_PAIR list in Common.cpp, so a
207+
// typo in one of the macro's pairs shows up here rather than as wrong-gamma pixels.
208+
constexpr std::pair<VkFormat, VkFormat> kSrgbUnormPairs[] = {
209+
{VK_FORMAT_R8G8B8A8_SRGB, VK_FORMAT_R8G8B8A8_UNORM},
210+
{VK_FORMAT_B8G8R8A8_SRGB, VK_FORMAT_B8G8R8A8_UNORM},
211+
{VK_FORMAT_ASTC_4x4_SRGB_BLOCK, VK_FORMAT_ASTC_4x4_UNORM_BLOCK},
212+
{VK_FORMAT_ASTC_5x4_SRGB_BLOCK, VK_FORMAT_ASTC_5x4_UNORM_BLOCK},
213+
{VK_FORMAT_ASTC_5x5_SRGB_BLOCK, VK_FORMAT_ASTC_5x5_UNORM_BLOCK},
214+
{VK_FORMAT_ASTC_6x5_SRGB_BLOCK, VK_FORMAT_ASTC_6x5_UNORM_BLOCK},
215+
{VK_FORMAT_ASTC_6x6_SRGB_BLOCK, VK_FORMAT_ASTC_6x6_UNORM_BLOCK},
216+
{VK_FORMAT_ASTC_8x5_SRGB_BLOCK, VK_FORMAT_ASTC_8x5_UNORM_BLOCK},
217+
{VK_FORMAT_ASTC_8x6_SRGB_BLOCK, VK_FORMAT_ASTC_8x6_UNORM_BLOCK},
218+
{VK_FORMAT_ASTC_8x8_SRGB_BLOCK, VK_FORMAT_ASTC_8x8_UNORM_BLOCK},
219+
{VK_FORMAT_ASTC_10x5_SRGB_BLOCK, VK_FORMAT_ASTC_10x5_UNORM_BLOCK},
220+
{VK_FORMAT_ASTC_10x6_SRGB_BLOCK, VK_FORMAT_ASTC_10x6_UNORM_BLOCK},
221+
{VK_FORMAT_ASTC_10x8_SRGB_BLOCK, VK_FORMAT_ASTC_10x8_UNORM_BLOCK},
222+
{VK_FORMAT_ASTC_10x10_SRGB_BLOCK, VK_FORMAT_ASTC_10x10_UNORM_BLOCK},
223+
{VK_FORMAT_ASTC_12x10_SRGB_BLOCK, VK_FORMAT_ASTC_12x10_UNORM_BLOCK},
224+
{VK_FORMAT_ASTC_12x12_SRGB_BLOCK, VK_FORMAT_ASTC_12x12_UNORM_BLOCK},
225+
{VK_FORMAT_ETC2_R8G8B8_SRGB_BLOCK, VK_FORMAT_ETC2_R8G8B8_UNORM_BLOCK},
226+
{VK_FORMAT_ETC2_R8G8B8A1_SRGB_BLOCK, VK_FORMAT_ETC2_R8G8B8A1_UNORM_BLOCK},
227+
{VK_FORMAT_ETC2_R8G8B8A8_SRGB_BLOCK, VK_FORMAT_ETC2_R8G8B8A8_UNORM_BLOCK},
228+
{VK_FORMAT_BC7_SRGB_BLOCK, VK_FORMAT_BC7_UNORM_BLOCK},
229+
};
230+
} // namespace
231+
232+
TEST(CommonTest, SrgbUnormFormatPairsTest) {
233+
for (const auto& [srgb, unorm] : kSrgbUnormPairs) {
234+
EXPECT_TRUE(igl::vulkan::isSrgbFormat(srgb)) << "sRGB format " << srgb << " not recognized";
235+
EXPECT_FALSE(igl::vulkan::isSrgbFormat(unorm)) << "UNORM format " << unorm << " reported sRGB";
236+
237+
EXPECT_EQ(igl::vulkan::srgbToUnorm(srgb), unorm);
238+
EXPECT_EQ(igl::vulkan::unormToSrgb(unorm), srgb);
239+
240+
// The three helpers share one macro list, so a mismatch means the list is internally
241+
// inconsistent rather than merely incomplete.
242+
EXPECT_EQ(igl::vulkan::unormToSrgb(igl::vulkan::srgbToUnorm(srgb)), srgb);
243+
EXPECT_EQ(igl::vulkan::srgbToUnorm(igl::vulkan::unormToSrgb(unorm)), unorm);
244+
}
245+
}
246+
247+
// Formats outside the pair list must pass through untouched: callers use a returned-unchanged
248+
// format as the signal that there is no counterpart to swap to.
249+
TEST(CommonTest, SrgbHelpersPassThroughUnpairedFormats) {
250+
const VkFormat unpaired[] = {
251+
VK_FORMAT_UNDEFINED,
252+
VK_FORMAT_R16G16B16A16_SFLOAT,
253+
VK_FORMAT_R32G32B32A32_SFLOAT,
254+
VK_FORMAT_D32_SFLOAT,
255+
VK_FORMAT_S8_UINT,
256+
VK_FORMAT_G8_B8R8_2PLANE_420_UNORM,
257+
// Deliberately omitted from the list: real Vulkan sRGB formats that no IGL TextureFormat
258+
// maps to, so they never reach Texture::create(). Pinned here so the scope decision is
259+
// explicit rather than an accident.
260+
VK_FORMAT_R8_SRGB,
261+
VK_FORMAT_R8G8_SRGB,
262+
VK_FORMAT_B8G8R8_SRGB,
263+
VK_FORMAT_A8B8G8R8_SRGB_PACK32,
264+
VK_FORMAT_BC1_RGB_SRGB_BLOCK,
265+
VK_FORMAT_BC2_SRGB_BLOCK,
266+
VK_FORMAT_BC3_SRGB_BLOCK,
267+
};
268+
for (const VkFormat format : unpaired) {
269+
EXPECT_FALSE(igl::vulkan::isSrgbFormat(format)) << "format " << format;
270+
EXPECT_EQ(igl::vulkan::srgbToUnorm(format), format);
271+
EXPECT_EQ(igl::vulkan::unormToSrgb(format), format);
272+
}
273+
}
274+
275+
// The pair list claims to cover exactly the sRGB formats textureFormatToVkFormat() can produce.
276+
// Walk every IGL TextureFormat and hold it to that: adding a new sRGB TextureFormat without
277+
// extending the list fails here instead of silently skipping the UNORM-base sRGB handling.
278+
TEST(CommonTest, EveryIglSrgbTextureFormatIsPaired) {
279+
// textureFormatToVkFormat() and fromTextureFormat() are exhaustive switches, so casting past
280+
// the last enumerator is undefined. Extend this bound when TextureFormat grows.
281+
constexpr auto kLastTextureFormat = TextureFormat::R5G6B5_UNorm;
282+
283+
size_t numChecked = 0;
284+
for (uint8_t i = 0; i <= static_cast<uint8_t>(kLastTextureFormat); ++i) {
285+
const auto format = static_cast<TextureFormat>(i);
286+
if (!TextureFormatProperties::fromTextureFormat(format).isSRGB()) {
287+
continue;
288+
}
289+
const VkFormat vkFormat = igl::vulkan::textureFormatToVkFormat(format);
290+
if (vkFormat == VK_FORMAT_UNDEFINED) {
291+
continue; // Not representable on Vulkan (e.g. some compressed formats)
292+
}
293+
294+
++numChecked;
295+
EXPECT_TRUE(igl::vulkan::isSrgbFormat(vkFormat))
296+
<< "TextureFormat " << static_cast<int>(i) << " is sRGB but VkFormat " << vkFormat
297+
<< " is missing from IGL_FOR_EACH_SRGB_UNORM_FORMAT_PAIR";
298+
EXPECT_NE(igl::vulkan::srgbToUnorm(vkFormat), vkFormat)
299+
<< "TextureFormat " << static_cast<int>(i) << " has no UNORM counterpart";
300+
}
301+
302+
// One pair in the list is unreachable today: IGL's SRGB8_A8_EAC_ETC2 and its UNORM sibling both
303+
// map to VK_FORMAT_UNDEFINED in textureFormatToVkFormat(), so no TextureFormat produces
304+
// VK_FORMAT_ETC2_R8G8B8A8_SRGB_BLOCK. The pair is kept so sRGB handling is already correct if
305+
// that mapping is ever filled in. Asserting the exact difference keeps both sides honest: this
306+
// fails if the mapping is added (drop the entry below) or if a pair is added without a format.
307+
constexpr VkFormat kPairsUnreachableFromTextureFormat[] = {VK_FORMAT_ETC2_R8G8B8A8_SRGB_BLOCK};
308+
EXPECT_EQ(numChecked,
309+
IGL_ARRAY_NUM_ELEMENTS(kSrgbUnormPairs) -
310+
IGL_ARRAY_NUM_ELEMENTS(kPairsUnreachableFromTextureFormat));
311+
}
312+
313+
// igl::sRGBToLinear()/linearTosRGB() in TextureFormat.h answer the same question one level up, on
314+
// TextureFormat. They cannot back these helpers -- they cover only the uncompressed pairs and
315+
// assert rather than pass through on non-sRGB input, while Texture::create() calls these on every
316+
// format -- but where the two overlap they must agree, or the VkImage and the IGL-level format
317+
// would disagree about what the linear counterpart is.
318+
TEST(CommonTest, SrgbHelpersAgreeWithTextureFormatHelpers) {
319+
const TextureFormat srgbFormats[] = {TextureFormat::RGBA_SRGB, TextureFormat::BGRA_SRGB};
320+
for (const TextureFormat srgbFormat : srgbFormats) {
321+
const TextureFormat linearFormat = igl::sRGBToLinear(srgbFormat);
322+
EXPECT_EQ(igl::linearTosRGB(linearFormat), srgbFormat);
323+
324+
const VkFormat vkSrgb = igl::vulkan::textureFormatToVkFormat(srgbFormat);
325+
const VkFormat vkLinear = igl::vulkan::textureFormatToVkFormat(linearFormat);
326+
EXPECT_EQ(igl::vulkan::srgbToUnorm(vkSrgb), vkLinear);
327+
EXPECT_EQ(igl::vulkan::unormToSrgb(vkLinear), vkSrgb);
328+
}
329+
}
330+
201331
// hasDepth / hasStencil *********************************************************************
202332
TEST(CommonTest, HasDepthTest) {
203333
EXPECT_TRUE(igl::vulkan::hasDepth(VK_FORMAT_D16_UNORM));

src/igl/vulkan/Common.cpp

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,89 @@ bool isTextureFormatBGR(VkFormat format) {
329329
format == VK_FORMAT_A2B10G10R10_UNORM_PACK32;
330330
}
331331

332+
// Single source of truth for sRGB <-> UNORM VkFormat pairs. isSrgbFormat(), srgbToUnorm() and
333+
// unormToSrgb() are all generated from this X-macro list, so a new format pair only needs to be
334+
// added in one place to keep the three helpers provably in sync. Each is emitted as a switch over
335+
// the enum, letting the compiler generate a jump table instead of a linear scan on the
336+
// Texture-creation hot path.
337+
//
338+
// This lists the sRGB formats textureFormatToVkFormat() can produce (RGBA/BGRA 8-bit, ASTC, ETC2,
339+
// BC7), plus VK_FORMAT_ETC2_R8G8B8A8_SRGB_BLOCK: TextureFormat::SRGB8_A8_EAC_ETC2 and its UNORM
340+
// sibling currently map to VK_FORMAT_UNDEFINED, so that pair is unreachable today and is kept only
341+
// so the handling is already right if that mapping is filled in. Other Vulkan sRGB formats
342+
// (BC1/BC2/BC3, R8/R8G8, 24-bit RGB, A8B8G8R8_*_PACK32) are
343+
// omitted because no IGL TextureFormat maps to them, so they never reach Texture::create(). A
344+
// format not listed here is treated as non-sRGB: its VkImage is created directly with the sRGB
345+
// VkFormat, without the UNORM-image + sRGB-view / linear-storage-view handling. Add the pair here
346+
// when introducing a new sRGB TextureFormat.
347+
//
348+
// igl::sRGBToLinear()/linearTosRGB() (TextureFormat.h) express the same pairing one level up, on
349+
// TextureFormat. They are not reusable here: they cover only the uncompressed pairs, and they
350+
// assert on a non-sRGB argument, whereas Texture::create() queries every format and needs an
351+
// identity result for the ones with no counterpart. CommonTest pins the two against each other
352+
// wherever they overlap.
353+
#define IGL_FOR_EACH_SRGB_UNORM_FORMAT_PAIR(FN) \
354+
FN(VK_FORMAT_R8G8B8A8_SRGB, VK_FORMAT_R8G8B8A8_UNORM) \
355+
FN(VK_FORMAT_B8G8R8A8_SRGB, VK_FORMAT_B8G8R8A8_UNORM) \
356+
FN(VK_FORMAT_ASTC_4x4_SRGB_BLOCK, VK_FORMAT_ASTC_4x4_UNORM_BLOCK) \
357+
FN(VK_FORMAT_ASTC_5x4_SRGB_BLOCK, VK_FORMAT_ASTC_5x4_UNORM_BLOCK) \
358+
FN(VK_FORMAT_ASTC_5x5_SRGB_BLOCK, VK_FORMAT_ASTC_5x5_UNORM_BLOCK) \
359+
FN(VK_FORMAT_ASTC_6x5_SRGB_BLOCK, VK_FORMAT_ASTC_6x5_UNORM_BLOCK) \
360+
FN(VK_FORMAT_ASTC_6x6_SRGB_BLOCK, VK_FORMAT_ASTC_6x6_UNORM_BLOCK) \
361+
FN(VK_FORMAT_ASTC_8x5_SRGB_BLOCK, VK_FORMAT_ASTC_8x5_UNORM_BLOCK) \
362+
FN(VK_FORMAT_ASTC_8x6_SRGB_BLOCK, VK_FORMAT_ASTC_8x6_UNORM_BLOCK) \
363+
FN(VK_FORMAT_ASTC_8x8_SRGB_BLOCK, VK_FORMAT_ASTC_8x8_UNORM_BLOCK) \
364+
FN(VK_FORMAT_ASTC_10x5_SRGB_BLOCK, VK_FORMAT_ASTC_10x5_UNORM_BLOCK) \
365+
FN(VK_FORMAT_ASTC_10x6_SRGB_BLOCK, VK_FORMAT_ASTC_10x6_UNORM_BLOCK) \
366+
FN(VK_FORMAT_ASTC_10x8_SRGB_BLOCK, VK_FORMAT_ASTC_10x8_UNORM_BLOCK) \
367+
FN(VK_FORMAT_ASTC_10x10_SRGB_BLOCK, VK_FORMAT_ASTC_10x10_UNORM_BLOCK) \
368+
FN(VK_FORMAT_ASTC_12x10_SRGB_BLOCK, VK_FORMAT_ASTC_12x10_UNORM_BLOCK) \
369+
FN(VK_FORMAT_ASTC_12x12_SRGB_BLOCK, VK_FORMAT_ASTC_12x12_UNORM_BLOCK) \
370+
FN(VK_FORMAT_ETC2_R8G8B8_SRGB_BLOCK, VK_FORMAT_ETC2_R8G8B8_UNORM_BLOCK) \
371+
FN(VK_FORMAT_ETC2_R8G8B8A1_SRGB_BLOCK, VK_FORMAT_ETC2_R8G8B8A1_UNORM_BLOCK) \
372+
FN(VK_FORMAT_ETC2_R8G8B8A8_SRGB_BLOCK, VK_FORMAT_ETC2_R8G8B8A8_UNORM_BLOCK) \
373+
FN(VK_FORMAT_BC7_SRGB_BLOCK, VK_FORMAT_BC7_UNORM_BLOCK)
374+
375+
bool isSrgbFormat(VkFormat format) {
376+
// NOLINTNEXTLINE(clang-diagnostic-switch-enum)
377+
switch (format) {
378+
#define IGL_SRGB_CASE(srgb, unorm) case srgb:
379+
IGL_FOR_EACH_SRGB_UNORM_FORMAT_PAIR(IGL_SRGB_CASE)
380+
#undef IGL_SRGB_CASE
381+
return true;
382+
default:
383+
return false;
384+
}
385+
}
386+
387+
VkFormat srgbToUnorm(VkFormat format) {
388+
// NOLINTNEXTLINE(clang-diagnostic-switch-enum)
389+
switch (format) {
390+
#define IGL_SRGB_CASE(srgb, unorm) \
391+
case srgb: \
392+
return unorm;
393+
IGL_FOR_EACH_SRGB_UNORM_FORMAT_PAIR(IGL_SRGB_CASE)
394+
#undef IGL_SRGB_CASE
395+
default:
396+
return format;
397+
}
398+
}
399+
400+
VkFormat unormToSrgb(VkFormat format) {
401+
// NOLINTNEXTLINE(clang-diagnostic-switch-enum)
402+
switch (format) {
403+
#define IGL_SRGB_CASE(srgb, unorm) \
404+
case unorm: \
405+
return srgb;
406+
IGL_FOR_EACH_SRGB_UNORM_FORMAT_PAIR(IGL_SRGB_CASE)
407+
#undef IGL_SRGB_CASE
408+
default:
409+
return format;
410+
}
411+
}
412+
413+
#undef IGL_FOR_EACH_SRGB_UNORM_FORMAT_PAIR
414+
332415
TextureFormat vkFormatToTextureFormat(VkFormat format) {
333416
return util::vkTextureFormatToTextureFormat(static_cast<int32_t>(format));
334417
}

src/igl/vulkan/Common.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,9 @@ TextureFormat vkFormatToTextureFormat(VkFormat format);
185185
VkFormat invertRedAndBlue(VkFormat format);
186186
bool isTextureFormatRGB(VkFormat format);
187187
bool isTextureFormatBGR(VkFormat format);
188+
bool isSrgbFormat(VkFormat format);
189+
VkFormat srgbToUnorm(VkFormat format);
190+
VkFormat unormToSrgb(VkFormat format);
188191
bool hasDepth(VkFormat format);
189192
bool hasStencil(VkFormat format);
190193
uint32_t getNumImagePlanes(VkFormat format);

0 commit comments

Comments
 (0)