From 551d2b6479dbf0d7bfced0084de63df1a6a6cb6f Mon Sep 17 00:00:00 2001 From: Robert Osfield Date: Tue, 14 Sep 2010 13:19:55 +0000 Subject: [PATCH] From Ulrich Hertlein, "not sure how severe this is but I believe there's a bug in Texture.cpp:applyTexImage2D_subload: unsigned char* data = = (unsigned char*)image->data(); if (needImageRescale) { // allocates rescale buffer data = new unsigned char[newTotalSize]; // calls gluScaleImage into the data buffer } const unsigned char* dataPtr = image->data(); // subloads 'dataPtr' // deletes 'data' In effect, the scaled data would never be used. I've also replaced bits of duplicate code in Texture1D/2D/2DArray/3D/Cubemap/Rectangle that checks if the texture image can/should be unref'd with common functionality in Texture.cpp. " --- include/osg/Texture | 8 ++++++++ src/osg/Image.cpp | 29 ----------------------------- src/osg/Texture.cpp | 16 ++++++++-------- src/osg/Texture1D.cpp | 5 +++-- src/osg/Texture2D.cpp | 8 +++----- src/osg/Texture2DArray.cpp | 6 +++--- src/osg/Texture3D.cpp | 5 +++-- src/osg/TextureCubeMap.cpp | 5 +++-- src/osg/TextureRectangle.cpp | 6 +++--- 9 files changed, 34 insertions(+), 54 deletions(-) diff --git a/include/osg/Texture b/include/osg/Texture index c49859336..f1d9b6f77 100644 --- a/include/osg/Texture +++ b/include/osg/Texture @@ -805,10 +805,13 @@ class OSG_EXPORT Texture : public osg::StateAttribute virtual void computeInternalFormat() const = 0; + /** Computes the internal format from Image parameters. */ void computeInternalFormatWithImage(const osg::Image& image) const; + /** Computes the texture dimension for the given Image. */ void computeRequiredTextureDimensions(State& state, const osg::Image& image,GLsizei& width, GLsizei& height,GLsizei& numMipmapLevels) const; + /** Computes the internal format type. */ void computeInternalFormatType() const; /** Helper method. Sets texture parameters. */ @@ -818,6 +821,11 @@ class OSG_EXPORT Texture : public osg::StateAttribute * glGenerateMipmapEXT() or GL_GENERATE_MIPMAP_SGIS are supported. */ bool isHardwareMipmapGenerationEnabled(const State& state) const; + /** Returns true if the associated Image should be released and it's safe to do so. */ + bool isSafeToUnrefImageData(const State& state) const { + return (_unrefImageDataAfterApply && state.getMaxTexturePoolSize()==0 && areAllTextureObjectsLoaded()); + } + /** Helper methods to be called before and after calling * gl[Compressed][Copy]Tex[Sub]Image2D to handle generating mipmaps. */ GenerateMipmapMode mipmapBeforeTexImage(const State& state, bool hardwareMipmapOn) const; diff --git a/src/osg/Image.cpp b/src/osg/Image.cpp index 715000dc2..182e8299a 100644 --- a/src/osg/Image.cpp +++ b/src/osg/Image.cpp @@ -31,35 +31,6 @@ #include "dxtctool.h" -#if defined(OSG_GLES1_AVAILABLE) || defined(OSG_GLES2_AVAILABLE) - #define GL_RED 0x1903 - #define GL_GREEN 0x1904 - #define GL_BLUE 0x1905 - #define GL_DEPTH_COMPONENT 0x1902 - #define GL_STENCIL_INDEX 0x1901 -#endif - -#if defined(OSG_GLES1_AVAILABLE) || defined(OSG_GLES2_AVAILABLE) || defined(OSG_GL3_AVAILABLE) - #define GL_BITMAP 0x1A00 - #define GL_COLOR_INDEX 0x1900 - #define GL_INTENSITY12 0x804C - #define GL_INTENSITY16 0x804D - #define GL_INTENSITY4 0x804A - #define GL_INTENSITY8 0x804B - #define GL_LUMINANCE12 0x8041 - #define GL_LUMINANCE12_ALPHA4 0x8046 - #define GL_LUMINANCE12_ALPHA12 0x8047 - #define GL_LUMINANCE16 0x8042 - #define GL_LUMINANCE16_ALPHA16 0x8048 - #define GL_LUMINANCE4 0x803F - #define GL_LUMINANCE4_ALPHA4 0x8043 - #define GL_LUMINANCE6_ALPHA2 0x8044 - #define GL_LUMINANCE8 0x8040 - #define GL_LUMINANCE8_ALPHA8 0x8045 - #define GL_RGBA8 0x8058 - #define GL_PACK_ROW_LENGTH 0x0D02 -#endif - using namespace osg; using namespace std; diff --git a/src/osg/Texture.cpp b/src/osg/Texture.cpp index eecd4cb83..541a0920a 100644 --- a/src/osg/Texture.cpp +++ b/src/osg/Texture.cpp @@ -1656,7 +1656,8 @@ void Texture::applyTexImage2D_load(State& state, GLenum target, const Image* ima glPixelStorei(GL_PACK_ALIGNMENT,image->getPacking()); gluScaleImage(image->getPixelFormat(), image->s(),image->t(),image->getDataType(),image->data(), - inwidth,inheight,image->getDataType(),dataPtr); + inwidth,inheight,image->getDataType(), + dataPtr); #else OSG_NOTICE<<"Warning: gluScaleImage(..) not supported, cannot subload image."<getPacking()); - unsigned char* data = (unsigned char*)image->data(); - + unsigned char* dataPtr = (unsigned char*)image->data(); bool needImageRescale = inwidth!=image->s() || inheight!=image->t(); if (needImageRescale) @@ -1888,9 +1888,9 @@ void Texture::applyTexImage2D_subload(State& state, GLenum target, const Image* } unsigned int newTotalSize = osg::Image::computeRowWidthInBytes(inwidth,image->getPixelFormat(),image->getDataType(),image->getPacking())*inheight; - data = new unsigned char [newTotalSize]; + dataPtr = new unsigned char [newTotalSize]; - if (!data) + if (!dataPtr) { OSG_WARN<<"Warning:: Not enough memory to resize image, cannot apply to texture."<getPacking()); gluScaleImage(image->getPixelFormat(), image->s(),image->t(),image->getDataType(),image->data(), - inwidth,inheight,image->getDataType(),data); + inwidth,inheight,image->getDataType(), + dataPtr); #else OSG_NOTICE<<"Warning: gluScaleImage(..) not supported, cannot subload image."<isMipmap() && isHardwareMipmapGenerationEnabled(state)); bool useGluBuildMipMaps = mipmappingRequired && (!useHardwareMipMapGeneration && !image->isMipmap()); - const unsigned char* dataPtr = image->data(); GLBufferObject* pbo = image->getOrCreateGLBufferObject(contextID); if (pbo && !needImageRescale && !useGluBuildMipMaps) { @@ -2037,7 +2037,7 @@ void Texture::applyTexImage2D_subload(State& state, GLenum target, const Image* if (needImageRescale) { // clean up the resized image. - delete [] data; + delete [] dataPtr; } } diff --git a/src/osg/Texture1D.cpp b/src/osg/Texture1D.cpp index fe1e40a1b..7fa929315 100644 --- a/src/osg/Texture1D.cpp +++ b/src/osg/Texture1D.cpp @@ -208,10 +208,11 @@ void Texture1D::apply(State& state) const _textureObjectBuffer[contextID] = textureObject; - if (state.getMaxTexturePoolSize()==0 && _unrefImageDataAfterApply && areAllTextureObjectsLoaded() && _image->getDataVariance()==STATIC) + // unref image data? + if (isSafeToUnrefImageData(state) && _image->getDataVariance()==STATIC) { Texture1D* non_const_this = const_cast(this); - non_const_this->_image = 0; + non_const_this->_image = NULL; } } diff --git a/src/osg/Texture2D.cpp b/src/osg/Texture2D.cpp index d6b2165bb..4ce89f06a 100644 --- a/src/osg/Texture2D.cpp +++ b/src/osg/Texture2D.cpp @@ -248,13 +248,11 @@ void Texture2D::apply(State& state) const // update the modified tag to show that it is upto date. getModifiedCount(contextID) = image->getModifiedCount(); - if (state.getMaxTexturePoolSize()==0 && - _unrefImageDataAfterApply && - areAllTextureObjectsLoaded() && - image->getDataVariance()==STATIC) + // unref image data? + if (isSafeToUnrefImageData(state) && image->getDataVariance()==STATIC) { Texture2D* non_const_this = const_cast(this); - non_const_this->_image = 0; + non_const_this->_image = NULL; } // in theory the following line is redundent, but in practice diff --git a/src/osg/Texture2DArray.cpp b/src/osg/Texture2DArray.cpp index 41715b0d1..b4169c81b 100644 --- a/src/osg/Texture2DArray.cpp +++ b/src/osg/Texture2DArray.cpp @@ -363,15 +363,15 @@ void Texture2DArray::apply(State& state) const textureObject->setAllocated(_numMipmapLevels,_internalFormat,_textureWidth,_textureHeight,_textureDepth,0); - // no idea what this for ;-) - if (state.getMaxTexturePoolSize()==0 && _unrefImageDataAfterApply && areAllTextureObjectsLoaded()) + // unref image data? + if (isSafeToUnrefImageData(state)) { Texture2DArray* non_const_this = const_cast(this); for (int n=0; n<_textureDepth; n++) { if (_images[n].valid() && _images[n]->getDataVariance()==STATIC) { - non_const_this->_images[n] = 0; + non_const_this->_images[n] = NULL; } } } diff --git a/src/osg/Texture3D.cpp b/src/osg/Texture3D.cpp index 40b034d76..476c13129 100644 --- a/src/osg/Texture3D.cpp +++ b/src/osg/Texture3D.cpp @@ -298,10 +298,11 @@ void Texture3D::apply(State& state) const _textureObjectBuffer[contextID] = textureObject; - if (state.getMaxTexturePoolSize()==0 && _unrefImageDataAfterApply && areAllTextureObjectsLoaded() && _image->getDataVariance()==STATIC) + // unref image data? + if (isSafeToUnrefImageData(state) && _image->getDataVariance()==STATIC) { Texture3D* non_const_this = const_cast(this); - non_const_this->_image = 0; + non_const_this->_image = NULL; } } diff --git a/src/osg/TextureCubeMap.cpp b/src/osg/TextureCubeMap.cpp index 5da6fa5c9..c97234b6d 100644 --- a/src/osg/TextureCubeMap.cpp +++ b/src/osg/TextureCubeMap.cpp @@ -312,14 +312,15 @@ void TextureCubeMap::apply(State& state) const _textureObjectBuffer[contextID] = textureObject; - if (state.getMaxTexturePoolSize()==0 && _unrefImageDataAfterApply && areAllTextureObjectsLoaded()) + // unref image data? + if (isSafeToUnrefImageData(state)) { TextureCubeMap* non_const_this = const_cast(this); for (int n=0; n<6; n++) { if (_images[n].valid() && _images[n]->getDataVariance()==STATIC) { - non_const_this->_images[n] = 0; + non_const_this->_images[n] = NULL; } } } diff --git a/src/osg/TextureRectangle.cpp b/src/osg/TextureRectangle.cpp index fdab0ce78..cca317488 100644 --- a/src/osg/TextureRectangle.cpp +++ b/src/osg/TextureRectangle.cpp @@ -259,12 +259,12 @@ void TextureRectangle::apply(State& state) const textureObject->setAllocated(true); } - if (state.getMaxTexturePoolSize()==0 && _unrefImageDataAfterApply && areAllTextureObjectsLoaded() && _image->getDataVariance()==STATIC) + // unref image data? + if (isSafeToUnrefImageData(state) && _image->getDataVariance()==STATIC) { TextureRectangle* non_const_this = const_cast(this); - non_const_this->_image = 0; + non_const_this->_image = NULL; } - } else if ( (_textureWidth!=0) && (_textureHeight!=0) && (_internalFormat!=0) ) {