Use the correct pixel formats for OpenGL on big endian #11889
Conversation
| @@ -558,7 +558,8 @@ static bool GL_CreateTexture(SDL_Renderer *renderer, SDL_Texture *texture, SDL_P | |||
| renderdata->glTexParameteri(textype, GL_TEXTURE_STORAGE_HINT_APPLE, | |||
| GL_STORAGE_CACHED_APPLE); | |||
| } | |||
| if (texture->access == SDL_TEXTUREACCESS_STREAMING && texture->format == SDL_PIXELFORMAT_ARGB8888 && (texture->w % 8) == 0) { | |||
| if (!SDL_ISPIXELFORMAT_FOURCC(texture->format) && SDL_PIXELLAYOUT(texture->format) == SDL_PACKEDLAYOUT_8888 && | |||
| texture->access == SDL_TEXTUREACCESS_STREAMING && (texture->w % 8) == 0) { | |||
| renderdata->glPixelStorei(GL_UNPACK_CLIENT_STORAGE_APPLE, GL_TRUE); | |||
There was a problem hiding this comment.
I'm pretty sure the pixel format has to be SDL_PIXELFORMAT_ARGB8888 in order to use this extension.
There was a problem hiding this comment.
Is this requirement documented somewhere? And in a similar vein, does this also require GL_UNSIGNED_INT_8_8_8_8_REV to be used as the texture type?
There was a problem hiding this comment.
This is the documentation:
https://registry.khronos.org/OpenGL/extensions/APPLE/APPLE_client_storage.txt
But it doesn't mention any of these requirements, so maybe it's okay as-is?
There was a problem hiding this comment.
The official Apple documentation mentions this:
The best format and data type combinations to use for texture data are:
GL_BGRA, GL_UNSIGNED_INT_8_8_8_8_REV
GL_BGRA, GL_UNSIGNED_SHORT_1_5_5_5_REV)
GL_YCBCR_422_APPLE, GL_UNSIGNED_SHORT_8_8_REV_APPLE
The combination GL_RGBA and GL_UNSIGNED_BYTE needs to be swizzled by many cards when the data is loaded, so it's not recommended.
It's unclear if it's strictly necessary to use those formats in order to use GL_UNPACK_CLIENT_STORAGE_APPLE, although it would make sense if the benefit of using it is negated when swizzling is required.
There was a problem hiding this comment.
Makes sense. Do you want to update the PR to reflect this?
There was a problem hiding this comment.
The combination GL_RGBA and GL_UNSIGNED_BYTE needs to be swizzled by many cards when the data is loaded, so it's not recommended.
This was certainly true in the early 2000's when this extension was written. :)
There was a problem hiding this comment.
Makes sense. Do you want to update the PR to reflect this?
We're wrapping up the 3.2.0 release, @ccawley2011, so if you want to make updates to this PR, this is the time to do it (or tell us the updates aren't necessary and we'll merge as-is, if that's the right thing to do).
There was a problem hiding this comment.
I think this should be OK as-is for now. I'll open a separate issue regarding optimal formats on Mac OS X.
There was a problem hiding this comment.
I think we should drop this part of the PR, and create a separate one for this. I'm not convinced this is the right logic, and changing when the Apple extension is used is conceptually a different change than the rest of this PR.
| SDL_AddSupportedTextureFormat(renderer, SDL_PIXELFORMAT_XRGB8888); | ||
| SDL_AddSupportedTextureFormat(renderer, SDL_PIXELFORMAT_XBGR8888); | ||
| SDL_AddSupportedTextureFormat(renderer, SDL_PIXELFORMAT_RGBA32); | ||
| /* TODO: Check for required extensions? */ |
There was a problem hiding this comment.
Does something still need to be done here?
This also adjusts the priority of available pixel formats, since
SDL_PIXELFORMAT_RGBA32is available in all versions of OpenGL while the others were added later on.