From 53fd8010884b72faf97ff50f41e5e851cde24722 Mon Sep 17 00:00:00 2001 From: Robert Osfield Date: Tue, 28 May 2013 11:44:36 +0000 Subject: [PATCH] =?UTF-8?q?From=20Diane=20Delall=C3=A9e=20&=20Sukender,=20?= =?UTF-8?q?"Added=20some=20support=20of=20non-modulus-4=20dimensions=20in?= =?UTF-8?q?=20S3TC-DXTC=20images=20(previous=20implementation=20seem=20jus?= =?UTF-8?q?t=20not=20to=20handle=20these=20properly).=20-=20Added=20missin?= =?UTF-8?q?g=20packing=20value=20on=20S3TC=20images.=20Images=20are=20code?= =?UTF-8?q?d=20with=204x4=20blocs,=20whatever=20the=20image=20size.=20So?= =?UTF-8?q?=20there=20is=20an=20horizontal=20packing=20of=204=20pixels=20(?= =?UTF-8?q?2=20bytes=20in=20DXT1,=204=20bytes=20in=20DXT2-5).=20-=20Added?= =?UTF-8?q?=20crash=20guard=20against=20writing=20corrupted=20S3TC=20image?= =?UTF-8?q?s.=20Notes:=20-=20What=20is=20missing=20is=20a=20support=20of?= =?UTF-8?q?=20"lines=20packing"=20in=20osg::Image=20(see=20code=20comments?= =?UTF-8?q?).=20-=20S3TC-DXTC=20vertical=20flipping=20crashes=20(access=20?= =?UTF-8?q?violation)=20with=20some=20unusual=20dimensions=20(see=20code).?= =?UTF-8?q?=20I=20could=20not=20implement=20missing=20cases,=20so=20I=20ad?= =?UTF-8?q?ded=20guards=20to=20avoid=20crashing."?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/osgPlugins/dds/ReaderWriterDDS.cpp | 27 +++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/src/osgPlugins/dds/ReaderWriterDDS.cpp b/src/osgPlugins/dds/ReaderWriterDDS.cpp index b8dca23d4..0b537a657 100644 --- a/src/osgPlugins/dds/ReaderWriterDDS.cpp +++ b/src/osgPlugins/dds/ReaderWriterDDS.cpp @@ -494,6 +494,8 @@ osg::Image* ReadDDSFile(std::istream& _istream) // while compressed formats will use DDPF_FOURCC with a four-character code. bool usingAlpha = ddsd.ddpfPixelFormat.dwFlags & DDPF_ALPHAPIXELS; + int packing(1); + bool isDXTC(false); // Uncompressed formats. if(ddsd.ddpfPixelFormat.dwFlags & DDPF_RGB) @@ -659,16 +661,22 @@ osg::Image* ReadDDSFile(std::istream& _istream) internalFormat = GL_COMPRESSED_RGB_S3TC_DXT1_EXT; pixelFormat = GL_COMPRESSED_RGB_S3TC_DXT1_EXT; } + packing = 2; // 4 bits/pixel. 4 px = 2 bytes + isDXTC = true; break; case FOURCC_DXT3: OSG_INFO << "ReadDDSFile info : format = DXT3" << std::endl; internalFormat = GL_COMPRESSED_RGBA_S3TC_DXT3_EXT; pixelFormat = GL_COMPRESSED_RGBA_S3TC_DXT3_EXT; + packing = 4; // 8 bits/pixel. 4 px = 4 bytes + isDXTC = true; break; case FOURCC_DXT5: OSG_INFO << "ReadDDSFile info : format = DXT5" << std::endl; internalFormat = GL_COMPRESSED_RGBA_S3TC_DXT5_EXT; pixelFormat = GL_COMPRESSED_RGBA_S3TC_DXT5_EXT; + packing = 4; // 8 bits/pixel. 4 px = 4 bytes + isDXTC = true; break; case FOURCC_ATI1: OSG_INFO << "ReadDDSFile info : format = ATI1" << std::endl; @@ -965,7 +973,7 @@ osg::Image* ReadDDSFile(std::istream& _istream) return NULL; } - unsigned int size = ComputeImageSizeInBytes( s, t, r, pixelFormat, dataType ); + unsigned int size = ComputeImageSizeInBytes( s, t, r, pixelFormat, dataType, packing ); // Take care of mipmaps if any. unsigned int sizeWithMipmaps = size; @@ -990,7 +998,7 @@ osg::Image* ReadDDSFile(std::istream& _istream) depth = osg::maximum( depth >> 1, 1 ); sizeWithMipmaps += - ComputeImageSizeInBytes( width, height, depth, pixelFormat, dataType ); + ComputeImageSizeInBytes( width, height, depth, pixelFormat, dataType, packing ); } } @@ -1020,7 +1028,7 @@ osg::Image* ReadDDSFile(std::istream& _istream) // this memory will not be used but it will not cause leak in worst meaning of this word. } - osgImage->setImage(s,t,r, internalFormat, pixelFormat, dataType, imageData, osg::Image::USE_NEW_DELETE); + osgImage->setImage(s,t,r, internalFormat, pixelFormat, dataType, imageData, osg::Image::USE_NEW_DELETE, packing); if (mipmap_offsets.size()>0) osgImage->setMipmapLevels(mipmap_offsets); @@ -1060,6 +1068,19 @@ bool WriteDDSFile(const osg::Image *img, std::ostream& fout) unsigned int pixelSize = osg::Image::computePixelSizeInBits(pixelFormat, dataType); unsigned int imageSize = img->getImageSizeInBytes(); + // Check that theorical image size (computation taking into account DXTC blocks) is not bigger than actual image size. + // This may happen, for instance, if some operation tuncated the data buffer non block-aligned. Example: + // - Read DXT1 image, size = 8x7. Actually, image data is 8x8 because it stores 4x4 blocks. + // - Some hypothetical operation wrongly assumes the data buffer is 8x7 and truncates the buffer. This may even lead to access violations. + // - Then we write the DXT1 image: last block(s) is (are) corrupt. + // Actually what could be very nice is to handle some "lines packing" (?) in DDS reading, indicating that the image buffer has "additional lines to reach a multiple of 4". + // Please note this can also produce false positives (ie. when data buffer is large enough, but getImageSizeInBytes() returns a smaller value). There is no way to detect this, until we fix getImageSizeInBytes() with "line packing". + unsigned int imageSizeTheorical = ComputeImageSizeInBytes( img->s(), img->t(), img->r(), pixelFormat, dataType, img->getPacking() ); + if (imageSize < imageSizeTheorical) { + OSG_FATAL << "Image cannot be written as DDS (Maybe a corrupt S3TC-DXTC image, with non %4 dimensions)." << std::endl; + return false; + } + ddsd.dwWidth = img->s(); ddsd.dwHeight = img->t(); int r = img->r();