sdl-gpu icon indicating copy to clipboard operation
sdl-gpu copied to clipboard

GPU_FormatEnum missing some formats

Open ephemer opened this issue 8 years ago • 6 comments

Hi! I'm looking to decode video into a native pixel format and display it on a GPU_image. The image appears but with the wrong colours, which makes me think the byte order is something other than RGBA (but not YUV)

Is there a reason other possible formats are absent from GPU_ImageFormatEnum? Maybe cross-platform compatibility? And would you accept a pull request with more formats if changing the image format solves our issue?

ephemer avatar Jul 13 '17 15:07 ephemer

Ok this seems to come down to the Mac rendering the video in BGRA and the texture being RGBA. If I manually swap the R and B bytes in a loop over the pixel data the video renders perfectly.

I saw that the BGRA GL feature appears disabled on my Mac, which surprises me given that appears to be its native render format (AVPlayer won't render the video in RGBA at all). Even if the feature was enabled, I can't see how to make a texture in that format either way?

I'll do some more research about whether BGRA is available on the Mac. It may be an error on my part, or it could point to an issue in the feature detection.

By the way, if I don't specify any pixel format at all, AVPlayer will return a 16bit format of some kind: I suspect it'd match GL_RGB SHORT_5_6_5, but I can't see how to conditionally set my texture to that format. I imagine this would be more efficient than unnecessarily using a 32-bit format, so my preference long term would be to get that working.

As I said originally, I'm happy to dig around and add some more formats, but since I'm pretty new to this I'd like to hear your opinion on whether or not that's fundamentally a good idea.

ephemer avatar Jul 13 '17 19:07 ephemer

Yep, I'm all for adding more texture formats. It may take some reorganization to make it more clear what a format needs to be functional.

There are probably a few missing pieces to BGRA support. Right off, I know these places need to be edited for new format support and BGRA might not even be handled: gpu_copy_image_pixels_only CreateUninitializedImage CreateImageUsingTexture

Also, CopyImageFromSurface() and similar could use an additional setting/hint to specify what the default format should be for textures that SDL_gpu needs to create itself.

The feature detection could be another problem. For BGRA, the renderer-specific header (e.g. SDL_gpu_OpenGL_3.h) tries to #define GL_BGRA, which is used in renderer_GL_common.inl to enable the extra code for BGRA. The GL_EXT_bgra extension will be checked at init.

Can you post your GL extensions list from that Mac? Can you tell if GL_BGRA is getting defined?

grimfang4 avatar Jul 17 '17 13:07 grimfang4

Is there an internal function to list the extensions or do I need to use something like this: http://www.geeks3d.com/20100722/tips-how-to-get-the-list-of-the-opengl-extensions-with-a-core-profile/ ?

I did log the result of isExtensionSupported("GL_EXT_bgra") specifically in GPU_init and got a 0 return value. That said, there are a whole bunch of google hits for people confused that they have to use BGRA on mac and not on other platforms, this is just one of many: https://discourse.libsdl.org/t/1-2-windows-linux-gl-rgba-osx-gl-bgra/17621

Also it seems like the extension existed once upon a time at least: https://www.opengl.org/discussion_boards/showthread.php/170475-Mac-OS-X-10-6-3-New-Extensions


edit: Metal's default pixel format is BGRA on both mac and iOS, so it seems unlikely that it wouldn't be supported! https://developer.apple.com/documentation/quartzcore/cametallayer/1478155-pixelformat

ephemer avatar Jul 17 '17 19:07 ephemer

That geeks3d link should work for enumerating the extensions. Maybe Mac isn't listing it, because they want to assume it as core.

grimfang4 avatar Jul 17 '17 20:07 grimfang4

In case you were wondering, I have left this for now. I'm pretty sure your hunch is right about Mac considering it core, but I find the performance of swapping the R&B bits around acceptable for now. Feel free to close this, although it may be useful for others to see that there are in fact formats missing.

ephemer avatar Aug 01 '17 18:08 ephemer

Yeah, let's leave it open. I'll see if I get some time to work on finishing BGRA support soon.

grimfang4 avatar Aug 01 '17 19:08 grimfang4