From 5ffb7c65a532fc46fbfb0dad9448d85267fa93e3 Mon Sep 17 00:00:00 2001 From: baldurk Date: Thu, 26 Feb 2015 12:34:15 +0000 Subject: [PATCH] Hack around glCopyImageSubData being broken for compressed cubes on AMD * Also for the hack that involves readback to CPU and reupload, make sure there's no pixel pack or unpack buffer bound for the duration. --- renderdoc/driver/gl/gl_common.cpp | 71 +++++++++++++++++++++++++++++- renderdoc/driver/gl/gl_common.h | 1 + renderdoc/driver/gl/gl_manager.cpp | 44 +++++++++++++++--- 3 files changed, 109 insertions(+), 7 deletions(-) diff --git a/renderdoc/driver/gl/gl_common.cpp b/renderdoc/driver/gl/gl_common.cpp index 7ca733716..365c9694a 100644 --- a/renderdoc/driver/gl/gl_common.cpp +++ b/renderdoc/driver/gl/gl_common.cpp @@ -195,7 +195,7 @@ void DoVendorChecks(const GLHookSet &gl, GLWindowingData context) // AMD throws an error if we try to copy the mips that are smaller than 4x4, if(gl.glGetError && gl.glGenTextures && gl.glBindTexture && gl.glCopyImageSubData && - gl.glTexStorage2D && gl.glTexParameteri && gl.glDeleteTextures) + gl.glTexStorage2D && gl.glTexSubImage2D && gl.glTexParameteri && gl.glDeleteTextures) { GLuint texs[2]; gl.glGenTextures(2, texs); @@ -226,6 +226,75 @@ void DoVendorChecks(const GLHookSet &gl, GLWindowingData context) gl.glBindTexture(eGL_TEXTURE_2D, 0); gl.glDeleteTextures(2, texs); + + while(gl.glGetError()); + + ////////////////////////////////////////////////////////////////////////// + // Check copying cubemaps + + gl.glGenTextures(2, texs); + + const size_t dim = 32; + + char buf[dim*dim/2]; + + gl.glBindTexture(eGL_TEXTURE_CUBE_MAP, texs[0]); + gl.glTexStorage2D(eGL_TEXTURE_CUBE_MAP, 1, eGL_COMPRESSED_RGBA_S3TC_DXT1_EXT, dim, dim); + gl.glTexParameteri(eGL_TEXTURE_CUBE_MAP, eGL_TEXTURE_MAX_LEVEL, 1); + + for(int i=0; i < 6; i++) + { + memset(buf, 0xba + i, sizeof(buf)); + gl.glCompressedTexSubImage2D(GLenum(eGL_TEXTURE_CUBE_MAP_POSITIVE_X+i), 0, 0, 0, dim, dim, eGL_COMPRESSED_RGBA_S3TC_DXT1_EXT, dim*dim/2, buf); + } + + gl.glBindTexture(eGL_TEXTURE_CUBE_MAP, texs[1]); + gl.glTexStorage2D(eGL_TEXTURE_CUBE_MAP, 1, eGL_COMPRESSED_RGBA_S3TC_DXT1_EXT, dim, dim); + gl.glTexParameteri(eGL_TEXTURE_CUBE_MAP, eGL_TEXTURE_MAX_LEVEL, 1); + + gl.glCopyImageSubData(texs[0], eGL_TEXTURE_CUBE_MAP, 0, 0, 0, 0, texs[1], eGL_TEXTURE_CUBE_MAP, 0, 0, 0, 0, dim, dim, 6); + + char cmp[dim*dim/2]; + + gl.glBindTexture(eGL_TEXTURE_CUBE_MAP, texs[0]); + + for(int i=0; i < 6; i++) + { + memset(buf, 0xba + i, sizeof(buf)); + RDCEraseEl(cmp); + gl.glGetCompressedTexImage(GLenum(eGL_TEXTURE_CUBE_MAP_POSITIVE_X+i), 0, cmp); + + RDCCOMPILE_ASSERT(sizeof(buf) == sizeof(buf), "Buffers are not matching sizes"); + + if(memcmp(buf, cmp, sizeof(buf))) + { + RDCERR("glGetTexImage from the source texture returns incorrect data!"); + VendorCheck[VendorCheck_AMD_copy_compressed_cubemaps] = true; // to be safe, enable the hack + } + } + + gl.glBindTexture(eGL_TEXTURE_CUBE_MAP, texs[1]); + + for(int i=0; i < 6; i++) + { + memset(buf, 0xba + i, sizeof(buf)); + RDCEraseEl(cmp); + gl.glGetCompressedTexImage(GLenum(eGL_TEXTURE_CUBE_MAP_POSITIVE_X+i), 0, cmp); + + RDCCOMPILE_ASSERT(sizeof(buf) == sizeof(buf), "Buffers are not matching sizes"); + + if(memcmp(buf, cmp, sizeof(buf))) + { + RDCWARN("Using hack to avoid glCopyImageSubData on cubemap textures"); + VendorCheck[VendorCheck_AMD_copy_compressed_cubemaps] = true; + break; + } + } + + gl.glBindTexture(eGL_TEXTURE_CUBE_MAP, 0); + gl.glDeleteTextures(2, texs); + + while(gl.glGetError()); } if(gl.glGetError && gl.glGenProgramPipelines && gl.glDeleteProgramPipelines && gl.glGetProgramPipelineiv) diff --git a/renderdoc/driver/gl/gl_common.h b/renderdoc/driver/gl/gl_common.h index c44d24061..2ac62dfad 100644 --- a/renderdoc/driver/gl/gl_common.h +++ b/renderdoc/driver/gl/gl_common.h @@ -122,6 +122,7 @@ enum VendorCheckEnum VendorCheck_AMD_copy_compressed_tinymips, VendorCheck_AMD_pipeline_compute_query, VendorCheck_NV_ClearNamedFramebufferfiBugs, + VendorCheck_AMD_copy_compressed_cubemaps, VendorCheck_Count, }; extern bool VendorCheck[VendorCheck_Count]; diff --git a/renderdoc/driver/gl/gl_manager.cpp b/renderdoc/driver/gl/gl_manager.cpp index 90aab271a..1a2d27513 100644 --- a/renderdoc/driver/gl/gl_manager.cpp +++ b/renderdoc/driver/gl/gl_manager.cpp @@ -387,9 +387,17 @@ bool GLResourceManager::Prepare_InitialState(GLResource res) bool iscomp = IsCompressedFormat(details.internalFormat); + bool avoidCopySubImage = false; + if(iscomp && VendorCheck[VendorCheck_AMD_copy_compressed_tinymips]) + avoidCopySubImage = true; + if(iscomp && details.curType == eGL_TEXTURE_CUBE_MAP && VendorCheck[VendorCheck_AMD_copy_compressed_cubemaps]) + avoidCopySubImage = true; + GLint packParams[8]; GLint unpackParams[8]; - if(iscomp && VendorCheck[VendorCheck_AMD_copy_compressed_tinymips]) + GLuint pixelPackBuffer; + GLuint pixelUnpackBuffer; + if(avoidCopySubImage) { gl.glGetIntegerv(eGL_PACK_SWAP_BYTES, &packParams[0]); gl.glGetIntegerv(eGL_PACK_LSB_FIRST, &packParams[1]); @@ -426,6 +434,11 @@ bool GLResourceManager::Prepare_InitialState(GLResource res) gl.glPixelStorei(eGL_UNPACK_SKIP_ROWS, 0); gl.glPixelStorei(eGL_UNPACK_SKIP_IMAGES, 0); gl.glPixelStorei(eGL_UNPACK_ALIGNMENT, 1); + + gl.glGetIntegerv(eGL_PIXEL_PACK_BUFFER_BINDING, (GLint *)&pixelPackBuffer); + gl.glGetIntegerv(eGL_PIXEL_UNPACK_BUFFER_BINDING, (GLint *)&pixelUnpackBuffer); + gl.glBindBuffer(eGL_PIXEL_PACK_BUFFER, 0); + gl.glBindBuffer(eGL_PIXEL_UNPACK_BUFFER, 0); } // copy over mips @@ -445,7 +458,12 @@ bool GLResourceManager::Prepare_InitialState(GLResource res) // AMD throws an error copying mips that are smaller than the block size in one dimension, so do copy via // CPU instead (will be slow, potentially we could optimise this if there's a different GPU-side image copy // routine that works on these dimensions. Hopefully there'll only be a couple of such mips). - if(iscomp && VendorCheck[VendorCheck_AMD_copy_compressed_tinymips] && (w < 4 || h < 4)) + // + // AMD also has issues copying cubemaps + if( + (iscomp && VendorCheck[VendorCheck_AMD_copy_compressed_tinymips] && (w < 4 || h < 4)) || + (iscomp && VendorCheck[VendorCheck_AMD_copy_compressed_cubemaps] && details.curType == eGL_TEXTURE_CUBE_MAP) + ) { GLenum targets[] = { eGL_TEXTURE_CUBE_MAP_POSITIVE_X, @@ -506,7 +524,7 @@ bool GLResourceManager::Prepare_InitialState(GLResource res) } } - if(iscomp && VendorCheck[VendorCheck_AMD_copy_compressed_tinymips]) + if(avoidCopySubImage) { gl.glPixelStorei(eGL_PACK_SWAP_BYTES, packParams[0]); gl.glPixelStorei(eGL_PACK_LSB_FIRST, packParams[1]); @@ -525,6 +543,9 @@ bool GLResourceManager::Prepare_InitialState(GLResource res) gl.glPixelStorei(eGL_UNPACK_SKIP_ROWS, unpackParams[5]); gl.glPixelStorei(eGL_UNPACK_SKIP_IMAGES, unpackParams[6]); gl.glPixelStorei(eGL_UNPACK_ALIGNMENT, unpackParams[7]); + + gl.glBindBuffer(eGL_PIXEL_PACK_BUFFER, pixelPackBuffer); + gl.glBindBuffer(eGL_PIXEL_UNPACK_BUFFER, pixelUnpackBuffer); } gl.glTextureParameterivEXT(res.name, details.curType, eGL_TEXTURE_MAX_LEVEL, (GLint *)&state->maxLevel); @@ -1220,9 +1241,15 @@ void GLResourceManager::Apply_InitialState(GLResource live, InitialContentData i bool iscomp = IsCompressedFormat(details.internalFormat); + bool avoidCopySubImage = false; + if(iscomp && VendorCheck[VendorCheck_AMD_copy_compressed_tinymips]) + avoidCopySubImage = true; + if(iscomp && details.curType == eGL_TEXTURE_CUBE_MAP && VendorCheck[VendorCheck_AMD_copy_compressed_cubemaps]) + avoidCopySubImage = true; + GLint packParams[8]; GLint unpackParams[8]; - if(iscomp && VendorCheck[VendorCheck_AMD_copy_compressed_tinymips]) + if(avoidCopySubImage) { gl.glGetIntegerv(eGL_PACK_SWAP_BYTES, &packParams[0]); gl.glGetIntegerv(eGL_PACK_LSB_FIRST, &packParams[1]); @@ -1278,7 +1305,12 @@ void GLResourceManager::Apply_InitialState(GLResource live, InitialContentData i // AMD throws an error copying mips that are smaller than the block size in one dimension, so do copy via // CPU instead (will be slow, potentially we could optimise this if there's a different GPU-side image copy // routine that works on these dimensions. Hopefully there'll only be a couple of such mips). - if(iscomp && VendorCheck[VendorCheck_AMD_copy_compressed_tinymips] && (w < 4 || h < 4)) + // + // AMD also has issues copying cubemaps + if( + (iscomp && VendorCheck[VendorCheck_AMD_copy_compressed_tinymips] && (w < 4 || h < 4)) || + (iscomp && VendorCheck[VendorCheck_AMD_copy_compressed_cubemaps] && details.curType == eGL_TEXTURE_CUBE_MAP) + ) { GLenum targets[] = { eGL_TEXTURE_CUBE_MAP_POSITIVE_X, @@ -1339,7 +1371,7 @@ void GLResourceManager::Apply_InitialState(GLResource live, InitialContentData i } } - if(iscomp && VendorCheck[VendorCheck_AMD_copy_compressed_tinymips]) + if(avoidCopySubImage) { gl.glPixelStorei(eGL_PACK_SWAP_BYTES, packParams[0]); gl.glPixelStorei(eGL_PACK_LSB_FIRST, packParams[1]);