From 24e6d8c73c62cf2b03bdcf3a13ee5e0271849138 Mon Sep 17 00:00:00 2001 From: AzaezelX Date: Thu, 25 Jun 2020 17:34:48 -0500 Subject: [PATCH 1/3] partial followup to #202. largely cleanups, though does include glCopyImageSubData for gl revs that support it TODOS: 1) non glCopyImageSubData for `void GFXGLCubemapArray::copyTo(GFXCubemapArray *pDstCubemap)` 2) while we don't get corruption showing in >0 mips for irradiance maps using the newer code, it nevertheless renders wrong. not 100% sure if renderdoc is showing blank mips because it expects it due to size, or if we are in fact generating em even though we don't use em in the end for irradiance references. --- Engine/source/gfx/gl/gfxGLCubemap.cpp | 2 +- Engine/source/gfx/gl/gfxGLDevice.cpp | 33 ++++++++++++--------- Engine/source/gfx/gl/gfxGLTextureObject.cpp | 2 +- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/Engine/source/gfx/gl/gfxGLCubemap.cpp b/Engine/source/gfx/gl/gfxGLCubemap.cpp index 541cd4fc7..7d70c1795 100644 --- a/Engine/source/gfx/gl/gfxGLCubemap.cpp +++ b/Engine/source/gfx/gl/gfxGLCubemap.cpp @@ -227,7 +227,7 @@ void GFXGLCubemap::initDynamic(U32 texSize, GFXFormat faceFormat, U32 mipLevels) } } - if( !isCompressed ) + if( !isCompressed && !mipLevels) glGenerateMipmap(GL_TEXTURE_CUBE_MAP); mInitialized = true; } diff --git a/Engine/source/gfx/gl/gfxGLDevice.cpp b/Engine/source/gfx/gl/gfxGLDevice.cpp index 57066acf9..5c9c8aea2 100644 --- a/Engine/source/gfx/gl/gfxGLDevice.cpp +++ b/Engine/source/gfx/gl/gfxGLDevice.cpp @@ -475,28 +475,33 @@ void GFXGLDevice::copyResource(GFXTextureObject* pDst, GFXCubemap* pSrc, const U const bool isCompressed = ImageUtil::isCompressedFormat(format); - U32 mipLevels = pGLSrc->getMipMapLevels(); - if (mipLevels < 1) mipLevels = 1;//ensure we loop at least the once + U32 mipLevels = gGLDst->getMipLevels(); for (U32 mip = 0; mip < mipLevels; mip++) { - U8* pixelData = pGLSrc->getTextureData(face, mip); - - glBindTexture(gGLDst->getBinding(), gGLDst->getHandle()); const U32 mipSize = getMax(U32(1), pGLSrc->getSize() >> mip); - if (isCompressed) + if (GFXGL->mCapabilities.copyImage) { - const U32 mipDataSize = getCompressedSurfaceSize(format, pGLSrc->getSize(), pGLSrc->getSize(), 0); - - glCompressedTexSubImage2D(gGLDst->getBinding(), mip, 0, 0, mipSize, mipSize, GFXGLTextureFormat[format], mipDataSize, pixelData); + glCopyImageSubData(pGLSrc->mCubemap, GL_TEXTURE_CUBE_MAP, mip, 0, 0, face, gGLDst->getHandle(), GL_TEXTURE_2D, mip, 0, 0, 0, mipSize, mipSize, 1); } else { - //glCopyImageSubData(pGLSrc->mCubemap, GFXGLCubemap::getEnumForFaceNumber(face), mip, 0, 0, face, gGLDst->getHandle(), GL_TEXTURE_2D, mip, 0, 0, 0, mipSize, mipSize, mip); - glTexSubImage2D(gGLDst->getBinding(), mip, 0, 0, mipSize, mipSize, GFXGLTextureFormat[format], GFXGLTextureType[format], pixelData); - } - glBindTexture(gGLDst->getBinding(), 0); + U8* pixelData = pGLSrc->getTextureData(face, mip); - delete[] pixelData; + glBindTexture(gGLDst->getBinding(), gGLDst->getHandle()); + if (isCompressed) + { + const U32 mipDataSize = getCompressedSurfaceSize(format, pGLSrc->getSize(), pGLSrc->getSize(), 0); + + glCompressedTexSubImage2D(gGLDst->getBinding(), mip, 0, 0, mipSize, mipSize, GFXGLTextureFormat[format], mipDataSize, pixelData); + } + else + { + glTexSubImage2D(gGLDst->getBinding(), mip, 0, 0, mipSize, mipSize, GFXGLTextureFormat[format], GFXGLTextureType[format], pixelData); + } + glBindTexture(gGLDst->getBinding(), 0); + + delete[] pixelData; + } } } diff --git a/Engine/source/gfx/gl/gfxGLTextureObject.cpp b/Engine/source/gfx/gl/gfxGLTextureObject.cpp index 4dac801d3..29c18bf6f 100644 --- a/Engine/source/gfx/gl/gfxGLTextureObject.cpp +++ b/Engine/source/gfx/gl/gfxGLTextureObject.cpp @@ -177,7 +177,6 @@ bool GFXGLTextureObject::copyToBmp(GBitmap * bmp) U32 mipLevels = getMipLevels(); - if (mipLevels < 1) mipLevels = 1;//ensure we loop at least the once for (U32 mip = 0; mip < mipLevels; mip++) { U32 srcPixelCount = bmp->getSurfaceSize(mip)/ srcBytesPerPixel; @@ -207,6 +206,7 @@ bool GFXGLTextureObject::copyToBmp(GBitmap * bmp) } } } + glBindTexture(mBinding, NULL); PROFILE_END(); return true; From ce2b05e0e2c3ef12eccc17cd89b990fcd1bb8ff9 Mon Sep 17 00:00:00 2001 From: AzaezelX Date: Mon, 29 Jun 2020 21:26:08 -0500 Subject: [PATCH 2/3] a) grab the source mipmap levels not the dest ones for copy. b) lets rely less on trusting sizeof*4 when we've gor a size per pixel lookup already there --- Engine/source/gfx/gl/gfxGLDevice.cpp | 2 +- Engine/source/gfx/gl/gfxGLTextureObject.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Engine/source/gfx/gl/gfxGLDevice.cpp b/Engine/source/gfx/gl/gfxGLDevice.cpp index 5c9c8aea2..e8caaaef4 100644 --- a/Engine/source/gfx/gl/gfxGLDevice.cpp +++ b/Engine/source/gfx/gl/gfxGLDevice.cpp @@ -475,7 +475,7 @@ void GFXGLDevice::copyResource(GFXTextureObject* pDst, GFXCubemap* pSrc, const U const bool isCompressed = ImageUtil::isCompressedFormat(format); - U32 mipLevels = gGLDst->getMipLevels(); + U32 mipLevels = pGLSrc->getMipMapLevels(); for (U32 mip = 0; mip < mipLevels; mip++) { const U32 mipSize = getMax(U32(1), pGLSrc->getSize() >> mip); diff --git a/Engine/source/gfx/gl/gfxGLTextureObject.cpp b/Engine/source/gfx/gl/gfxGLTextureObject.cpp index 29c18bf6f..fd686854a 100644 --- a/Engine/source/gfx/gl/gfxGLTextureObject.cpp +++ b/Engine/source/gfx/gl/gfxGLTextureObject.cpp @@ -189,7 +189,7 @@ bool GFXGLTextureObject::copyToBmp(GBitmap * bmp) PROFILE_START(GFXGLTextureObject_copyToBmp_pixCopy); if (mFormat == GFXFormatR16G16B16A16F) { - dMemcpy(dest, orig, srcPixelCount * sizeof(U16) * 4); + dMemcpy(dest, orig, srcPixelCount * srcBytesPerPixel); } else { From c57205b1f47e417e2f5be9f3c54608b76bb19a4f Mon Sep 17 00:00:00 2001 From: AzaezelX Date: Fri, 3 Jul 2020 22:56:44 -0500 Subject: [PATCH 3/3] from @rextimmy: final fixes --- Engine/source/gfx/gl/gfxGLCubemap.cpp | 29 ++++++-------- Engine/source/gfx/gl/gfxGLDevice.cpp | 44 ++++++++++++++++----- Engine/source/gfx/gl/gfxGLEnumTranslate.cpp | 9 +++++ Engine/source/gfx/gl/gfxGLEnumTranslate.h | 2 + 4 files changed, 56 insertions(+), 28 deletions(-) diff --git a/Engine/source/gfx/gl/gfxGLCubemap.cpp b/Engine/source/gfx/gl/gfxGLCubemap.cpp index 7d70c1795..99f3f02a8 100644 --- a/Engine/source/gfx/gl/gfxGLCubemap.cpp +++ b/Engine/source/gfx/gl/gfxGLCubemap.cpp @@ -31,13 +31,6 @@ #include "gfx/bitmap/imageUtils.h" -static GLenum faceList[6] = -{ - GL_TEXTURE_CUBE_MAP_POSITIVE_X, GL_TEXTURE_CUBE_MAP_NEGATIVE_X, - GL_TEXTURE_CUBE_MAP_POSITIVE_Y, GL_TEXTURE_CUBE_MAP_NEGATIVE_Y, - GL_TEXTURE_CUBE_MAP_POSITIVE_Z, GL_TEXTURE_CUBE_MAP_NEGATIVE_Z -}; - GFXGLCubemap::GFXGLCubemap() : mCubemap(0), mDynamicTexSize(0), @@ -59,7 +52,7 @@ GFXGLCubemap::~GFXGLCubemap() GLenum GFXGLCubemap::getEnumForFaceNumber(U32 face) { - return faceList[face]; + return GFXGLFaceType[face]; } void GFXGLCubemap::fillCubeTextures(GFXTexHandle* faces) @@ -105,14 +98,14 @@ void GFXGLCubemap::fillCubeTextures(GFXTexHandle* faces) const U32 mipDataSize = getCompressedSurfaceSize( mFaceFormat, mWidth, mHeight, mip ); U8* buf = glTex->getTextureData( mip ); - glCompressedTexImage2D(faceList[i], mip, GFXGLTextureInternalFormat[mFaceFormat], mipWidth, mipHeight, 0, mipDataSize, buf); + glCompressedTexImage2D(GFXGLFaceType[i], mip, GFXGLTextureInternalFormat[mFaceFormat], mipWidth, mipHeight, 0, mipDataSize, buf); delete[] buf; } } else { U8* buf = glTex->getTextureData(); - glTexImage2D(faceList[i], 0, GFXGLTextureInternalFormat[faceFormat], mWidth, mHeight, + glTexImage2D(GFXGLFaceType[i], 0, GFXGLTextureInternalFormat[faceFormat], mWidth, mHeight, 0, GFXGLTextureFormat[faceFormat], GFXGLTextureType[faceFormat], buf); delete[] buf; } @@ -181,9 +174,9 @@ void GFXGLCubemap::initStatic( DDSFile *dds ) const U32 mipWidth = getMax( U32(1), mWidth >> mip ); const U32 mipHeight = getMax( U32(1), mHeight >> mip ); if (isCompressed) - glCompressedTexImage2D(faceList[faceIndex], mip, GFXGLTextureInternalFormat[mFaceFormat], mipWidth, mipHeight, 0, dds->getSurfaceSize(mip), dds->mSurfaces[i]->mMips[mip]); + glCompressedTexImage2D(GFXGLFaceType[faceIndex], mip, GFXGLTextureInternalFormat[mFaceFormat], mipWidth, mipHeight, 0, dds->getSurfaceSize(mip), dds->mSurfaces[i]->mMips[mip]); else - glTexImage2D(faceList[faceIndex], mip, GFXGLTextureInternalFormat[mFaceFormat], mipWidth, mipHeight, 0, + glTexImage2D(GFXGLFaceType[faceIndex], mip, GFXGLTextureInternalFormat[mFaceFormat], mipWidth, mipHeight, 0, GFXGLTextureFormat[mFaceFormat], GFXGLTextureType[mFaceFormat], dds->mSurfaces[i]->mMips[mip]); } } @@ -195,7 +188,7 @@ void GFXGLCubemap::initDynamic(U32 texSize, GFXFormat faceFormat, U32 mipLevels) mDynamicTexSize = texSize; mFaceFormat = faceFormat; const bool isCompressed = ImageUtil::isCompressedFormat(faceFormat); - mMipMapLevels = getMax( (U32)1, getMaxMipmaps( texSize, texSize, 1 ) ); + mMipMapLevels = ImageUtil::getMaxMipCount( texSize, texSize); glGenTextures(1, &mCubemap); PRESERVE_CUBEMAP_TEXTURE(); @@ -217,12 +210,12 @@ void GFXGLCubemap::initDynamic(U32 texSize, GFXFormat faceFormat, U32 mipLevels) { const U32 mipSize = getMax( U32(1), texSize >> mip ); const U32 mipDataSize = getCompressedSurfaceSize( mFaceFormat, texSize, texSize, mip ); - glCompressedTexImage2D(faceList[i], mip, GFXGLTextureInternalFormat[mFaceFormat], mipSize, mipSize, 0, mipDataSize, NULL); + glCompressedTexImage2D(GFXGLFaceType[i], mip, GFXGLTextureInternalFormat[mFaceFormat], mipSize, mipSize, 0, mipDataSize, NULL); } } else { - glTexImage2D( faceList[i], 0, GFXGLTextureInternalFormat[faceFormat], texSize, texSize, + glTexImage2D( GFXGLFaceType[i], 0, GFXGLTextureInternalFormat[faceFormat], texSize, texSize, 0, GFXGLTextureFormat[faceFormat], GFXGLTextureType[faceFormat], NULL); } } @@ -302,9 +295,9 @@ U8* GFXGLCubemap::getTextureData(U32 face, U32 mip) glBindTexture(GL_TEXTURE_CUBE_MAP, mCubemap); if (ImageUtil::isCompressedFormat(mFaceFormat)) - glGetCompressedTexImage(faceList[face], mip, data); + glGetCompressedTexImage(GFXGLFaceType[face], mip, data); else - glGetTexImage(faceList[face], mip, GFXGLTextureFormat[mFaceFormat], GFXGLTextureType[mFaceFormat], data); + glGetTexImage(GFXGLFaceType[face], mip, GFXGLTextureFormat[mFaceFormat], GFXGLTextureType[mFaceFormat], data); return data; } @@ -480,7 +473,7 @@ void GFXGLCubemapArray::copyTo(GFXCubemapArray *pDstCubemap) /*if (isCompressed) { const U32 mipDataSize = getCompressedSurfaceSize(mFormat, mSize, mSize, currentMip); - glCompressedTexImage2D(faceList[face], currentMip, GFXGLTextureInternalFormat[mFormat], mipSize, mipSize, 0, mipDataSize, pixelData); + glCompressedTexImage2D(GFXGLFaceType[face], currentMip, GFXGLTextureInternalFormat[mFormat], mipSize, mipSize, 0, mipDataSize, pixelData); } else {*/ //TODO figure out xyzOffsets diff --git a/Engine/source/gfx/gl/gfxGLDevice.cpp b/Engine/source/gfx/gl/gfxGLDevice.cpp index e8caaaef4..18048b694 100644 --- a/Engine/source/gfx/gl/gfxGLDevice.cpp +++ b/Engine/source/gfx/gl/gfxGLDevice.cpp @@ -471,36 +471,60 @@ void GFXGLDevice::copyResource(GFXTextureObject* pDst, GFXCubemap* pSrc, const U GFXGLTextureObject* gGLDst = static_cast(pDst); GFXGLCubemap* pGLSrc = static_cast(pSrc); - GFXFormat format = pGLSrc->getFormat(); - + const GFXFormat format = pGLSrc->getFormat(); const bool isCompressed = ImageUtil::isCompressedFormat(format); + const U32 mipLevels = pGLSrc->getMipMapLevels(); + const U32 texSize = pGLSrc->getSize(); + + //set up pbo if we don't have copyImage support + if (!GFXGL->mCapabilities.copyImage) + { + const GLuint pbo = gGLDst->getBuffer(); + glBindBuffer(GL_PIXEL_PACK_BUFFER, pbo); + //allocate data + glBufferData(GL_PIXEL_PACK_BUFFER, texSize * texSize * GFXFormat_getByteSize(format), NULL, GL_STREAM_COPY); + } - U32 mipLevels = pGLSrc->getMipMapLevels(); for (U32 mip = 0; mip < mipLevels; mip++) { - const U32 mipSize = getMax(U32(1), pGLSrc->getSize() >> mip); + const U32 mipSize = texSize >> mip; if (GFXGL->mCapabilities.copyImage) { glCopyImageSubData(pGLSrc->mCubemap, GL_TEXTURE_CUBE_MAP, mip, 0, 0, face, gGLDst->getHandle(), GL_TEXTURE_2D, mip, 0, 0, 0, mipSize, mipSize, 1); } else { - U8* pixelData = pGLSrc->getTextureData(face, mip); + //pbo id + const GLuint pbo = gGLDst->getBuffer(); + //copy source texture data to pbo + glBindTexture(GL_TEXTURE_CUBE_MAP, pGLSrc->mCubemap); + glBindBuffer(GL_PIXEL_PACK_BUFFER, pbo); + + if (isCompressed) + glGetCompressedTexImage(GFXGLFaceType[face], mip, NULL); + else + glGetTexImage(GFXGLFaceType[face], mip, GFXGLTextureFormat[format], GFXGLTextureType[format], NULL); + + glBindBuffer(GL_PIXEL_PACK_BUFFER, 0); + glBindTexture(GL_TEXTURE_CUBE_MAP, 0); + + //copy data from pbo to destination + glBindBuffer(GL_PIXEL_UNPACK_BUFFER, pbo); glBindTexture(gGLDst->getBinding(), gGLDst->getHandle()); + if (isCompressed) { const U32 mipDataSize = getCompressedSurfaceSize(format, pGLSrc->getSize(), pGLSrc->getSize(), 0); - - glCompressedTexSubImage2D(gGLDst->getBinding(), mip, 0, 0, mipSize, mipSize, GFXGLTextureFormat[format], mipDataSize, pixelData); + glCompressedTexSubImage2D(gGLDst->getBinding(), mip, 0, 0, mipSize, mipSize, GFXGLTextureFormat[format], mipDataSize, NULL); } else { - glTexSubImage2D(gGLDst->getBinding(), mip, 0, 0, mipSize, mipSize, GFXGLTextureFormat[format], GFXGLTextureType[format], pixelData); + glTexSubImage2D(gGLDst->getBinding(), mip, 0, 0, mipSize, mipSize, GFXGLTextureFormat[format], GFXGLTextureType[format], NULL); } - glBindTexture(gGLDst->getBinding(), 0); - delete[] pixelData; + glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0); + glBindTexture(gGLDst->getBinding(), 0); } } } diff --git a/Engine/source/gfx/gl/gfxGLEnumTranslate.cpp b/Engine/source/gfx/gl/gfxGLEnumTranslate.cpp index 0ab380db5..422f25bc1 100644 --- a/Engine/source/gfx/gl/gfxGLEnumTranslate.cpp +++ b/Engine/source/gfx/gl/gfxGLEnumTranslate.cpp @@ -38,6 +38,7 @@ GLint* GFXGLTextureSwizzle[GFXFormat_COUNT]; GLenum GFXGLBufferType[GFXBufferType_COUNT]; GLenum GFXGLCullMode[GFXCull_COUNT]; GLenum GFXGLFillMode[GFXFill_COUNT]; +GLenum GFXGLFaceType[6]; void GFXGLEnumTranslate::init() { @@ -273,4 +274,12 @@ void GFXGLEnumTranslate::init() GFXGLFillMode[GFXFillPoint] = GL_POINT; GFXGLFillMode[GFXFillWireframe] = GL_LINE; GFXGLFillMode[GFXFillSolid] = GL_FILL; + + //cubemap face type + GFXGLFaceType[0] = GL_TEXTURE_CUBE_MAP_POSITIVE_X; + GFXGLFaceType[1] = GL_TEXTURE_CUBE_MAP_NEGATIVE_X; + GFXGLFaceType[2] = GL_TEXTURE_CUBE_MAP_POSITIVE_Y; + GFXGLFaceType[3] = GL_TEXTURE_CUBE_MAP_NEGATIVE_Y; + GFXGLFaceType[4] = GL_TEXTURE_CUBE_MAP_POSITIVE_Z; + GFXGLFaceType[5] = GL_TEXTURE_CUBE_MAP_NEGATIVE_Z; } diff --git a/Engine/source/gfx/gl/gfxGLEnumTranslate.h b/Engine/source/gfx/gl/gfxGLEnumTranslate.h index 000f64093..02a6b87b6 100644 --- a/Engine/source/gfx/gl/gfxGLEnumTranslate.h +++ b/Engine/source/gfx/gl/gfxGLEnumTranslate.h @@ -50,4 +50,6 @@ extern GLenum GFXGLCullMode[GFXCull_COUNT]; extern GLenum GFXGLFillMode[GFXFill_COUNT]; +extern GLenum GFXGLFaceType[6]; + #endif