SDL icon indicating copy to clipboard operation
SDL copied to clipboard

Support more pixel formats with the Vulkan renderer

Open ccawley2011 opened this issue 1 year ago • 8 comments

The new formats more closely match how the specifications describe how packed vs. byte formats are stored and should work in theory on both big and little endian. Support for SDL_PIXELFORMAT_XRGB8888 was removed because it wasn't clear how the existing code handled missing alpha components.

I'm not sure about SDL_PIXELFORMAT_XBGR2101010, but I don't know how to test it.

ccawley2011 avatar May 27 '24 18:05 ccawley2011

@danginsburg, can you check this?

SDL_PIXELFORMAT_XRGB8888 should be handled like ARGB8888 but blending operations treat the alpha channel as opaque.

slouken avatar May 27 '24 19:05 slouken

As an aside, it would be nice to support some of the 16-bit packed formats like VK_FORMAT_R4G4B4A4_UNORM_PACK16, but since none of them have SRGB variants I'm not sure how to correctly approach this.

ccawley2011 avatar May 27 '24 20:05 ccawley2011

As an aside, it would be nice to support some of the 16-bit packed formats like VK_FORMAT_R4G4B4A4_UNORM_PACK16, but since none of them have SRGB variants I'm not sure how to correctly approach this.

In practice I haven't seen them being used for rendering. Usually textures are converted to RGBA or a compressed texture format.

slouken avatar May 27 '24 20:05 slouken

I don't really have the context to understand what the impact of this change would be. What is removing SDL_PIXELFORMAT_ARGB8888 and SDL_PIXELFORMAT_XRGB8888 from SDL_AddSupportedTextureFormat and replacing them with SDL_PIXELFORMAT_RGBA32 and SDL_PIXELFORMAT_BGRA32 going to do?

Also, I wonder if you need to do optimalTilingSupport checks for some of these since for example R8G8B8_SRGB/UNORM have pretty limited support in the wild (see vulkan.gpuinfo.org).

danginsburg avatar May 27 '24 21:05 danginsburg

I don't really have the context to understand what the impact of this change would be. What is removing SDL_PIXELFORMAT_ARGB8888 and SDL_PIXELFORMAT_XRGB8888 from SDL_AddSupportedTextureFormat and replacing them with SDL_PIXELFORMAT_RGBA32 and SDL_PIXELFORMAT_BGRA32 going to do?

The Vulkan specs state that VK_FORMAT_B8G8R8A8_UNORM specifies a four-component, 32-bit unsigned normalized format that has an 8-bit B component in byte 0, an 8-bit G component in byte 1, an 8-bit R component in byte 2, and an 8-bit A component in byte 3., however SDL_PIXELFORMAT_ARGB8888 specifies a format with an 8-bit A component in bits 24..31, an 8-bit R component in bits 16..23, an 8-bit G component in bits 8..15, and an 8-bit B component in bits 0..7. On little endian machines these both mean the same thing, but on big endian machines these have the components in the opposite order to each other. SDL_PIXELFORMAT_BGRA32 is an alias to handle this difference.

Also, I wonder if you need to do optimalTilingSupport checks for some of these since for example R8G8B8_SRGB/UNORM have pretty limited support in the wild (see vulkan.gpuinfo.org).

That probably depends on if it's still faster than converting in software. It's listed after the R8G8B8A8 formats so it shouldn't be picked as a default for the most part, but it may still be nice to extend the SDL render API to report that certain supported formats are slower.

In practice I haven't seen them being used for rendering. Usually textures are converted to RGBA or a compressed texture format.

I'm mainly interested in the 16-bit pixel formats for dynamic textures in emulators, game reimplementations and video decoders, but it should be useful for static images on lower-end devices as well (such as .bmp and .tga).

ccawley2011 avatar May 27 '24 21:05 ccawley2011

By the way, we're not trying to stuff in all the formats that can be supported, we're trying to include the common formats that are fast for current drivers. If a format is allowed according to the spec, but mobile GPU vendors implement slow paths for access, for example, we wouldn't list those formats. The 3-byte RGB formats come to mind, and the 16-bit formats might be in a similar situation.

If a format is supported but slow, it's actually better to leave it off, since SDL will convert textures to fast formats during texture creation and upload.

slouken avatar May 28 '24 14:05 slouken

That probably depends on if it's still faster than converting in software. It's listed after the R8G8B8A8 formats so it shouldn't be picked as a default for the most part, but it may still be nice to extend the SDL render API to report that certain supported formats are slower.

It's not a performance thing, for non-mandatory formats you need to check vkGetPhysicalDeviceFormatProperties to see if it's supported for the uses. See https://registry.khronos.org/vulkan/specs/1.3/html/vkspec.html#formats-mandatory-features-32bit for the list of mandatory formats. I think the previously exposed formats were mandatory so not checked, if not that's a bug.

danginsburg avatar May 28 '24 15:05 danginsburg

@ccawley2011, can you update this PR to check vkGetPhysicalDeviceFormatProperties for the appropriate formats?

slouken avatar Aug 06 '24 17:08 slouken