From Ulrich Hertlein, "not sure how severe this is but I believe there's a bug in

Texture.cpp:applyTexImage2D_subload:

<code>
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'
</code>

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.

"
This commit is contained in:
Robert Osfield 2010-09-14 13:19:55 +00:00
parent d55ada3790
commit 551d2b6479
9 changed files with 34 additions and 54 deletions

View File

@ -805,10 +805,13 @@ class OSG_EXPORT Texture : public osg::StateAttribute
virtual void computeInternalFormat() const = 0; virtual void computeInternalFormat() const = 0;
/** Computes the internal format from Image parameters. */
void computeInternalFormatWithImage(const osg::Image& image) const; 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; void computeRequiredTextureDimensions(State& state, const osg::Image& image,GLsizei& width, GLsizei& height,GLsizei& numMipmapLevels) const;
/** Computes the internal format type. */
void computeInternalFormatType() const; void computeInternalFormatType() const;
/** Helper method. Sets texture parameters. */ /** Helper method. Sets texture parameters. */
@ -818,6 +821,11 @@ class OSG_EXPORT Texture : public osg::StateAttribute
* glGenerateMipmapEXT() or GL_GENERATE_MIPMAP_SGIS are supported. */ * glGenerateMipmapEXT() or GL_GENERATE_MIPMAP_SGIS are supported. */
bool isHardwareMipmapGenerationEnabled(const State& state) const; 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 /** Helper methods to be called before and after calling
* gl[Compressed][Copy]Tex[Sub]Image2D to handle generating mipmaps. */ * gl[Compressed][Copy]Tex[Sub]Image2D to handle generating mipmaps. */
GenerateMipmapMode mipmapBeforeTexImage(const State& state, bool hardwareMipmapOn) const; GenerateMipmapMode mipmapBeforeTexImage(const State& state, bool hardwareMipmapOn) const;

View File

@ -31,35 +31,6 @@
#include "dxtctool.h" #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 osg;
using namespace std; using namespace std;

View File

