From 4ee2a61a15eae1d0bd2866264f51c392610e5091 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Tue, 1 Oct 2013 15:33:40 +0200 Subject: [PATCH 1/7] Added a siply wrapper .h/cpp pair to abstract away the fact that miniz in provided as a single .c file which we include directly, and to avoid linker problems. --- library/PolyVoxCore/CMakeLists.txt | 2 + .../include/PolyVoxCore/Impl/MinizWrapper.h | 39 +++++++++++++++++++ .../include/PolyVoxCore/MinizCompressor.h | 4 +- .../PolyVoxCore/source/MinizCompressor.cpp | 14 +++---- .../polyvoxcore/source/Impl/MinizWrapper.cpp | 27 +++++++++++++ 5 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 library/PolyVoxCore/include/PolyVoxCore/Impl/MinizWrapper.h create mode 100644 library/polyvoxcore/source/Impl/MinizWrapper.cpp diff --git a/library/PolyVoxCore/CMakeLists.txt b/library/PolyVoxCore/CMakeLists.txt index 5521f1dc..83a25fb9 100644 --- a/library/PolyVoxCore/CMakeLists.txt +++ b/library/PolyVoxCore/CMakeLists.txt @@ -110,6 +110,7 @@ SET(CORE_INC_FILES SET(IMPL_SRC_FILES source/Impl/ErrorHandling.cpp source/Impl/MarchingCubesTables.cpp + source/Impl/MinizWrapper.cpp source/Impl/RandomUnitVectors.cpp source/Impl/RandomVectors.cpp source/Impl/Timer.cpp @@ -124,6 +125,7 @@ SET(IMPL_INC_FILES include/PolyVoxCore/Impl/Config.h include/PolyVoxCore/Impl/ErrorHandling.h include/PolyVoxCore/Impl/MarchingCubesTables.h + include/PolyVoxCore/Impl/MinizWrapper.h include/PolyVoxCore/Impl/RandomUnitVectors.h include/PolyVoxCore/Impl/RandomVectors.h include/PolyVoxCore/Impl/SubArray.h diff --git a/library/PolyVoxCore/include/PolyVoxCore/Impl/MinizWrapper.h b/library/PolyVoxCore/include/PolyVoxCore/Impl/MinizWrapper.h new file mode 100644 index 00000000..d74a3278 --- /dev/null +++ b/library/PolyVoxCore/include/PolyVoxCore/Impl/MinizWrapper.h @@ -0,0 +1,39 @@ +/******************************************************************************* +Copyright (c) 2005-2013 David Williams + +This software is provided 'as-is', without any express or implied +warranty. In no event will the authors be held liable for any damages +arising from the use of this software. + +Permission is granted to anyone to use this software for any purpose, +including commercial applications, and to alter it and redistribute it +freely, subject to the following restrictions: + + 1. The origin of this software must not be misrepresented; you must not + claim that you wrote the original software. If you use this software + in a product, an acknowledgment in the product documentation would be + appreciated but is not required. + + 2. Altered source versions must be plainly marked as such, and must not be + misrepresented as being the original software. + + 3. This notice may not be removed or altered from any source + distribution. +*******************************************************************************/ + +#ifndef __PolyVox_MinizWrapper_H__ +#define __PolyVox_MinizWrapper_H__ + +// Diable things we don't need, and in particular the zlib compatible names which +// would cause conflicts if a user application is using both PolyVox and zlib. +#define MINIZ_NO_STDIO +#define MINIZ_NO_ARCHIVE_APIS +#define MINIZ_NO_TIME +#define MINIZ_NO_ZLIB_APIS +#define MINIZ_NO_ZLIB_COMPATIBLE_NAMES +#define MINIZ_NO_MALLOC + +#define MINIZ_HEADER_FILE_ONLY +#include "PolyVoxCore/Impl/miniz.c" + +#endif //__PolyVox_MinizWrapper_H__ \ No newline at end of file diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h b/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h index 7ea8f967..2f445387 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h @@ -3,6 +3,8 @@ #include "PolyVoxCore/Compressor.h" +#include "PolyVoxCore/Impl/MinizWrapper.h" + namespace PolyVox { /** @@ -34,7 +36,7 @@ namespace PolyVox // tdefl_compressor contains all the state needed by the low-level compressor so it's a pretty big struct (~300k). // We're storing it by void* because miniz does not supply a header and we don't want to include the .c file from // here as it will cause linker problems. - void* m_pDeflator; + tdefl_compressor* m_pDeflator; }; } diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index aa35c644..d2b127d3 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -4,12 +4,12 @@ // Diable things we don't need, and in particular the zlib compatible names which // would cause conflicts if a user application is using both PolyVox and zlib. -#define MINIZ_NO_STDIO +/*#define MINIZ_NO_STDIO #define MINIZ_NO_ARCHIVE_APIS #define MINIZ_NO_TIME #define MINIZ_NO_ZLIB_APIS #define MINIZ_NO_ZLIB_COMPATIBLE_NAMES -#define MINIZ_NO_MALLOC +#define MINIZ_NO_MALLOC*/ #include "PolyVoxCore/Impl/ErrorHandling.h" @@ -20,7 +20,7 @@ // directives) but the other problem is that we are using #pragma GCC system_header to supress warnings which would // then be in the .c part of the code. If we ever update GCC on the CDash machine so that it properly supports '#pragma // GCC diagnosic ignored' (or so that it doesn't warn in the first place) then we can reconsider spliting miniz.c in two. -#include "PolyVoxCore/Impl/miniz.c" +//#include "PolyVoxCore/Impl/miniz.c" #include @@ -39,14 +39,14 @@ namespace PolyVox { // Create and store the deflator. tdefl_compressor* pDeflator = new tdefl_compressor; - m_pDeflator = reinterpret_cast(pDeflator); + m_pDeflator = /*reinterpret_cast*/(pDeflator); // The number of dictionary probes to use at each compression level (0-10). 0=implies fastest/minimal possible probing. // The discontinuity is unsettling but may be explained by the 'iCompressionLevel <= 3' check later? static const mz_uint s_tdefl_num_probes[11] = { 0, 1, 6, 32, 16, 32, 128, 256, 512, 768, 1500 }; // Create tdefl() compatible flags (we have to compose the low-level flags ourselves, or use tdefl_create_comp_flags_from_zip_params() but that means MINIZ_NO_ZLIB_APIS can't be defined). - m_uCompressionFlags = TDEFL_WRITE_ZLIB_HEADER | s_tdefl_num_probes[MZ_MIN(10, iCompressionLevel)] | ((iCompressionLevel <= 3) ? TDEFL_GREEDY_PARSING_FLAG : 0); + m_uCompressionFlags = TDEFL_WRITE_ZLIB_HEADER | s_tdefl_num_probes[(std::min)(10, iCompressionLevel)] | ((iCompressionLevel <= 3) ? TDEFL_GREEDY_PARSING_FLAG : 0); if (!iCompressionLevel) { m_uCompressionFlags |= TDEFL_FORCE_ALL_RAW_BLOCKS; @@ -56,7 +56,7 @@ namespace PolyVox MinizCompressor::~MinizCompressor() { // Delete the deflator - tdefl_compressor* pDeflator = reinterpret_cast(m_pDeflator); + tdefl_compressor* pDeflator = /*reinterpret_cast*/(m_pDeflator); delete pDeflator; } @@ -67,7 +67,7 @@ namespace PolyVox unsigned long source_len = uUncompressedInputSize; // This is really over conservative. (And lame, but it's actually pretty tricky to compute a true upper bound given the way tdefl's blocking works.) - return MZ_MAX(128 + (source_len * 110) / 100, 128 + source_len + ((source_len / (31 * 1024)) + 1) * 5); + return (std::max)(128 + (source_len * 110) / 100, 128 + source_len + ((source_len / (31 * 1024)) + 1) * 5); } uint32_t MinizCompressor::compress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) diff --git a/library/polyvoxcore/source/Impl/MinizWrapper.cpp b/library/polyvoxcore/source/Impl/MinizWrapper.cpp new file mode 100644 index 00000000..9670ba41 --- /dev/null +++ b/library/polyvoxcore/source/Impl/MinizWrapper.cpp @@ -0,0 +1,27 @@ +/******************************************************************************* +Copyright (c) 2005-2013 David Williams + +This software is provided 'as-is', without any express or implied +warranty. In no event will the authors be held liable for any damages +arising from the use of this software. + +Permission is granted to anyone to use this software for any purpose, +including commercial applications, and to alter it and redistribute it +freely, subject to the following restrictions: + + 1. The origin of this software must not be misrepresented; you must not + claim that you wrote the original software. If you use this software + in a product, an acknowledgment in the product documentation would be + appreciated but is not required. + + 2. Altered source versions must be plainly marked as such, and must not be + misrepresented as being the original software. + + 3. This notice may not be removed or altered from any source + distribution. +*******************************************************************************/ + +#include "PolyVoxCore/Impl/MinizWrapper.h" + +#undef MINIZ_HEADER_FILE_ONLY +#include "PolyVoxCore/Impl/miniz.c" \ No newline at end of file From f63bb510b3dd372394d5d380ea75259a6d58ddbb Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Tue, 1 Oct 2013 15:51:23 +0200 Subject: [PATCH 2/7] Merged some code from MinizCompressor into MinizBlockCompressor. We don't really need two separate classes for this stuff. --- .../PolyVoxCore/MinizBlockCompressor.h | 16 ++- .../PolyVoxCore/MinizBlockCompressor.inl | 114 ++++++++++++++++-- 2 files changed, 120 insertions(+), 10 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h index e4b57cf6..57698a34 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h @@ -26,7 +26,7 @@ freely, subject to the following restrictions: #include "PolyVoxCore/BlockCompressor.h" -#include "PolyVoxCore/MinizCompressor.h" +#include "PolyVoxCore/Impl/MinizWrapper.h" namespace PolyVox { @@ -37,13 +37,23 @@ namespace PolyVox class MinizBlockCompressor : public BlockCompressor { public: - MinizBlockCompressor(); + MinizBlockCompressor(int iCompressionLevel = 6); // Miniz defines MZ_DEFAULT_LEVEL = 6 so we use the same here ~MinizBlockCompressor(); void compress(UncompressedBlock* pSrcBlock, CompressedBlock* pDstBlock); void decompress(CompressedBlock* pSrcBlock, UncompressedBlock* pDstBlock); - MinizCompressor* m_pCompressor; + private: + uint32_t getMaxCompressedSize(uint32_t uUncompressedInputSize); + uint32_t compress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); + uint32_t decompress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); + + unsigned int m_uCompressionFlags; + + // tdefl_compressor contains all the state needed by the low-level compressor so it's a pretty big struct (~300k). + // We're storing it by void* because miniz does not supply a header and we don't want to include the .c file from + // here as it will cause linker problems. + tdefl_compressor* m_pDeflator; }; } diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl index 39afa12f..59ad7708 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl +++ b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl @@ -21,18 +21,36 @@ freely, subject to the following restrictions: distribution. *******************************************************************************/ +#include + namespace PolyVox { template - MinizBlockCompressor::MinizBlockCompressor() + MinizBlockCompressor::MinizBlockCompressor(int iCompressionLevel) + :m_pDeflator(0) { - m_pCompressor = new MinizCompressor; + // Create and store the deflator. + tdefl_compressor* pDeflator = new tdefl_compressor; + m_pDeflator = /*reinterpret_cast*/(pDeflator); + + // The number of dictionary probes to use at each compression level (0-10). 0=implies fastest/minimal possible probing. + // The discontinuity is unsettling but may be explained by the 'iCompressionLevel <= 3' check later? + static const mz_uint s_tdefl_num_probes[11] = { 0, 1, 6, 32, 16, 32, 128, 256, 512, 768, 1500 }; + + // Create tdefl() compatible flags (we have to compose the low-level flags ourselves, or use tdefl_create_comp_flags_from_zip_params() but that means MINIZ_NO_ZLIB_APIS can't be defined). + m_uCompressionFlags = TDEFL_WRITE_ZLIB_HEADER | s_tdefl_num_probes[(std::min)(10, iCompressionLevel)] | ((iCompressionLevel <= 3) ? TDEFL_GREEDY_PARSING_FLAG : 0); + if (!iCompressionLevel) + { + m_uCompressionFlags |= TDEFL_FORCE_ALL_RAW_BLOCKS; + } } template MinizBlockCompressor::~MinizBlockCompressor() { - delete m_pCompressor; + // Delete the deflator + tdefl_compressor* pDeflator = /*reinterpret_cast*/(m_pDeflator); + delete pDeflator; } template @@ -50,7 +68,7 @@ namespace PolyVox try { // Perform the compression - uCompressedLength = m_pCompressor->compress(pSrcData, uSrcLength, pDstData, uDstLength); + uCompressedLength = compress(pSrcData, uSrcLength, pDstData, uDstLength); // Copy the resulting compressed data into the compressed block pDstBlock->setData(pDstData, uCompressedLength); @@ -62,7 +80,7 @@ namespace PolyVox // Note that ideally we will choose our earlier buffer size so that this almost never happens. logWarning() << "The compressor failed to compress the block, probabaly due to the buffer being too small."; logWarning() << "The compression will be tried again with a larger buffer"; - uint32_t uMaxCompressedSize = m_pCompressor->getMaxCompressedSize(uSrcLength); + uint32_t uMaxCompressedSize = getMaxCompressedSize(uSrcLength); uint8_t* buffer = new uint8_t[ uMaxCompressedSize ]; pDstData = reinterpret_cast( buffer ); @@ -71,7 +89,7 @@ namespace PolyVox try { // Perform the compression - uCompressedLength = m_pCompressor->compress(pSrcData, uSrcLength, pDstData, uDstLength); + uCompressedLength = compress(pSrcData, uSrcLength, pDstData, uDstLength); // Copy the resulting compressed data into the compressed block pDstBlock->setData(pDstData, uCompressedLength); @@ -98,8 +116,90 @@ namespace PolyVox //RLECompressor compressor; - uint32_t uUncompressedLength = m_pCompressor->decompress(pSrcData, uSrcLength, pDstData, uDstLength); + uint32_t uUncompressedLength = decompress(pSrcData, uSrcLength, pDstData, uDstLength); POLYVOX_ASSERT(uUncompressedLength == pDstBlock->getDataSizeInBytes(), "Destination length has changed."); } + + template + uint32_t MinizBlockCompressor::getMaxCompressedSize(uint32_t uUncompressedInputSize) + { + // The contents of this function are copied from miniz's 'mz_deflateBound()' + // (which we can't use because it is part of the zlib-style higher level API). + unsigned long source_len = uUncompressedInputSize; + + // This is really over conservative. (And lame, but it's actually pretty tricky to compute a true upper bound given the way tdefl's blocking works.) + return (std::max)(128 + (source_len * 110) / 100, 128 + source_len + ((source_len / (31 * 1024)) + 1) * 5); + } + + template + uint32_t MinizBlockCompressor::compress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) + { + //Get the deflator + tdefl_compressor* pDeflator = reinterpret_cast(m_pDeflator); + + // It seems we have to reinitialise the deflator for each fresh dataset (it's probably intended for streaming, which we're not doing here) + tdefl_status status = tdefl_init(pDeflator, NULL, NULL, m_uCompressionFlags); + if (status != TDEFL_STATUS_OKAY) + { + std::stringstream ss; + ss << "tdefl_init() failed with return code '" << status << "'"; + POLYVOX_THROW(std::runtime_error, ss.str()); + } + + // Change the type to avoid compiler warnings + size_t ulSrcLength = uSrcLength; + size_t ulDstLength = uDstLength; + + // Compress as much of the input as possible (or all of it) to the output buffer. + status = tdefl_compress(pDeflator, pSrcData, &ulSrcLength, pDstData, &ulDstLength, TDEFL_FINISH); + + //Check whther the compression was successful. + if (status != TDEFL_STATUS_DONE) + { + std::stringstream ss; + ss << "tdefl_compress() failed with return code '" << status << "'"; + POLYVOX_THROW(std::runtime_error, ss.str()); + } + + // The compression modifies 'ulDstLength' to hold the new length. + return ulDstLength; + } + + template + uint32_t MinizBlockCompressor::decompress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) + { + // I don't know exactly why this limitation exists but it's an implementation detail of miniz. It shouldn't matter for our purposes + // as our detination is a Block and those are always a power of two. If you need to use this class for other purposes then you'll + // probably have to scale up your dst buffer to the nearest appropriate size. Alternatively you can use the mz_uncompress function, + // but that means enabling parts of the miniz API which are #defined out at the top of this file. + POLYVOX_ASSERT(isPowerOf2(uDstLength), "Miniz decompressor requires the destination buffer to have a size which is a power of two."); + if(isPowerOf2(uDstLength) == false) + { + POLYVOX_THROW(std::invalid_argument, "Miniz decompressor requires the destination buffer to have a size which is a power of two."); + } + + // Change the type to avoid compiler warnings + size_t ulSrcLength = uSrcLength; + size_t ulDstLength = uDstLength; + + // Create and initialise the decompressor (I believe this is much small than the compressor). + tinfl_decompressor inflator; + tinfl_init(&inflator); + + // Do the decompression. In some scenarios 'tinfl_decompress' would be called multiple times with the same dest buffer but + // different locations within it. In our scenario it's only called once so the start and the location are the same (both pDstData). + tinfl_status status = tinfl_decompress(&inflator, (const mz_uint8 *)pSrcData, &ulSrcLength, (mz_uint8 *)pDstData, (mz_uint8 *)pDstData, &ulDstLength, TINFL_FLAG_PARSE_ZLIB_HEADER); + + //Check whther the decompression was successful. + if (status != TINFL_STATUS_DONE) + { + std::stringstream ss; + ss << "tinfl_decompress() failed with return code '" << status << "'"; + POLYVOX_THROW(std::runtime_error, ss.str()); + } + + // The decompression modifies 'ulDstLength' to hold the new length. + return ulDstLength; + } } \ No newline at end of file From 319a0ce352d1beee88f560056a5ab30f0d409835 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Tue, 1 Oct 2013 15:58:32 +0200 Subject: [PATCH 3/7] Removed old compression classes. --- library/PolyVoxCore/CMakeLists.txt | 3 - .../include/PolyVoxCore/Compressor.h | 100 ------------- .../PolyVoxCore/MinizBlockCompressor.h | 2 - .../PolyVoxCore/MinizBlockCompressor.inl | 5 + .../include/PolyVoxCore/MinizCompressor.h | 43 ------ .../include/PolyVoxCore/RLEBlockCompressor.h | 2 - .../PolyVoxCore/source/MinizCompressor.cpp | 141 ------------------ 7 files changed, 5 insertions(+), 291 deletions(-) delete mode 100644 library/PolyVoxCore/include/PolyVoxCore/Compressor.h delete mode 100644 library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h delete mode 100644 library/PolyVoxCore/source/MinizCompressor.cpp diff --git a/library/PolyVoxCore/CMakeLists.txt b/library/PolyVoxCore/CMakeLists.txt index 83a25fb9..4d4fbd43 100644 --- a/library/PolyVoxCore/CMakeLists.txt +++ b/library/PolyVoxCore/CMakeLists.txt @@ -31,7 +31,6 @@ endif() SET(CORE_SRC_FILES source/ArraySizes.cpp source/AStarPathfinder.cpp - source/MinizCompressor.cpp source/Region.cpp source/VertexTypes.cpp ) @@ -52,7 +51,6 @@ SET(CORE_INC_FILES include/PolyVoxCore/BlockCompressor.h include/PolyVoxCore/CompressedBlock.h include/PolyVoxCore/CompressedBlock.inl - include/PolyVoxCore/Compressor.h include/PolyVoxCore/CubicSurfaceExtractor.h include/PolyVoxCore/CubicSurfaceExtractor.inl include/PolyVoxCore/CubicSurfaceExtractorWithNormals.h @@ -77,7 +75,6 @@ SET(CORE_INC_FILES include/PolyVoxCore/MaterialDensityPair.h include/PolyVoxCore/MinizBlockCompressor.h include/PolyVoxCore/MinizBlockCompressor.inl - include/PolyVoxCore/MinizCompressor.h include/PolyVoxCore/Pager.h include/PolyVoxCore/PolyVoxForwardDeclarations.h include/PolyVoxCore/Picking.h diff --git a/library/PolyVoxCore/include/PolyVoxCore/Compressor.h b/library/PolyVoxCore/include/PolyVoxCore/Compressor.h deleted file mode 100644 index e85610de..00000000 --- a/library/PolyVoxCore/include/PolyVoxCore/Compressor.h +++ /dev/null @@ -1,100 +0,0 @@ -/******************************************************************************* -Copyright (c) 2005-2009 David Williams - -This software is provided 'as-is', without any express or implied -warranty. In no event will the authors be held liable for any damages -arising from the use of this software. - -Permission is granted to anyone to use this software for any purpose, -including commercial applications, and to alter it and redistribute it -freely, subject to the following restrictions: - - 1. The origin of this software must not be misrepresented; you must not - claim that you wrote the original software. If you use this software - in a product, an acknowledgment in the product documentation would be - appreciated but is not required. - - 2. Altered source versions must be plainly marked as such, and must not be - misrepresented as being the original software. - - 3. This notice may not be removed or altered from any source - distribution. -*******************************************************************************/ - -#ifndef __PolyVox_Compressor_H__ -#define __PolyVox_Compressor_H__ - -#include "PolyVoxCore/Impl/TypeDef.h" - -namespace PolyVox -{ - /** - * Provides an interface for performing compression of data. - * - * This class provides an interface which can be implemented by derived classes which perform data compression. - * The main purpose of this is to allow the user to change the compression algorithm which is used by a LargeVolume, - * based on the kind of voxel data it is storing. Users may also wish to use Compressor subclasses in more general - * compression-related scenarios but this is not well tested. - * - * If you wish to implement your own compression algorithms for use in PolyVox then you should begin by subclassing this class. - * - * \sa MinizCompressor, RLECompressor - */ - class Compressor - { - public: - /// Constructor - Compressor() {}; - /// Destructor - virtual ~Compressor() {}; - - /** - * Computes a worst-case scenario for how big the output can be for a given input size. - * - * If necessary you can use this as a destination buffer size, though it may be somewhat - * wasteful. It is not guarenteed that compression actually shrinks the data, so the - * worst-case value returned by this function may be bigger than the input size. - * - * \param uUncompressedInputSize The size of the uncompressed input data - * \return The largest possible size of the compressed output data. - */ - virtual uint32_t getMaxCompressedSize(uint32_t uUncompressedInputSize) = 0; - - /** - * Compresses the data. - * - * Performs compression of the data pointed to by pSrcData and stores the result in pDstData. - * The user is responsible for allocating both buffers and for making sure that the destination - * buffer is large enough to hold the result. If you don't know how big the compressed data - * will be (and you probably won't know this) then you can call getMaxCompressedSize() to get - * an upper bound. The *actual* compressed size is then returned by this function and you can - * use this to copy your compressed data to a more suitably size buffer. - * - * \param pSrcData A pointer to the data to be compressed. - * \param uSrcLength The length of the data to be compressed. - * \param pDstData A pointer to the memory where the result should be stored. - * \param uDstLength The length of the destination buffer (compression will fail if this isn't big enough). - * \return The size of the resulting compressed data. - */ - virtual uint32_t compress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) = 0; - - /** - * Decompresses the data. - * - * Performs decompression of the data pointed to by pSrcData and stores the result in pDstData. - * The user is responsible for allocating both buffers and for making sure that the destination - * buffer is large enough to hold the result. This means you need to know how large the resulting - * data might be, so before you compress the data it may be worth storing this information somewhere. - * The *actual* decompressed size is then returned by this function - * - * \param pSrcData A pointer to the data to be decompressed. - * \param uSrcLength The length of the data to be decompressed. - * \param pDstData A pointer to the memory where the result should be stored. - * \param uDstLength The length of the destination buffer (decompression will fail if this isn't big enough). - * \return The size of the resulting uncompressed data. - */ - virtual uint32_t decompress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) = 0; - }; -} - -#endif //__PolyVox_Compressor_H__ diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h index 57698a34..dafcfc7e 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h @@ -51,8 +51,6 @@ namespace PolyVox unsigned int m_uCompressionFlags; // tdefl_compressor contains all the state needed by the low-level compressor so it's a pretty big struct (~300k). - // We're storing it by void* because miniz does not supply a header and we don't want to include the .c file from - // here as it will cause linker problems. tdefl_compressor* m_pDeflator; }; } diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl index 59ad7708..b204e2c2 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl +++ b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl @@ -25,6 +25,11 @@ freely, subject to the following restrictions: namespace PolyVox { + /** + * You can specify a compression level when constructing this compressor. This controls the tradeoff between speed and compression + * rate. Levels 0-9 are the standard zlib-style levels, 10 is best possible compression (not zlib compatible, and may be very slow). + * \param iCompressionLevel The desired compression level. + */ template MinizBlockCompressor::MinizBlockCompressor(int iCompressionLevel) :m_pDeflator(0) diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h b/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h deleted file mode 100644 index 2f445387..00000000 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h +++ /dev/null @@ -1,43 +0,0 @@ -#ifndef __PolyVox_MinizCompressor_H__ -#define __PolyVox_MinizCompressor_H__ - -#include "PolyVoxCore/Compressor.h" - -#include "PolyVoxCore/Impl/MinizWrapper.h" - -namespace PolyVox -{ - /** - * Performs compression of data using the miniz library. - * - * This compressor implements the DEFLATE (http://en.wikipedia.org/wiki/Deflate) compression algorithm via the pubic domain - * 'miniz' library (https://code.google.com/p/miniz/). This is a general purpose compression algorithm, and within PolyVox it - * is intended for situations in which the alternative RLECompressor is not appropriate. It is a good default choice if you - * are not sure which compressor is best for your needs. - * - * \sa RLECompressor - */ - class MinizCompressor : public Compressor - { - public: - /// Constructor - MinizCompressor(int iCompressionLevel = 6); // Miniz defines MZ_DEFAULT_LEVEL = 6 so we use the same here - /// Destructor - ~MinizCompressor(); - - // API documentation is in base class and gets inherited by Doxygen. - uint32_t getMaxCompressedSize(uint32_t uUncompressedInputSize); - uint32_t compress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); - uint32_t decompress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); - - private: - unsigned int m_uCompressionFlags; - - // tdefl_compressor contains all the state needed by the low-level compressor so it's a pretty big struct (~300k). - // We're storing it by void* because miniz does not supply a header and we don't want to include the .c file from - // here as it will cause linker problems. - tdefl_compressor* m_pDeflator; - }; -} - -#endif //__PolyVox_MinizCompressor_H__ diff --git a/library/PolyVoxCore/include/PolyVoxCore/RLEBlockCompressor.h b/library/PolyVoxCore/include/PolyVoxCore/RLEBlockCompressor.h index fd77522f..ad388b6e 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/RLEBlockCompressor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/RLEBlockCompressor.h @@ -26,8 +26,6 @@ freely, subject to the following restrictions: #include "PolyVoxCore/BlockCompressor.h" -#include "PolyVoxCore/MinizCompressor.h" - namespace PolyVox { template diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp deleted file mode 100644 index d2b127d3..00000000 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ /dev/null @@ -1,141 +0,0 @@ -#include "PolyVoxCore/MinizCompressor.h" - -#include "PolyVoxCore/Impl/Utility.h" - -// Diable things we don't need, and in particular the zlib compatible names which -// would cause conflicts if a user application is using both PolyVox and zlib. -/*#define MINIZ_NO_STDIO -#define MINIZ_NO_ARCHIVE_APIS -#define MINIZ_NO_TIME -#define MINIZ_NO_ZLIB_APIS -#define MINIZ_NO_ZLIB_COMPATIBLE_NAMES -#define MINIZ_NO_MALLOC*/ - -#include "PolyVoxCore/Impl/ErrorHandling.h" - -// The miniz library is supplied only as a single .c file without a header. The examples just include the .c file -// directly which is also what we do here. Actually is is possible to define 'MINIZ_HEADER_FILE_ONLY' to treat -// the .c file as a header, but this seems messy in terms of our project and CMake as we keep the headers and source -// files in seperate folders. We could create our own header for miniz (based on the stuff between the MINIZ_HEADER_FILE_ONLY -// directives) but the other problem is that we are using #pragma GCC system_header to supress warnings which would -// then be in the .c part of the code. If we ever update GCC on the CDash machine so that it properly supports '#pragma -// GCC diagnosic ignored' (or so that it doesn't warn in the first place) then we can reconsider spliting miniz.c in two. -//#include "PolyVoxCore/Impl/miniz.c" - - -#include - -using namespace std; - -namespace PolyVox -{ - /** - * You can specify a compression level when constructing this compressor. This controls the tradeoff between speed and compression - * rate. Levels 0-9 are the standard zlib-style levels, 10 is best possible compression (not zlib compatible, and may be very slow). - * \param iCompressionLevel The desired compression level. - */ - MinizCompressor::MinizCompressor(int iCompressionLevel) - :m_pDeflator(0) - { - // Create and store the deflator. - tdefl_compressor* pDeflator = new tdefl_compressor; - m_pDeflator = /*reinterpret_cast*/(pDeflator); - - // The number of dictionary probes to use at each compression level (0-10). 0=implies fastest/minimal possible probing. - // The discontinuity is unsettling but may be explained by the 'iCompressionLevel <= 3' check later? - static const mz_uint s_tdefl_num_probes[11] = { 0, 1, 6, 32, 16, 32, 128, 256, 512, 768, 1500 }; - - // Create tdefl() compatible flags (we have to compose the low-level flags ourselves, or use tdefl_create_comp_flags_from_zip_params() but that means MINIZ_NO_ZLIB_APIS can't be defined). - m_uCompressionFlags = TDEFL_WRITE_ZLIB_HEADER | s_tdefl_num_probes[(std::min)(10, iCompressionLevel)] | ((iCompressionLevel <= 3) ? TDEFL_GREEDY_PARSING_FLAG : 0); - if (!iCompressionLevel) - { - m_uCompressionFlags |= TDEFL_FORCE_ALL_RAW_BLOCKS; - } - } - - MinizCompressor::~MinizCompressor() - { - // Delete the deflator - tdefl_compressor* pDeflator = /*reinterpret_cast*/(m_pDeflator); - delete pDeflator; - } - - uint32_t MinizCompressor::getMaxCompressedSize(uint32_t uUncompressedInputSize) - { - // The contents of this function are copied from miniz's 'mz_deflateBound()' - // (which we can't use because it is part of the zlib-style higher level API). - unsigned long source_len = uUncompressedInputSize; - - // This is really over conservative. (And lame, but it's actually pretty tricky to compute a true upper bound given the way tdefl's blocking works.) - return (std::max)(128 + (source_len * 110) / 100, 128 + source_len + ((source_len / (31 * 1024)) + 1) * 5); - } - - uint32_t MinizCompressor::compress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) - { - //Get the deflator - tdefl_compressor* pDeflator = reinterpret_cast(m_pDeflator); - - // It seems we have to reinitialise the deflator for each fresh dataset (it's probably intended for streaming, which we're not doing here) - tdefl_status status = tdefl_init(pDeflator, NULL, NULL, m_uCompressionFlags); - if (status != TDEFL_STATUS_OKAY) - { - stringstream ss; - ss << "tdefl_init() failed with return code '" << status << "'"; - POLYVOX_THROW(std::runtime_error, ss.str()); - } - - // Change the type to avoid compiler warnings - size_t ulSrcLength = uSrcLength; - size_t ulDstLength = uDstLength; - - // Compress as much of the input as possible (or all of it) to the output buffer. - status = tdefl_compress(pDeflator, pSrcData, &ulSrcLength, pDstData, &ulDstLength, TDEFL_FINISH); - - //Check whther the compression was successful. - if (status != TDEFL_STATUS_DONE) - { - stringstream ss; - ss << "tdefl_compress() failed with return code '" << status << "'"; - POLYVOX_THROW(std::runtime_error, ss.str()); - } - - // The compression modifies 'ulDstLength' to hold the new length. - return ulDstLength; - } - - uint32_t MinizCompressor::decompress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) - { - // I don't know exactly why this limitation exists but it's an implementation detail of miniz. It shouldn't matter for our purposes - // as our detination is a Block and those are always a power of two. If you need to use this class for other purposes then you'll - // probably have to scale up your dst buffer to the nearest appropriate size. Alternatively you can use the mz_uncompress function, - // but that means enabling parts of the miniz API which are #defined out at the top of this file. - POLYVOX_ASSERT(isPowerOf2(uDstLength), "Miniz decompressor requires the destination buffer to have a size which is a power of two."); - if(isPowerOf2(uDstLength) == false) - { - POLYVOX_THROW(std::invalid_argument, "Miniz decompressor requires the destination buffer to have a size which is a power of two."); - } - - // Change the type to avoid compiler warnings - size_t ulSrcLength = uSrcLength; - size_t ulDstLength = uDstLength; - - // Create and initialise the decompressor (I believe this is much small than the compressor). - tinfl_decompressor inflator; - tinfl_init(&inflator); - - // Do the decompression. In some scenarios 'tinfl_decompress' would be called multiple times with the same dest buffer but - // different locations within it. In our scenario it's only called once so the start and the location are the same (both pDstData). - tinfl_status status = tinfl_decompress(&inflator, (const mz_uint8 *)pSrcData, &ulSrcLength, (mz_uint8 *)pDstData, (mz_uint8 *)pDstData, &ulDstLength, TINFL_FLAG_PARSE_ZLIB_HEADER); - - //Check whther the decompression was successful. - if (status != TINFL_STATUS_DONE) - { - stringstream ss; - ss << "tinfl_decompress() failed with return code '" << status << "'"; - POLYVOX_THROW(std::runtime_error, ss.str()); - } - - // The decompression modifies 'ulDstLength' to hold the new length. - return ulDstLength; - } -} From 513c3a90b09f5d9b3828d4180a3ce8ce07c5e9a7 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Tue, 1 Oct 2013 16:33:39 +0200 Subject: [PATCH 4/7] Rearranging some miniz code to simplify it a bit. --- .../PolyVoxCore/MinizBlockCompressor.inl | 41 ++++++++----------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl index b204e2c2..49ae7b74 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl +++ b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl @@ -35,8 +35,7 @@ namespace PolyVox :m_pDeflator(0) { // Create and store the deflator. - tdefl_compressor* pDeflator = new tdefl_compressor; - m_pDeflator = /*reinterpret_cast*/(pDeflator); + m_pDeflator = new tdefl_compressor; // The number of dictionary probes to use at each compression level (0-10). 0=implies fastest/minimal possible probing. // The discontinuity is unsettling but may be explained by the 'iCompressionLevel <= 3' check later? @@ -54,19 +53,27 @@ namespace PolyVox MinizBlockCompressor::~MinizBlockCompressor() { // Delete the deflator - tdefl_compressor* pDeflator = /*reinterpret_cast*/(m_pDeflator); - delete pDeflator; + delete m_pDeflator; } template void MinizBlockCompressor::compress(UncompressedBlock* pSrcBlock, CompressedBlock* pDstBlock) { + // It seems we have to reinitialise the deflator for each fresh dataset (it's probably intended for streaming, which we're not doing here) + tdefl_status status = tdefl_init(m_pDeflator, NULL, NULL, m_uCompressionFlags); + if (status != TDEFL_STATUS_OKAY) + { + std::stringstream ss; + ss << "tdefl_init() failed with return code '" << status << "'"; + POLYVOX_THROW(std::runtime_error, ss.str()); + } + void* pSrcData = reinterpret_cast(pSrcBlock->getData()); - uint32_t uSrcLength = pSrcBlock->getDataSizeInBytes(); + size_t uSrcLength = pSrcBlock->getDataSizeInBytes(); uint8_t tempBuffer[10000]; uint8_t* pDstData = reinterpret_cast( tempBuffer ); - uint32_t uDstLength = 10000; + size_t uDstLength = 10000; uint32_t uCompressedLength = 0; @@ -138,26 +145,10 @@ namespace PolyVox } template - uint32_t MinizBlockCompressor::compress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) + uint32_t MinizBlockCompressor::compress(const void* pSrcData, size_t uSrcLength, void* pDstData, size_t uDstLength) { - //Get the deflator - tdefl_compressor* pDeflator = reinterpret_cast(m_pDeflator); - - // It seems we have to reinitialise the deflator for each fresh dataset (it's probably intended for streaming, which we're not doing here) - tdefl_status status = tdefl_init(pDeflator, NULL, NULL, m_uCompressionFlags); - if (status != TDEFL_STATUS_OKAY) - { - std::stringstream ss; - ss << "tdefl_init() failed with return code '" << status << "'"; - POLYVOX_THROW(std::runtime_error, ss.str()); - } - - // Change the type to avoid compiler warnings - size_t ulSrcLength = uSrcLength; - size_t ulDstLength = uDstLength; - // Compress as much of the input as possible (or all of it) to the output buffer. - status = tdefl_compress(pDeflator, pSrcData, &ulSrcLength, pDstData, &ulDstLength, TDEFL_FINISH); + tdefl_status status = tdefl_compress(m_pDeflator, pSrcData, &uSrcLength, pDstData, &uDstLength, TDEFL_FINISH); //Check whther the compression was successful. if (status != TDEFL_STATUS_DONE) @@ -168,7 +159,7 @@ namespace PolyVox } // The compression modifies 'ulDstLength' to hold the new length. - return ulDstLength; + return uDstLength; } template From 29ca1e763f32aff69dd1f7591503f430ce0fa3df Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Wed, 2 Oct 2013 15:09:55 +0200 Subject: [PATCH 5/7] Renamed functions to avoid confusion. --- .../include/PolyVoxCore/MinizBlockCompressor.h | 4 ++-- .../include/PolyVoxCore/MinizBlockCompressor.inl | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h index dafcfc7e..577cf782 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h @@ -45,8 +45,8 @@ namespace PolyVox private: uint32_t getMaxCompressedSize(uint32_t uUncompressedInputSize); - uint32_t compress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); - uint32_t decompress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); + uint32_t compressWithMiniz(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); + uint32_t decompressWithMiniz(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); unsigned int m_uCompressionFlags; diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl index 49ae7b74..cf3ff800 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl +++ b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl @@ -80,7 +80,7 @@ namespace PolyVox try { // Perform the compression - uCompressedLength = compress(pSrcData, uSrcLength, pDstData, uDstLength); + uCompressedLength = compressWithMiniz(pSrcData, uSrcLength, pDstData, uDstLength); // Copy the resulting compressed data into the compressed block pDstBlock->setData(pDstData, uCompressedLength); @@ -101,7 +101,7 @@ namespace PolyVox try { // Perform the compression - uCompressedLength = compress(pSrcData, uSrcLength, pDstData, uDstLength); + uCompressedLength = compressWithMiniz(pSrcData, uSrcLength, pDstData, uDstLength); // Copy the resulting compressed data into the compressed block pDstBlock->setData(pDstData, uCompressedLength); @@ -128,7 +128,7 @@ namespace PolyVox //RLECompressor compressor; - uint32_t uUncompressedLength = decompress(pSrcData, uSrcLength, pDstData, uDstLength); + uint32_t uUncompressedLength = decompressWithMiniz(pSrcData, uSrcLength, pDstData, uDstLength); POLYVOX_ASSERT(uUncompressedLength == pDstBlock->getDataSizeInBytes(), "Destination length has changed."); } @@ -145,7 +145,7 @@ namespace PolyVox } template - uint32_t MinizBlockCompressor::compress(const void* pSrcData, size_t uSrcLength, void* pDstData, size_t uDstLength) + uint32_t MinizBlockCompressor::compressWithMiniz(const void* pSrcData, size_t uSrcLength, void* pDstData, size_t uDstLength) { // Compress as much of the input as possible (or all of it) to the output buffer. tdefl_status status = tdefl_compress(m_pDeflator, pSrcData, &uSrcLength, pDstData, &uDstLength, TDEFL_FINISH); @@ -163,7 +163,7 @@ namespace PolyVox } template - uint32_t MinizBlockCompressor::decompress(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) + uint32_t MinizBlockCompressor::decompressWithMiniz(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) { // I don't know exactly why this limitation exists but it's an implementation detail of miniz. It shouldn't matter for our purposes // as our detination is a Block and those are always a power of two. If you need to use this class for other purposes then you'll From a4e09c24815c8eabd16fc8304651b7d406ff58f1 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Wed, 2 Oct 2013 15:36:21 +0200 Subject: [PATCH 6/7] Small fixes (including crash fix). --- .../PolyVoxCore/MinizBlockCompressor.inl | 45 +++++++++---------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl index cf3ff800..fbba8856 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl +++ b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl @@ -59,21 +59,12 @@ namespace PolyVox template void MinizBlockCompressor::compress(UncompressedBlock* pSrcBlock, CompressedBlock* pDstBlock) { - // It seems we have to reinitialise the deflator for each fresh dataset (it's probably intended for streaming, which we're not doing here) - tdefl_status status = tdefl_init(m_pDeflator, NULL, NULL, m_uCompressionFlags); - if (status != TDEFL_STATUS_OKAY) - { - std::stringstream ss; - ss << "tdefl_init() failed with return code '" << status << "'"; - POLYVOX_THROW(std::runtime_error, ss.str()); - } - void* pSrcData = reinterpret_cast(pSrcBlock->getData()); size_t uSrcLength = pSrcBlock->getDataSizeInBytes(); - uint8_t tempBuffer[10000]; + uint8_t tempBuffer[1000]; uint8_t* pDstData = reinterpret_cast( tempBuffer ); - size_t uDstLength = 10000; + size_t uDstLength = 1000; uint32_t uCompressedLength = 0; @@ -121,16 +112,17 @@ namespace PolyVox template void MinizBlockCompressor::decompress(CompressedBlock* pSrcBlock, UncompressedBlock* pDstBlock) { + // Get raw pointers so that the data can be decompressed directly into the destination block. const void* pSrcData = reinterpret_cast(pSrcBlock->getData()); void* pDstData = reinterpret_cast(pDstBlock->getData()); uint32_t uSrcLength = pSrcBlock->getDataSizeInBytes(); uint32_t uDstLength = pDstBlock->getDataSizeInBytes(); - - //RLECompressor compressor; + // Perform the decompression uint32_t uUncompressedLength = decompressWithMiniz(pSrcData, uSrcLength, pDstData, uDstLength); - POLYVOX_ASSERT(uUncompressedLength == pDstBlock->getDataSizeInBytes(), "Destination length has changed."); + // We know we should have received exactly one block's worth of data. If not then something went wrong. + POLYVOX_THROW_IF(uUncompressedLength != pDstBlock->getDataSizeInBytes(), std::runtime_error, "Decompressed data does not have the expected length"); } template @@ -147,10 +139,19 @@ namespace PolyVox template uint32_t MinizBlockCompressor::compressWithMiniz(const void* pSrcData, size_t uSrcLength, void* pDstData, size_t uDstLength) { - // Compress as much of the input as possible (or all of it) to the output buffer. - tdefl_status status = tdefl_compress(m_pDeflator, pSrcData, &uSrcLength, pDstData, &uDstLength, TDEFL_FINISH); + // It seems we have to reinitialise the deflator for each fresh dataset (it's probably intended for streaming, which we're not doing here) + tdefl_status status = tdefl_init(m_pDeflator, NULL, NULL, m_uCompressionFlags); + if (status != TDEFL_STATUS_OKAY) + { + std::stringstream ss; + ss << "tdefl_init() failed with return code '" << status << "'"; + POLYVOX_THROW(std::runtime_error, ss.str()); + } - //Check whther the compression was successful. + // Compress as much of the input as possible (or all of it) to the output buffer. + status = tdefl_compress(m_pDeflator, pSrcData, &uSrcLength, pDstData, &uDstLength, TDEFL_FINISH); + + //Check whether the compression was successful. if (status != TDEFL_STATUS_DONE) { std::stringstream ss; @@ -166,14 +167,10 @@ namespace PolyVox uint32_t MinizBlockCompressor::decompressWithMiniz(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) { // I don't know exactly why this limitation exists but it's an implementation detail of miniz. It shouldn't matter for our purposes - // as our detination is a Block and those are always a power of two. If you need to use this class for other purposes then you'll + // as our destination is a Block and those are always a power of two. If you need to use this code for other purposes then you'll // probably have to scale up your dst buffer to the nearest appropriate size. Alternatively you can use the mz_uncompress function, // but that means enabling parts of the miniz API which are #defined out at the top of this file. - POLYVOX_ASSERT(isPowerOf2(uDstLength), "Miniz decompressor requires the destination buffer to have a size which is a power of two."); - if(isPowerOf2(uDstLength) == false) - { - POLYVOX_THROW(std::invalid_argument, "Miniz decompressor requires the destination buffer to have a size which is a power of two."); - } + POLYVOX_THROW_IF(isPowerOf2(uDstLength) == false, std::invalid_argument, "Miniz decompressor requires the destination buffer to have a size which is a power of two."); // Change the type to avoid compiler warnings size_t ulSrcLength = uSrcLength; @@ -187,7 +184,7 @@ namespace PolyVox // different locations within it. In our scenario it's only called once so the start and the location are the same (both pDstData). tinfl_status status = tinfl_decompress(&inflator, (const mz_uint8 *)pSrcData, &ulSrcLength, (mz_uint8 *)pDstData, (mz_uint8 *)pDstData, &ulDstLength, TINFL_FLAG_PARSE_ZLIB_HEADER); - //Check whther the decompression was successful. + //Check whether the decompression was successful. if (status != TINFL_STATUS_DONE) { std::stringstream ss; From 498f21f63fb89a39c17702a5bae230123a33ca34 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Wed, 2 Oct 2013 16:48:30 +0200 Subject: [PATCH 7/7] Replaced arrays with std::vector. --- .../PolyVoxCore/MinizBlockCompressor.h | 5 +++ .../PolyVoxCore/MinizBlockCompressor.inl | 31 ++++++++++++------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h index 577cf782..04a7438f 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.h @@ -44,12 +44,17 @@ namespace PolyVox void decompress(CompressedBlock* pSrcBlock, UncompressedBlock* pDstBlock); private: + uint32_t getExpectedCompressedSize(uint32_t uUncompressedInputSize); uint32_t getMaxCompressedSize(uint32_t uUncompressedInputSize); uint32_t compressWithMiniz(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); uint32_t decompressWithMiniz(const void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); unsigned int m_uCompressionFlags; + // Data gets compressed into this, then we check how big the result + // is and copy the required number of bytes to the destination block. + std::vector m_vecTempBuffer; + // tdefl_compressor contains all the state needed by the low-level compressor so it's a pretty big struct (~300k). tdefl_compressor* m_pDeflator; }; diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl index fbba8856..c820ec34 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl +++ b/library/PolyVoxCore/include/PolyVoxCore/MinizBlockCompressor.inl @@ -62,9 +62,11 @@ namespace PolyVox void* pSrcData = reinterpret_cast(pSrcBlock->getData()); size_t uSrcLength = pSrcBlock->getDataSizeInBytes(); - uint8_t tempBuffer[1000]; - uint8_t* pDstData = reinterpret_cast( tempBuffer ); - size_t uDstLength = 1000; + // This compressor is expected to be used many times to compress a large number of blocks, but they are all + // expected to have the same size. Therefore the resize() function below will only perform allocation once. + m_vecTempBuffer.resize(getExpectedCompressedSize(uSrcLength)); + uint8_t* pDstData = &(m_vecTempBuffer[0]); + size_t uDstLength = m_vecTempBuffer.size(); uint32_t uCompressedLength = 0; @@ -82,12 +84,13 @@ namespace PolyVox // buffer is not big enough. So now we try again using a buffer that is definitely big enough. // Note that ideally we will choose our earlier buffer size so that this almost never happens. logWarning() << "The compressor failed to compress the block, probabaly due to the buffer being too small."; - logWarning() << "The compression will be tried again with a larger buffer"; - uint32_t uMaxCompressedSize = getMaxCompressedSize(uSrcLength); - uint8_t* buffer = new uint8_t[ uMaxCompressedSize ]; + logWarning() << "The compression will be tried again with a larger buffer."; - pDstData = reinterpret_cast( buffer ); - uDstLength = uMaxCompressedSize; + std::vector vecExtraBigBuffer; + vecExtraBigBuffer.resize(getMaxCompressedSize(uSrcLength)); + + uint8_t* pDstData = &(vecExtraBigBuffer[0]); + size_t uDstLength = vecExtraBigBuffer.size(); try { @@ -101,11 +104,8 @@ namespace PolyVox { // At this point it didn't work even with a bigger buffer. // Not much more we can do so just rethrow the exception. - delete[] buffer; POLYVOX_THROW(std::runtime_error, "Failed to compress block data"); } - - delete[] buffer; } } @@ -125,6 +125,15 @@ namespace PolyVox POLYVOX_THROW_IF(uUncompressedLength != pDstBlock->getDataSizeInBytes(), std::runtime_error, "Decompressed data does not have the expected length"); } + template + uint32_t MinizBlockCompressor::getExpectedCompressedSize(uint32_t uUncompressedInputSize) + { + // We expect this block compressor will be used for smoothly changing volume data such as density fields and so + // the compression rate might not be great. The value beloew is basically a guess based on previous experience. + uint32_t uExpectedCompressionRate = 4; + return uUncompressedInputSize / uExpectedCompressionRate; + } + template uint32_t MinizBlockCompressor::getMaxCompressedSize(uint32_t uUncompressedInputSize) {