From ff1181e12a7532d4185b50f3a279854847c22c6b Mon Sep 17 00:00:00 2001 From: Pavel Krajcevski Date: Mon, 3 Nov 2014 18:24:19 -0500 Subject: [PATCH] Lots of QOL fixes: 1. Avoid the need for multiple-of-four input textures. If you don't pass a multiple of four then we'll do our best to pad the image. 2. Fix a bunch of bugs where we didn't accurately report why we couldn't compress a texture and just crashed instead. 3. Some code refactoring to make certain if statements more readable. --- CLTool/src/tc.cpp | 120 +++++++++++++++++++++-------------------- Core/src/TexComp.cpp | 73 ++++++++++++++++++------- IO/src/ImageLoader.cpp | 42 +++++---------- 3 files changed, 131 insertions(+), 104 deletions(-) diff --git a/CLTool/src/tc.cpp b/CLTool/src/tc.cpp index 158ef5d..32b08ad 100644 --- a/CLTool/src/tc.cpp +++ b/CLTool/src/tc.cpp @@ -116,13 +116,14 @@ void ExtractBasename(const char *filename, char *buf, size_t bufSz) { int main(int argc, char **argv) { int fileArg = 1; - if(fileArg == argc) { + if (fileArg == argc) { PrintUsage(); exit(1); } - char decompressedOutput[256]; decompressedOutput[0] = '\0'; - bool bNoDecompress = false; + char decompressedOutput[256]; + decompressedOutput[0] = '\0'; + bool bDecompress = true; int numJobs = 0; int quality = 50; int numThreads = 1; @@ -139,10 +140,10 @@ int main(int argc, char **argv) { do { knowArg = false; - if(strcmp(argv[fileArg], "-n") == 0) { + if (strcmp(argv[fileArg], "-n") == 0) { fileArg++; - if(fileArg == argc || (numCompressions = atoi(argv[fileArg])) < 0) { + if (fileArg == argc || (numCompressions = atoi(argv[fileArg])) < 0) { PrintUsage(); exit(1); } @@ -152,26 +153,26 @@ int main(int argc, char **argv) { continue; } - if(strcmp(argv[fileArg], "-f") == 0) { + if (strcmp(argv[fileArg], "-f") == 0) { fileArg++; - if(fileArg == argc) { + if (fileArg == argc) { PrintUsage(); exit(1); } else { - if(!strcmp(argv[fileArg], "PVRTC")) { + if (!strcmp(argv[fileArg], "PVRTC")) { format = FasTC::eCompressionFormat_PVRTC4; - } else if(!strcmp(argv[fileArg], "PVRTCLib")) { + } else if (!strcmp(argv[fileArg], "PVRTCLib")) { format = FasTC::eCompressionFormat_PVRTC4; bUsePVRTexLib = true; - } else if(!strcmp(argv[fileArg], "BPTCLib")) { + } else if (!strcmp(argv[fileArg], "BPTCLib")) { format = FasTC::eCompressionFormat_BPTC; bUseNVTT = true; - } else if(!strcmp(argv[fileArg], "ETC1")) { + } else if (!strcmp(argv[fileArg], "ETC1")) { format = FasTC::eCompressionFormat_ETC1; - } else if(!strcmp(argv[fileArg], "DXT1")) { + } else if (!strcmp(argv[fileArg], "DXT1")) { format = FasTC::eCompressionFormat_DXT1; - } else if(!strcmp(argv[fileArg], "DXT5")) { + } else if (!strcmp(argv[fileArg], "DXT5")) { format = FasTC::eCompressionFormat_DXT5; } } @@ -181,10 +182,10 @@ int main(int argc, char **argv) { continue; } - if(strcmp(argv[fileArg], "-d") == 0) { + if (strcmp(argv[fileArg], "-d") == 0) { fileArg++; - - if(fileArg == argc) { + + if (fileArg == argc) { PrintUsage(); exit(1); } else { @@ -198,38 +199,38 @@ int main(int argc, char **argv) { continue; } - if(strcmp(argv[fileArg], "-nd") == 0) { + if (strcmp(argv[fileArg], "-nd") == 0) { fileArg++; - bNoDecompress = true; + bDecompress = false; knowArg = true; continue; } - if(strcmp(argv[fileArg], "-l") == 0) { + if (strcmp(argv[fileArg], "-l") == 0) { fileArg++; bSaveLog = true; knowArg = true; continue; } - - if(strcmp(argv[fileArg], "-v") == 0) { + + if (strcmp(argv[fileArg], "-v") == 0) { fileArg++; bVerbose = true; knowArg = true; continue; } - - if(strcmp(argv[fileArg], "-simd") == 0) { + + if (strcmp(argv[fileArg], "-simd") == 0) { fileArg++; bUseSIMD = true; knowArg = true; continue; } - if(strcmp(argv[fileArg], "-t") == 0) { + if (strcmp(argv[fileArg], "-t") == 0) { fileArg++; - - if(fileArg == argc || (numThreads = atoi(argv[fileArg])) < 1) { + + if (fileArg == argc || (numThreads = atoi(argv[fileArg])) < 1) { PrintUsage(); exit(1); } @@ -239,10 +240,10 @@ int main(int argc, char **argv) { continue; } - if(strcmp(argv[fileArg], "-q") == 0) { + if (strcmp(argv[fileArg], "-q") == 0) { fileArg++; - - if(fileArg == argc || (quality = atoi(argv[fileArg])) < 0) { + + if (fileArg == argc || (quality = atoi(argv[fileArg])) < 0) { PrintUsage(); exit(1); } @@ -252,10 +253,10 @@ int main(int argc, char **argv) { continue; } - if(strcmp(argv[fileArg], "-j") == 0) { + if (strcmp(argv[fileArg], "-j") == 0) { fileArg++; - - if(fileArg == argc || (numJobs = atoi(argv[fileArg])) < 0) { + + if (fileArg == argc || (numJobs = atoi(argv[fileArg])) < 0) { PrintUsage(); exit(1); } @@ -265,16 +266,16 @@ int main(int argc, char **argv) { continue; } - if(strcmp(argv[fileArg], "-a") == 0) { + if (strcmp(argv[fileArg], "-a") == 0) { fileArg++; bUseAtomics = true; knowArg = true; continue; } - } while(knowArg && fileArg < argc); + } while (knowArg && fileArg < argc); - if(fileArg == argc) { + if (fileArg == argc) { PrintUsage(); exit(1); } @@ -282,15 +283,14 @@ int main(int argc, char **argv) { char basename[256]; ExtractBasename(argv[fileArg], basename, 256); - ImageFile file (argv[fileArg]); - if(!file.Load()) { - fprintf(stderr, "Error loading file: %s\n", argv[fileArg]); + ImageFile file(argv[fileArg]); + if (!file.Load()) { return 1; } FasTC::Image<> img(*file.GetImage()); - if(bVerbose) { + if (bVerbose) { fprintf(stdout, "Entropy: %.5f\n", img.ComputeEntropy()); fprintf(stdout, "Mean Local Entropy: %.5f\n", img.ComputeMeanLocalEntropy()); } @@ -298,12 +298,12 @@ int main(int argc, char **argv) { std::ofstream logFile; ThreadSafeStreambuf streamBuf(logFile); std::ostream logStream(&streamBuf); - if(bSaveLog) { + if (bSaveLog) { char logname[256]; sprintf(logname, "%s.log", basename); logFile.open(logname); } - + SCompressionSettings settings; settings.format = format; settings.bUseSIMD = bUseSIMD; @@ -314,36 +314,40 @@ int main(int argc, char **argv) { settings.iJobSize = numJobs; settings.bUsePVRTexLib = bUsePVRTexLib; settings.bUseNVTT = bUseNVTT; - if(bSaveLog) { + if (bSaveLog) { settings.logStream = &logStream; } else { settings.logStream = NULL; } CompressedImage *ci = CompressImage(&img, settings); - if(NULL == ci) { - fprintf(stderr, "Error compressing image!\n"); + if (NULL == ci) { return 1; } - double PSNR = img.ComputePSNR(ci); - if(PSNR > 0.0) { - fprintf(stdout, "PSNR: %.3f\n", PSNR); - } - else { - fprintf(stderr, "Error computing PSNR\n"); - } + if (ci->GetWidth() != img.GetWidth() || + ci->GetHeight() != img.GetHeight()) { + fprintf(stderr, "Cannot compute image metrics: compressed and uncompressed dimensions differ.\n"); + } else { + double PSNR = img.ComputePSNR(ci); + if(PSNR > 0.0) { + fprintf(stdout, "PSNR: %.3f\n", PSNR); + } + else { + fprintf(stderr, "Error computing PSNR\n"); + } - if(bVerbose) { - double SSIM = img.ComputeSSIM(ci); - if(SSIM > 0.0) { - fprintf(stdout, "SSIM: %.9f\n", SSIM); - } else { - fprintf(stderr, "Error computing SSIM\n"); + if(bVerbose) { + double SSIM = img.ComputeSSIM(ci); + if(SSIM > 0.0) { + fprintf(stdout, "SSIM: %.9f\n", SSIM); + } else { + fprintf(stderr, "Error computing SSIM\n"); + } } } - if(!bNoDecompress) { + if(bDecompress) { if(decompressedOutput[0] != '\0') { memcpy(basename, decompressedOutput, 256); } else if(format == FasTC::eCompressionFormat_BPTC) { diff --git a/Core/src/TexComp.cpp b/Core/src/TexComp.cpp index d85faae..0a4d92a 100644 --- a/Core/src/TexComp.cpp +++ b/Core/src/TexComp.cpp @@ -45,16 +45,16 @@ #include #include -#include #include #include #include +#include -#include "ETCCompressor.h" -#include "DXTCompressor.h" #include "BPTCCompressor.h" +#include "CompressionFormat.h" #include "CompressionFuncs.h" -#include "Image.h" +#include "DXTCompressor.h" +#include "ETCCompressor.h" #include "ImageFile.h" #include "Pixel.h" #include "PVRTCCompressor.h" @@ -188,7 +188,10 @@ static double CompressImageInSerial( const SCompressionSettings &settings ) { CompressionFunc f = ChooseFuncFromSettings(settings); - CompressionFuncWithStats fStats = ChooseFuncFromSettingsWithStats(settings); + CompressionFuncWithStats fStats = NULL; + if (settings.logStream) { + fStats = ChooseFuncFromSettingsWithStats(settings); + } double cmpTimeTotal = 0.0; @@ -392,30 +395,51 @@ CompressedImage *CompressImage( ) { if(!img) return NULL; - const uint32 w = img->GetWidth(); - const uint32 h = img->GetHeight(); + uint32 width = img->GetWidth(); + uint32 height = img->GetHeight(); + assert(width > 0); + assert(height > 0); + + // Make sure that the width and height of the image is a multiple of + // the block size of the format + uint32 blockDims[2]; + FasTC::GetBlockDimensions(settings.format, blockDims); + if ((width % blockDims[0]) != 0 || (height % blockDims[1]) != 0) { + ReportError("WARNING - Image size is not a multiple of block size. Padding with zeros..."); + uint32 newWidth = ((width + (blockDims[0] - 1)) / blockDims[0]) * blockDims[0]; + uint32 newHeight = ((height + (blockDims[1] - 1)) / blockDims[1]) * blockDims[1]; + + assert(newWidth > width || newHeight > height); + assert(newWidth % blockDims[0] == 0); + assert(newHeight % blockDims[1] == 0); + + width = newWidth; + height = newHeight; + } + + uint32 dataSz = width * height * 4; + uint32 *data = new uint32[dataSz / 4]; + memset(data, 0, dataSz); CompressedImage *outImg = NULL; - const unsigned int dataSz = w * h * 4; - uint32 *data = new uint32[dataSz / 4]; - - assert(dataSz > 0); // Allocate data based on the compression method uint32 cmpDataSz = CompressedImage::GetCompressedSize(dataSz, settings.format); // Make sure that we have RGBA data... img->ComputePixels(); - const PixelType *pixels = img->GetPixels(); - for(uint32 i = 0; i < img->GetNumPixels(); i++) { - data[i] = pixels[i].Pack(); + for(uint32 j = 0; j < img->GetHeight(); j++) { + for(uint32 i = 0; i < img->GetWidth(); i++) { + data[j * width + i] = (*img)(i, j).Pack(); + } } unsigned char *cmpData = new unsigned char[cmpDataSz]; - CompressImageData(reinterpret_cast(data), w, h, cmpData, cmpDataSz, settings); + uint8 *dataPtr = reinterpret_cast(data); + if (CompressImageData(dataPtr, width, height, cmpData, cmpDataSz, settings)) { + outImg = new CompressedImage(width, height, settings.format, cmpData); + } - outImg = new CompressedImage(w, h, settings.format, cmpData); - delete [] data; delete [] cmpData; return outImg; @@ -433,7 +457,7 @@ bool CompressImageData( uint8 *compressedData, const uint32 cmpDataSz, const SCompressionSettings &settings -) { +) { uint32 dataSz = width * height * 4; @@ -471,6 +495,19 @@ bool CompressImageData( } } + uint32 blockDims[2]; + FasTC::GetBlockDimensions(settings.format, blockDims); + if ((width % blockDims[0]) != 0 || (height % blockDims[1]) != 0) { + ReportError("ERROR - CompressImageData: width or height is not multiple of block dimension"); + return false; + } else if (settings.format == FasTC::eCompressionFormat_PVRTC4 && + ((width & (width - 1)) != 0 || + (height & (height - 1)) != 0 || + width != height)) { + ReportError("ERROR - CompressImageData: PVRTC4 images must be square and power-of-two."); + return false; + } + // Allocate data based on the compression method uint32 compressedDataSzNeeded = CompressedImage::GetCompressedSize(dataSz, settings.format); diff --git a/IO/src/ImageLoader.cpp b/IO/src/ImageLoader.cpp index cbc9a23..b2b060d 100644 --- a/IO/src/ImageLoader.cpp +++ b/IO/src/ImageLoader.cpp @@ -85,8 +85,8 @@ unsigned int ImageLoader::GetChannelForPixel(uint32 x, uint32 y, uint32 ch) { return 0; } - uint32 prec; - const uint8 *data; + uint32 prec = 0; + const uint8 *data = NULL; switch(ch) { case 0: @@ -135,7 +135,7 @@ unsigned int ImageLoader::GetChannelForPixel(uint32 x, uint32 y, uint32 ch) { } } - return ret; + return static_cast(ret); } else if(prec > 8) { const int32 toShift = prec - 8; @@ -164,10 +164,10 @@ bool ImageLoader::LoadFromPixelBuffer(const uint32 *data, bool flipY) { if(flipY) idx = (m_Height - j - 1)*m_Height + i; uint32 pixel = data[idx]; - m_RedData[pIdx] = pixel & 0xFF; - m_GreenData[pIdx] = (pixel >> 8) & 0xFF; - m_BlueData[pIdx] = (pixel >> 16) & 0xFF; - m_AlphaData[pIdx] = (pixel >> 24) & 0xFF; + m_RedData[pIdx] = static_cast(pixel & 0xFF); + m_GreenData[pIdx] = static_cast((pixel >> 8) & 0xFF); + m_BlueData[pIdx] = static_cast((pixel >> 16) & 0xFF); + m_AlphaData[pIdx] = static_cast((pixel >> 24) & 0xFF); } } @@ -189,24 +189,13 @@ FasTC::Image<> *ImageLoader::LoadImage() { m_Width = GetWidth(); m_Height = GetHeight(); - // Populate buffer in block stream order... make - // sure that width and height are aligned to multiples of four. - const unsigned int aw = ((m_Width + 3) >> 2) << 2; - const unsigned int ah = ((m_Height + 3) >> 2) << 2; - // Create RGBA buffer - const unsigned int dataSz = 4 * aw * ah; + const unsigned int dataSz = 4 * GetWidth() * GetHeight(); m_PixelData = new unsigned char[dataSz]; -#ifndef NDEBUG - if(aw != m_Width || ah != m_Height) - fprintf(stderr, "Warning: Image dimension not multiple of four. " - "Space will be filled with black.\n"); -#endif - int byteIdx = 0; - for(uint32 j = 0; j < ah; j++) { - for(uint32 i = 0; i < aw; i++) { + for(uint32 j = 0; j < GetHeight(); j++) { + for(uint32 i = 0; i < GetWidth(); i++) { unsigned int redVal = GetChannelForPixel(i, j, 0); if(redVal == INT_MAX) { @@ -239,22 +228,19 @@ FasTC::Image<> *ImageLoader::LoadImage() { } // Red channel - m_PixelData[byteIdx++] = redVal & 0xFF; + m_PixelData[byteIdx++] = static_cast(redVal & 0xFF); // Green channel - m_PixelData[byteIdx++] = greenVal & 0xFF; + m_PixelData[byteIdx++] = static_cast(greenVal & 0xFF); // Blue channel - m_PixelData[byteIdx++] = blueVal & 0xFF; + m_PixelData[byteIdx++] = static_cast(blueVal & 0xFF); // Alpha channel - m_PixelData[byteIdx++] = alphaVal & 0xFF; + m_PixelData[byteIdx++] = static_cast(alphaVal & 0xFF); } } - m_Width = aw; - m_Height = ah; - uint32 *pixels = reinterpret_cast(m_PixelData); return new FasTC::Image<>(m_Width, m_Height, pixels); }