@ -1656,7 +1656,8 @@ void Texture::applyTexImage2D_load(State& state, GLenum target, const Image* ima
glPixelStorei(GL_PACK_ALIGNMENT,image->getPacking()); glPixelStorei(GL_PACK_ALIGNMENT,image->getPacking());
gluScaleImage(image->getPixelFormat(), gluScaleImage(image->getPixelFormat(),
image->s(),image->t(),image->getDataType(),image->data(), image->s(),image->t(),image->getDataType(),image->data(),
inwidth,inheight,image->getDataType(),dataPtr); inwidth,inheight,image->getDataType(),
dataPtr);
#else #else
OSG_NOTICE<<"Warning: gluScaleImage(..) not supported, cannot subload image."<<std::endl; OSG_NOTICE<<"Warning: gluScaleImage(..) not supported, cannot subload image."<<std::endl;
return; return;
@ -1868,8 +1869,7 @@ void Texture::applyTexImage2D_subload(State& state, GLenum target, const Image*
glPixelStorei(GL_UNPACK_ALIGNMENT,image->getPacking()); glPixelStorei(GL_UNPACK_ALIGNMENT,image->getPacking());
unsigned char* data = (unsigned char*)image->data(); unsigned char* dataPtr = (unsigned char*)image->data();
bool needImageRescale = inwidth!=image->s() || inheight!=image->t(); bool needImageRescale = inwidth!=image->s() || inheight!=image->t();
if (needImageRescale) 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; 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."<<std::endl; OSG_WARN<<"Warning:: Not enough memory to resize image, cannot apply to texture."<<std::endl;
return; return;
@ -1903,7 +1903,8 @@ void Texture::applyTexImage2D_subload(State& state, GLenum target, const Image*
glPixelStorei(GL_PACK_ALIGNMENT,image->getPacking()); glPixelStorei(GL_PACK_ALIGNMENT,image->getPacking());
gluScaleImage(image->getPixelFormat(), gluScaleImage(image->getPixelFormat(),
image->s(),image->t(),image->getDataType(),image->data(), image->s(),image->t(),image->getDataType(),image->data(),
inwidth,inheight,image->getDataType(),data); inwidth,inheight,image->getDataType(),
dataPtr);
#else #else
OSG_NOTICE<<"Warning: gluScaleImage(..) not supported, cannot subload image."<<std::endl; OSG_NOTICE<<"Warning: gluScaleImage(..) not supported, cannot subload image."<<std::endl;
return; return;
@ -1915,7 +1916,6 @@ void Texture::applyTexImage2D_subload(State& state, GLenum target, const Image*
bool useHardwareMipMapGeneration = mipmappingRequired && (!image->isMipmap() && isHardwareMipmapGenerationEnabled(state)); bool useHardwareMipMapGeneration = mipmappingRequired && (!image->isMipmap() && isHardwareMipmapGenerationEnabled(state));
bool useGluBuildMipMaps = mipmappingRequired && (!useHardwareMipMapGeneration && !image->isMipmap()); bool useGluBuildMipMaps = mipmappingRequired && (!useHardwareMipMapGeneration && !image->isMipmap());
const unsigned char* dataPtr = image->data();
GLBufferObject* pbo = image->getOrCreateGLBufferObject(contextID); GLBufferObject* pbo = image->getOrCreateGLBufferObject(contextID);
if (pbo && !needImageRescale && !useGluBuildMipMaps) if (pbo && !needImageRescale && !useGluBuildMipMaps)
{ {
@ -2037,7 +2037,7 @@ void Texture::applyTexImage2D_subload(State& state, GLenum target, const Image*
if (needImageRescale) if (needImageRescale)
{ {
// clean up the resized image. // clean up the resized image.
delete [] data; delete [] dataPtr;
} }
} }

View File

@ -208,10 +208,11 @@ void Texture1D::apply(State& state) const
_textureObjectBuffer[contextID] = textureObject; _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<Texture1D*>(this); Texture1D* non_const_this = const_cast<Texture1D*>(this);
non_const_this->_image = 0; non_const_this->_image = NULL;
} }
} }

View File

@ -248,13 +248,11 @@ void Texture2D::apply(State& state) const
// update the modified tag to show that it is upto date. // update the modified tag to show that it is upto date.
getModifiedCount(contextID) = image->getModifiedCount(); getModifiedCount(contextID) = image->getModifiedCount();
if (state.getMaxTexturePoolSize()==0 && // unref image data?
_unrefImageDataAfterApply && if (isSafeToUnrefImageData(state) && image->getDataVariance()==STATIC)
areAllTextureObjectsLoaded() &&
image->getDataVariance()==STATIC)
{ {
Texture2D* non_const_this = const_cast<Texture2D*>(this); Texture2D* non_const_this = const_cast<Texture2D*>(this);
non_const_this->_image = 0; non_const_this->_image = NULL;
} }
// in theory the following line is redundent, but in practice // in theory the following line is redundent, but in practice

View File

@ -363,15 +363,15 @@ void Texture2DArray::apply(State& state) const
textureObject->setAllocated(_numMipmapLevels,_internalFormat,_textureWidth,_textureHeight,_textureDepth,0); textureObject->setAllocated(_numMipmapLevels,_internalFormat,_textureWidth,_textureHeight,_textureDepth,0);
// no idea what this for ;-) // unref image data?
if (state.getMaxTexturePoolSize()==0 && _unrefImageDataAfterApply && areAllTextureObjectsLoaded()) if (isSafeToUnrefImageData(state))
{ {
Texture2DArray* non_const_this = const_cast<Texture2DArray*>(this); Texture2DArray* non_const_this = const_cast<Texture2DArray*>(this);
for (int n=0; n<_textureDepth; n++) for (int n=0; n<_textureDepth; n++)
{ {
if (_images[n].valid() && _images[n]->getDataVariance()==STATIC) if (_images[n].valid() && _images[n]->getDataVariance()==STATIC)
{ {
non_const_this->_images[n] = 0; non_const_this->_images[n] = NULL;
} }
} }
} }

View File

@ -298,10 +298,11 @@ void Texture3D::apply(State& state) const
_textureObjectBuffer[contextID] = textureObject; _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<Texture3D*>(this); Texture3D* non_const_this = const_cast<Texture3D*>(this);
non_const_this->_image = 0; non_const_this->_image = NULL;
} }
} }

View File

@ -312,14 +312,15 @@ void TextureCubeMap::apply(State& state) const
_textureObjectBuffer[contextID] = textureObject; _textureObjectBuffer[contextID] = textureObject;
if (state.getMaxTexturePoolSize()==0 && _unrefImageDataAfterApply && areAllTextureObjectsLoaded()) // unref image data?
if (isSafeToUnrefImageData(state))
{ {
TextureCubeMap* non_const_this = const_cast<TextureCubeMap*>(this); TextureCubeMap* non_const_this = const_cast<TextureCubeMap*>(this);
for (int n=0; n<6; n++) for (int n=0; n<6; n++)
{ {
if (_images[n].valid() && _images[n]->getDataVariance()==STATIC) if (_images[n].valid() && _images[n]->getDataVariance()==STATIC)
{ {
non_const_this->_images[n] = 0; non_const_this->_images[n] = NULL;
} }
} }
} }

View File

@ -259,12 +259,12 @@ void TextureRectangle::apply(State& state) const
textureObject->setAllocated(true); 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<TextureRectangle*>(this); TextureRectangle* non_const_this = const_cast<TextureRectangle*>(this);
non_const_this->_image = 0; non_const_this->_image = NULL;
} }
} }
else if ( (_textureWidth!=0) && (_textureHeight!=0) && (_internalFormat!=0) ) else if ( (_textureWidth!=0) && (_textureHeight!=0) && (_internalFormat!=0) )
{ {