From 1a39577ec0e08a81e5687c3b83f3a75de32d26d0 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Thu, 21 Feb 2013 16:44:44 +0100 Subject: [PATCH 01/19] CubicSurfaceExtractor now uses int for some internal work instead of floats. --- .../PolyVoxCore/CubicSurfaceExtractor.h | 2 +- .../PolyVoxCore/CubicSurfaceExtractor.inl | 57 +++++++++---------- 2 files changed, 28 insertions(+), 31 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.h b/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.h index d3e0a2f1..b77c83c2 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.h @@ -122,7 +122,7 @@ namespace PolyVox void execute(); private: - int32_t addVertex(float fX, float fY, float fZ, uint32_t uMaterial, Array<3, IndexAndMaterial>& existingVertices); + int32_t addVertex(uint32_t uX, uint32_t uY, uint32_t uZ, uint32_t uMaterial, Array<3, IndexAndMaterial>& existingVertices); bool performQuadMerging(std::list& quads); bool mergeQuads(Quad& q1, Quad& q2); diff --git a/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.inl b/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.inl index 1f3ee2cc..0b0ed9eb 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.inl +++ b/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.inl @@ -94,20 +94,20 @@ namespace PolyVox // X if(m_funcIsQuadNeededCallback(currentVoxel, negXVoxel, material)) { - uint32_t v0 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); - uint32_t v1 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) + 0.5f, material, m_currentSliceVertices); - uint32_t v2 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) + 0.5f, static_cast(regZ) + 0.5f, material, m_currentSliceVertices); - uint32_t v3 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) + 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); + uint32_t v0 = addVertex(regX , regY , regZ , material, m_previousSliceVertices); + uint32_t v1 = addVertex(regX , regY , regZ + 1, material, m_currentSliceVertices); + uint32_t v2 = addVertex(regX , regY + 1, regZ + 1, material, m_currentSliceVertices); + uint32_t v3 = addVertex(regX , regY + 1, regZ , material, m_previousSliceVertices); m_vecQuads[NegativeX][regX].push_back(Quad(v0, v1, v2, v3)); } if(m_funcIsQuadNeededCallback(negXVoxel, currentVoxel, material)) { - uint32_t v0 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); - uint32_t v1 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) + 0.5f, material, m_currentSliceVertices); - uint32_t v2 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) + 0.5f, static_cast(regZ) + 0.5f, material, m_currentSliceVertices); - uint32_t v3 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) + 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); + uint32_t v0 = addVertex(regX , regY , regZ , material, m_previousSliceVertices); + uint32_t v1 = addVertex(regX , regY , regZ + 1, material, m_currentSliceVertices); + uint32_t v2 = addVertex(regX , regY + 1, regZ + 1, material, m_currentSliceVertices); + uint32_t v3 = addVertex(regX , regY + 1, regZ , material, m_previousSliceVertices); m_vecQuads[PositiveX][regX].push_back(Quad(v0, v3, v2, v1)); } @@ -115,20 +115,20 @@ namespace PolyVox // Y if(m_funcIsQuadNeededCallback(currentVoxel, negYVoxel, material)) { - uint32_t v0 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); - uint32_t v1 = addVertex(static_cast(regX) + 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); - uint32_t v2 = addVertex(static_cast(regX) + 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) + 0.5f, material, m_currentSliceVertices); - uint32_t v3 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) + 0.5f, material, m_currentSliceVertices); + uint32_t v0 = addVertex(regX , regY , regZ , material, m_previousSliceVertices); + uint32_t v1 = addVertex(regX + 1, regY , regZ , material, m_previousSliceVertices); + uint32_t v2 = addVertex(regX + 1, regY , regZ + 1, material, m_currentSliceVertices); + uint32_t v3 = addVertex(regX , regY , regZ + 1, material, m_currentSliceVertices); m_vecQuads[NegativeY][regY].push_back(Quad(v0, v1, v2, v3)); } if(m_funcIsQuadNeededCallback(negYVoxel, currentVoxel, material)) { - uint32_t v0 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); - uint32_t v1 = addVertex(static_cast(regX) + 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); - uint32_t v2 = addVertex(static_cast(regX) + 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) + 0.5f, material, m_currentSliceVertices); - uint32_t v3 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) + 0.5f, material, m_currentSliceVertices); + uint32_t v0 = addVertex(regX , regY , regZ , material, m_previousSliceVertices); + uint32_t v1 = addVertex(regX + 1, regY , regZ , material, m_previousSliceVertices); + uint32_t v2 = addVertex(regX + 1, regY , regZ + 1, material, m_currentSliceVertices); + uint32_t v3 = addVertex(regX , regY , regZ + 1, material, m_currentSliceVertices); m_vecQuads[PositiveY][regY].push_back(Quad(v0, v3, v2, v1)); } @@ -136,20 +136,20 @@ namespace PolyVox // Z if(m_funcIsQuadNeededCallback(currentVoxel, negZVoxel, material)) { - uint32_t v0 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); - uint32_t v1 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) + 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); - uint32_t v2 = addVertex(static_cast(regX) + 0.5f, static_cast(regY) + 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); - uint32_t v3 = addVertex(static_cast(regX) + 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); + uint32_t v0 = addVertex(regX , regY , regZ , material, m_previousSliceVertices); + uint32_t v1 = addVertex(regX , regY + 1, regZ , material, m_previousSliceVertices); + uint32_t v2 = addVertex(regX + 1, regY + 1, regZ , material, m_previousSliceVertices); + uint32_t v3 = addVertex(regX + 1, regY , regZ , material, m_previousSliceVertices); m_vecQuads[NegativeZ][regZ].push_back(Quad(v0, v1, v2, v3)); } if(m_funcIsQuadNeededCallback(negZVoxel, currentVoxel, material)) { - uint32_t v0 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); - uint32_t v1 = addVertex(static_cast(regX) - 0.5f, static_cast(regY) + 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); - uint32_t v2 = addVertex(static_cast(regX) + 0.5f, static_cast(regY) + 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); - uint32_t v3 = addVertex(static_cast(regX) + 0.5f, static_cast(regY) - 0.5f, static_cast(regZ) - 0.5f, material, m_previousSliceVertices); + uint32_t v0 = addVertex(regX , regY , regZ , material, m_previousSliceVertices); + uint32_t v1 = addVertex(regX , regY + 1, regZ , material, m_previousSliceVertices); + uint32_t v2 = addVertex(regX + 1, regY + 1, regZ , material, m_previousSliceVertices); + uint32_t v3 = addVertex(regX + 1, regY , regZ , material, m_previousSliceVertices); m_vecQuads[PositiveZ][regZ].push_back(Quad(v0, v3, v2, v1)); } @@ -198,19 +198,16 @@ namespace PolyVox } template - int32_t CubicSurfaceExtractor::addVertex(float fX, float fY, float fZ, uint32_t uMaterialIn, Array<3, IndexAndMaterial>& existingVertices) + int32_t CubicSurfaceExtractor::addVertex(uint32_t uX, uint32_t uY, uint32_t uZ, uint32_t uMaterialIn, Array<3, IndexAndMaterial>& existingVertices) { - uint32_t uX = static_cast(fX + 0.75f); - uint32_t uY = static_cast(fY + 0.75f); - for(uint32_t ct = 0; ct < MaxVerticesPerPosition; ct++) { IndexAndMaterial& rEntry = existingVertices[uX][uY][ct]; if(rEntry.iIndex == -1) { - //No vertices matched and we've now hit an empty space. Fill it by creating a vertex. - rEntry.iIndex = m_meshCurrent->addVertex(PositionMaterial(Vector3DFloat(fX, fY, fZ), uMaterialIn)); + //No vertices matched and we've now hit an empty space. Fill it by creating a vertex. The 0.5f offset is because vertices set between voxels in order to build cubes around them. + rEntry.iIndex = m_meshCurrent->addVertex(PositionMaterial(Vector3DFloat(static_cast(uX) - 0.5f, static_cast(uY) - 0.5f, static_cast(uZ) - 0.5f), uMaterialIn)); rEntry.uMaterial = uMaterialIn; return rEntry.iIndex; From eb8ace0c54fa992cab861e568c62ce7484526d97 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Thu, 21 Feb 2013 16:56:57 +0100 Subject: [PATCH 02/19] Replaced high level miniz interface with low-level version. --- .../PolyVoxCore/source/MinizCompressor.cpp | 70 ++++++++++++++++++- 1 file changed, 68 insertions(+), 2 deletions(-) diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index 2d749a54..0c3496d8 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -34,7 +34,7 @@ namespace PolyVox return static_cast(mz_compressBound(static_cast(uUncompressedInputSize))); } - uint32_t MinizCompressor::compress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) + /*uint32_t MinizCompressor::compress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) { mz_ulong ulDstLength = uDstLength; @@ -48,10 +48,50 @@ namespace PolyVox } // Return the number of bytes written to the output. + return ulDstLength; + }*/ + + uint32_t MinizCompressor::compress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) + { + int level = 9; + + // tdefl_compressor contains all the state needed by the low-level compressor so it's a pretty big struct (~300k). + // This example makes it a global vs. putting it on the stack, of course in real-world usage you'll probably malloc() or new it. + tdefl_compressor g_deflator; + + // The number of dictionary probes to use at each compression level (0-10). 0=implies fastest/minimal possible probing. + 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). + mz_uint comp_flags = TDEFL_WRITE_ZLIB_HEADER | s_tdefl_num_probes[MZ_MIN(10, level)] | ((level <= 3) ? TDEFL_GREEDY_PARSING_FLAG : 0); + if (!level) + comp_flags |= TDEFL_FORCE_ALL_RAW_BLOCKS; + + tdefl_status status = tdefl_init(&g_deflator, NULL, NULL, comp_flags); + assert(status == TDEFL_STATUS_OKAY); + if (status != TDEFL_STATUS_OKAY) + { + stringstream ss; + ss << "tdefl_init() failed with return code '" << status << "'"; + POLYVOX_THROW(std::runtime_error, ss.str()); + } + + size_t ulDstLength = uDstLength; + // Compress as much of the input as possible (or all of it) to the output buffer. + status = tdefl_compress(&g_deflator, pSrcData, &uSrcLength, pDstData, &ulDstLength, TDEFL_FINISH); + + assert(status == TDEFL_STATUS_DONE); + if (status != TDEFL_STATUS_DONE) + { + stringstream ss; + ss << "tdefl_compress() failed with return code '" << status << "'"; + POLYVOX_THROW(std::runtime_error, ss.str()); + } + return ulDstLength; } - uint32_t MinizCompressor::decompress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) + /*uint32_t MinizCompressor::decompress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) { mz_ulong ulDstLength = uDstLength; @@ -63,6 +103,32 @@ namespace PolyVox POLYVOX_THROW(std::runtime_error, ss.str()); } + return ulDstLength; + }*/ + + uint32_t MinizCompressor::decompress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) + { + size_t ulDstLength = uDstLength; + + //int result = mz_uncompress((unsigned char*) pDstData, &ulDstLength, (const unsigned char*) pSrcData, uSrcLength); + //assert(result == MZ_OK); + + //Check dest length is power of two! If it's a problem we could fall back on mz_uncompress at the expense of performance and bringing in more of the library. + + 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, &uSrcLength, (mz_uint8 *)pDstData, (mz_uint8 *)pDstData, &ulDstLength, TINFL_FLAG_PARSE_ZLIB_HEADER); + + if (status != TINFL_STATUS_DONE) + { + stringstream ss; + ss << "tinfl_decompress() failed with return code '" << status << "'"; + POLYVOX_THROW(std::runtime_error, ss.str()); + } + return ulDstLength; } } From 81eab0ebfba5e8fad177117f2f91f26453118e17 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Fri, 22 Feb 2013 17:03:47 +0100 Subject: [PATCH 03/19] Work on low-level version of compression. --- .../include/PolyVoxCore/MinizCompressor.h | 10 ++++- .../PolyVoxCore/source/MinizCompressor.cpp | 44 ++++++++++++------- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h b/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h index 2770300e..80a3717a 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h @@ -8,12 +8,20 @@ namespace PolyVox class MinizCompressor : public Compressor { public: - MinizCompressor(); + MinizCompressor(int iCompressionLevel = 6); // Miniz defines MZ_DEFAULT_LEVEL = 6 so we use the same here ~MinizCompressor(); uint32_t getMaxCompressedSize(uint32_t uUncompressedInputSize); uint32_t compress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); uint32_t decompress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); + + private: + int m_iCompressionLevel; + + // 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* g_deflator; }; } diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index 0c3496d8..19b86c9c 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -5,9 +5,9 @@ #define MINIZ_NO_STDIO #define MINIZ_NO_ARCHIVE_APIS #define MINIZ_NO_TIME -//#define MINIZ_NO_ZLIB_APIS +#define MINIZ_NO_ZLIB_APIS #define MINIZ_NO_ZLIB_COMPATIBLE_NAMES -//#define MINIZ_NO_MALLOC +#define MINIZ_NO_MALLOC #include "PolyVoxCore/Impl/ErrorHandling.h" // For some unknown reason the miniz library is supplied only as a @@ -21,19 +21,32 @@ using namespace std; namespace PolyVox { - MinizCompressor::MinizCompressor() + // Compression levels: 0-9 are the standard zlib-style levels, 10 is best possible compression (not zlib compatible, and may be very slow) + MinizCompressor::MinizCompressor(int iCompressionLevel) + :m_iCompressionLevel(iCompressionLevel) + ,g_deflator(0) { + tdefl_compressor* pDeflator = new tdefl_compressor; + g_deflator = reinterpret_cast(pDeflator); } MinizCompressor::~MinizCompressor() { + tdefl_compressor* pDeflator = reinterpret_cast(g_deflator); + delete pDeflator; } uint32_t MinizCompressor::getMaxCompressedSize(uint32_t uUncompressedInputSize) { - return static_cast(mz_compressBound(static_cast(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 MZ_MAX(128 + (source_len * 110) / 100, 128 + source_len + ((source_len / (31 * 1024)) + 1) * 5); } + // The commented out function is left here for reference and debugging purposes. /*uint32_t MinizCompressor::compress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) { mz_ulong ulDstLength = uDstLength; @@ -51,24 +64,23 @@ namespace PolyVox return ulDstLength; }*/ + // The behaviour of this function should be the same as the commented out version above (except that it requires the destination to be a power of two), + // but it's implemented using the lower level API which does not conflict with zlib or perform any memory allocations. uint32_t MinizCompressor::compress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) { - int level = 9; - - // tdefl_compressor contains all the state needed by the low-level compressor so it's a pretty big struct (~300k). - // This example makes it a global vs. putting it on the stack, of course in real-world usage you'll probably malloc() or new it. - tdefl_compressor g_deflator; + tdefl_compressor* pDeflator = reinterpret_cast(g_deflator); // The number of dictionary probes to use at each compression level (0-10). 0=implies fastest/minimal possible probing. 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). - mz_uint comp_flags = TDEFL_WRITE_ZLIB_HEADER | s_tdefl_num_probes[MZ_MIN(10, level)] | ((level <= 3) ? TDEFL_GREEDY_PARSING_FLAG : 0); - if (!level) + mz_uint comp_flags = TDEFL_WRITE_ZLIB_HEADER | s_tdefl_num_probes[MZ_MIN(10, m_iCompressionLevel)] | ((m_iCompressionLevel <= 3) ? TDEFL_GREEDY_PARSING_FLAG : 0); + if (!m_iCompressionLevel) + { comp_flags |= TDEFL_FORCE_ALL_RAW_BLOCKS; + } - tdefl_status status = tdefl_init(&g_deflator, NULL, NULL, comp_flags); - assert(status == TDEFL_STATUS_OKAY); + tdefl_status status = tdefl_init(pDeflator, NULL, NULL, comp_flags); if (status != TDEFL_STATUS_OKAY) { stringstream ss; @@ -78,9 +90,8 @@ namespace PolyVox size_t ulDstLength = uDstLength; // Compress as much of the input as possible (or all of it) to the output buffer. - status = tdefl_compress(&g_deflator, pSrcData, &uSrcLength, pDstData, &ulDstLength, TDEFL_FINISH); + status = tdefl_compress(pDeflator, pSrcData, &uSrcLength, pDstData, &ulDstLength, TDEFL_FINISH); - assert(status == TDEFL_STATUS_DONE); if (status != TDEFL_STATUS_DONE) { stringstream ss; @@ -91,6 +102,7 @@ namespace PolyVox return ulDstLength; } + // The commented out function is left here for reference and debugging purposes. /*uint32_t MinizCompressor::decompress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) { mz_ulong ulDstLength = uDstLength; @@ -106,6 +118,8 @@ namespace PolyVox return ulDstLength; }*/ + // The behaviour of this function should be the same as the commented out version above (except that it requires the destination to be a power of two), + // but it's implemented using the lower level API which does not conflict with zlib or perform any memory allocations. uint32_t MinizCompressor::decompress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) { size_t ulDstLength = uDstLength; From 34671130fbd7bc626a3238fc25ce79928e8bc317 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Mon, 25 Feb 2013 16:33:39 +0100 Subject: [PATCH 04/19] Tidying up Miniz compression code. --- .../include/PolyVoxCore/MinizCompressor.h | 4 +-- .../PolyVoxCore/source/MinizCompressor.cpp | 32 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h b/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h index 80a3717a..f6a5ed5c 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h @@ -16,12 +16,12 @@ namespace PolyVox uint32_t decompress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); private: - int m_iCompressionLevel; + 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. - void* g_deflator; + void* m_pDeflator; }; } diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index 19b86c9c..7e7032a9 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -23,16 +23,25 @@ namespace PolyVox { // Compression levels: 0-9 are the standard zlib-style levels, 10 is best possible compression (not zlib compatible, and may be very slow) MinizCompressor::MinizCompressor(int iCompressionLevel) - :m_iCompressionLevel(iCompressionLevel) - ,g_deflator(0) + :m_pDeflator(0) { tdefl_compressor* pDeflator = new tdefl_compressor; - g_deflator = 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. + 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); + if (!iCompressionLevel) + { + m_uCompressionFlags |= TDEFL_FORCE_ALL_RAW_BLOCKS; + } } MinizCompressor::~MinizCompressor() { - tdefl_compressor* pDeflator = reinterpret_cast(g_deflator); + tdefl_compressor* pDeflator = reinterpret_cast(m_pDeflator); delete pDeflator; } @@ -68,19 +77,10 @@ namespace PolyVox // but it's implemented using the lower level API which does not conflict with zlib or perform any memory allocations. uint32_t MinizCompressor::compress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) { - tdefl_compressor* pDeflator = reinterpret_cast(g_deflator); + tdefl_compressor* pDeflator = reinterpret_cast(m_pDeflator); - // The number of dictionary probes to use at each compression level (0-10). 0=implies fastest/minimal possible probing. - 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). - mz_uint comp_flags = TDEFL_WRITE_ZLIB_HEADER | s_tdefl_num_probes[MZ_MIN(10, m_iCompressionLevel)] | ((m_iCompressionLevel <= 3) ? TDEFL_GREEDY_PARSING_FLAG : 0); - if (!m_iCompressionLevel) - { - comp_flags |= TDEFL_FORCE_ALL_RAW_BLOCKS; - } - - tdefl_status status = tdefl_init(pDeflator, NULL, NULL, comp_flags); + // 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; From f70498e806afc281d131207f861466da0dd9b9e5 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Mon, 25 Feb 2013 16:34:21 +0100 Subject: [PATCH 05/19] Removed old code. --- .../PolyVoxCore/source/MinizCompressor.cpp | 34 ------------------- tests/testvolume.cpp | 8 ++--- tests/testvolume.h | 4 +-- 3 files changed, 6 insertions(+), 40 deletions(-) diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index 7e7032a9..3b5b177e 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -55,24 +55,6 @@ namespace PolyVox return MZ_MAX(128 + (source_len * 110) / 100, 128 + source_len + ((source_len / (31 * 1024)) + 1) * 5); } - // The commented out function is left here for reference and debugging purposes. - /*uint32_t MinizCompressor::compress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) - { - mz_ulong ulDstLength = uDstLength; - - // Do the compression - int result = mz_compress((unsigned char*)pDstData, &ulDstLength, (const unsigned char*) pSrcData, uSrcLength); - if(result != MZ_OK) - { - stringstream ss; - ss << "mz_compress() failed with return code '" << result << "'"; - POLYVOX_THROW(std::runtime_error, ss.str()); - } - - // Return the number of bytes written to the output. - return ulDstLength; - }*/ - // The behaviour of this function should be the same as the commented out version above (except that it requires the destination to be a power of two), // but it's implemented using the lower level API which does not conflict with zlib or perform any memory allocations. uint32_t MinizCompressor::compress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) @@ -102,22 +84,6 @@ namespace PolyVox return ulDstLength; } - // The commented out function is left here for reference and debugging purposes. - /*uint32_t MinizCompressor::decompress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) - { - mz_ulong ulDstLength = uDstLength; - - int result = mz_uncompress((unsigned char*) pDstData, &ulDstLength, (const unsigned char*) pSrcData, uSrcLength); - if(result != MZ_OK) - { - stringstream ss; - ss << "mz_uncompress() failed with return code '" << result << "'"; - POLYVOX_THROW(std::runtime_error, ss.str()); - } - - return ulDstLength; - }*/ - // The behaviour of this function should be the same as the commented out version above (except that it requires the destination to be a power of two), // but it's implemented using the lower level API which does not conflict with zlib or perform any memory allocations. uint32_t MinizCompressor::decompress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) diff --git a/tests/testvolume.cpp b/tests/testvolume.cpp index 2bd15cbd..74a3ffa1 100644 --- a/tests/testvolume.cpp +++ b/tests/testvolume.cpp @@ -312,7 +312,7 @@ TestVolume::~TestVolume() * RawVolume Tests */ -void TestVolume::testRawVolumeDirectAccessAllInternalForwards() +/*void TestVolume::testRawVolumeDirectAccessAllInternalForwards() { int32_t result = 0; @@ -398,13 +398,13 @@ void TestVolume::testRawVolumeSamplersWithExternalBackwards() result = testSamplersWithWrappingBackwards(m_pRawVolume, -1, -3, -2, 2, 5, 4); } QCOMPARE(result, static_cast(-769775893)); -} +}*/ /* * SimpleVolume Tests */ -void TestVolume::testSimpleVolumeDirectAccessAllInternalForwards() +/*void TestVolume::testSimpleVolumeDirectAccessAllInternalForwards() { int32_t result = 0; QBENCHMARK @@ -482,7 +482,7 @@ void TestVolume::testSimpleVolumeSamplersWithExternalBackwards() result = testSamplersWithWrappingBackwards(m_pSimpleVolume, -1, -3, -2, 2, 5, 4); } QCOMPARE(result, static_cast(-769775893)); -} +}*/ /* * LargeVolume Tests diff --git a/tests/testvolume.h b/tests/testvolume.h index 0c363109..59a63ec8 100644 --- a/tests/testvolume.h +++ b/tests/testvolume.h @@ -37,7 +37,7 @@ public: ~TestVolume(); private slots: - void testRawVolumeDirectAccessAllInternalForwards(); + /*void testRawVolumeDirectAccessAllInternalForwards(); void testRawVolumeSamplersAllInternalForwards(); void testRawVolumeDirectAccessWithExternalForwards(); void testRawVolumeSamplersWithExternalForwards(); @@ -53,7 +53,7 @@ private slots: void testSimpleVolumeDirectAccessAllInternalBackwards(); void testSimpleVolumeSamplersAllInternalBackwards(); void testSimpleVolumeDirectAccessWithExternalBackwards(); - void testSimpleVolumeSamplersWithExternalBackwards(); + void testSimpleVolumeSamplersWithExternalBackwards();*/ void testLargeVolumeDirectAccessAllInternalForwards(); void testLargeVolumeSamplersAllInternalForwards(); From e770baeb057c19bc8d3fd41bc2fcb503294145df Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Mon, 25 Feb 2013 16:46:04 +0100 Subject: [PATCH 06/19] Added assert for buffer size. --- library/PolyVoxCore/source/MinizCompressor.cpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index 3b5b177e..8f478bd3 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -1,5 +1,7 @@ #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 @@ -88,13 +90,18 @@ namespace PolyVox // but it's implemented using the lower level API which does not conflict with zlib or perform any memory allocations. uint32_t MinizCompressor::decompress(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."); + } + size_t ulDstLength = uDstLength; - //int result = mz_uncompress((unsigned char*) pDstData, &ulDstLength, (const unsigned char*) pSrcData, uSrcLength); - //assert(result == MZ_OK); - - //Check dest length is power of two! If it's a problem we could fall back on mz_uncompress at the expense of performance and bringing in more of the library. - tinfl_decompressor inflator; tinfl_init(&inflator); From c42270f16551c749b207dcdb784a7167ae61f614 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Mon, 25 Feb 2013 16:51:57 +0100 Subject: [PATCH 07/19] Possible Linux fix? I didn't reproduce the error myself so I'm just being guided by the CDash messages. --- library/PolyVoxCore/source/MinizCompressor.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index 8f478bd3..adfd31a6 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -72,9 +72,11 @@ namespace PolyVox POLYVOX_THROW(std::runtime_error, ss.str()); } + 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, &uSrcLength, pDstData, &ulDstLength, TDEFL_FINISH); + status = tdefl_compress(pDeflator, pSrcData, &ulSrcLength, pDstData, &ulDstLength, TDEFL_FINISH); if (status != TDEFL_STATUS_DONE) { @@ -100,6 +102,7 @@ namespace PolyVox POLYVOX_THROW(std::invalid_argument, "Miniz decompressor requires the destination buffer to have a size which is a power of two."); } + size_t ulSrcLength = uSrcLength; size_t ulDstLength = uDstLength; tinfl_decompressor inflator; @@ -107,7 +110,7 @@ namespace PolyVox // 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, &uSrcLength, (mz_uint8 *)pDstData, (mz_uint8 *)pDstData, &ulDstLength, TINFL_FLAG_PARSE_ZLIB_HEADER); + tinfl_status status = tinfl_decompress(&inflator, (const mz_uint8 *)pSrcData, &ulSrcLength, (mz_uint8 *)pDstData, (mz_uint8 *)pDstData, &ulDstLength, TINFL_FLAG_PARSE_ZLIB_HEADER); if (status != TINFL_STATUS_DONE) { From 62370868c84b642a013b3e1fa5bd2c6ac9513bdf Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Mon, 25 Feb 2013 17:06:12 +0100 Subject: [PATCH 08/19] Reverted accidental changes to tests. Updated comments in compression code. --- library/PolyVoxCore/source/MinizCompressor.cpp | 17 ++++++++++++----- tests/testvolume.cpp | 8 ++++---- tests/testvolume.h | 4 ++-- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index adfd31a6..33fd2978 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -27,13 +27,15 @@ namespace PolyVox 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). + // 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); if (!iCompressionLevel) { @@ -43,6 +45,7 @@ namespace PolyVox MinizCompressor::~MinizCompressor() { + // Delete the deflator tdefl_compressor* pDeflator = reinterpret_cast(m_pDeflator); delete pDeflator; } @@ -57,10 +60,9 @@ namespace PolyVox return MZ_MAX(128 + (source_len * 110) / 100, 128 + source_len + ((source_len / (31 * 1024)) + 1) * 5); } - // The behaviour of this function should be the same as the commented out version above (except that it requires the destination to be a power of two), - // but it's implemented using the lower level API which does not conflict with zlib or perform any memory allocations. uint32_t MinizCompressor::compress(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) @@ -72,12 +74,14 @@ namespace PolyVox 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; @@ -85,11 +89,10 @@ namespace PolyVox POLYVOX_THROW(std::runtime_error, ss.str()); } + // The compression modifies 'ulDstLength' to hold the new length. return ulDstLength; } - // The behaviour of this function should be the same as the commented out version above (except that it requires the destination to be a power of two), - // but it's implemented using the lower level API which does not conflict with zlib or perform any memory allocations. uint32_t MinizCompressor::decompress(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 @@ -102,9 +105,11 @@ namespace PolyVox 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); @@ -112,6 +117,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. if (status != TINFL_STATUS_DONE) { stringstream ss; @@ -119,6 +125,7 @@ namespace PolyVox POLYVOX_THROW(std::runtime_error, ss.str()); } + // The decompression modifies 'ulDstLength' to hold the new length. return ulDstLength; } } diff --git a/tests/testvolume.cpp b/tests/testvolume.cpp index 74a3ffa1..2bd15cbd 100644 --- a/tests/testvolume.cpp +++ b/tests/testvolume.cpp @@ -312,7 +312,7 @@ TestVolume::~TestVolume() * RawVolume Tests */ -/*void TestVolume::testRawVolumeDirectAccessAllInternalForwards() +void TestVolume::testRawVolumeDirectAccessAllInternalForwards() { int32_t result = 0; @@ -398,13 +398,13 @@ void TestVolume::testRawVolumeSamplersWithExternalBackwards() result = testSamplersWithWrappingBackwards(m_pRawVolume, -1, -3, -2, 2, 5, 4); } QCOMPARE(result, static_cast(-769775893)); -}*/ +} /* * SimpleVolume Tests */ -/*void TestVolume::testSimpleVolumeDirectAccessAllInternalForwards() +void TestVolume::testSimpleVolumeDirectAccessAllInternalForwards() { int32_t result = 0; QBENCHMARK @@ -482,7 +482,7 @@ void TestVolume::testSimpleVolumeSamplersWithExternalBackwards() result = testSamplersWithWrappingBackwards(m_pSimpleVolume, -1, -3, -2, 2, 5, 4); } QCOMPARE(result, static_cast(-769775893)); -}*/ +} /* * LargeVolume Tests diff --git a/tests/testvolume.h b/tests/testvolume.h index 59a63ec8..0c363109 100644 --- a/tests/testvolume.h +++ b/tests/testvolume.h @@ -37,7 +37,7 @@ public: ~TestVolume(); private slots: - /*void testRawVolumeDirectAccessAllInternalForwards(); + void testRawVolumeDirectAccessAllInternalForwards(); void testRawVolumeSamplersAllInternalForwards(); void testRawVolumeDirectAccessWithExternalForwards(); void testRawVolumeSamplersWithExternalForwards(); @@ -53,7 +53,7 @@ private slots: void testSimpleVolumeDirectAccessAllInternalBackwards(); void testSimpleVolumeSamplersAllInternalBackwards(); void testSimpleVolumeDirectAccessWithExternalBackwards(); - void testSimpleVolumeSamplersWithExternalBackwards();*/ + void testSimpleVolumeSamplersWithExternalBackwards(); void testLargeVolumeDirectAccessAllInternalForwards(); void testLargeVolumeSamplersAllInternalForwards(); From 80025eaa46d2daeb9725db5c67b05918c92af7d7 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Tue, 26 Feb 2013 10:49:01 +0100 Subject: [PATCH 09/19] Attempting to disable GCC compiler warnings in miniz.c (as I don't want to modify external code). --- library/PolyVoxCore/source/MinizCompressor.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index 33fd2978..df4c0947 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -15,7 +15,11 @@ // For some unknown reason the miniz library is supplied only as a // single .c file without a header. Apparently the only way to use // it is then to #include it directly which is what the examples do. +// We also disable some warnings as I don't want to fix external code. +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wno-enum-compare" #include "PolyVoxCore/Impl/miniz.c" +#pragma GCC diagnostic pop #include From 7e50dcbd937a73d448d1dd4d04d9a15f1473bded Mon Sep 17 00:00:00 2001 From: David Williams Date: Wed, 27 Feb 2013 14:25:10 +0100 Subject: [PATCH 10/19] It seems push/pop of diagnostic pragmas is only supported on GCC >= 4.6. I've just disabled the warning for the whole file instead. --- library/PolyVoxCore/source/MinizCompressor.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index df4c0947..5fb1b7b5 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -16,10 +16,9 @@ // single .c file without a header. Apparently the only way to use // it is then to #include it directly which is what the examples do. // We also disable some warnings as I don't want to fix external code. -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wno-enum-compare" +#pragma GCC diagnostic ignored "-Wenum-compare" #include "PolyVoxCore/Impl/miniz.c" -#pragma GCC diagnostic pop + #include From 4534f721b5c7203295b70fc45746f77e24e6bdc0 Mon Sep 17 00:00:00 2001 From: David Williams Date: Wed, 27 Feb 2013 15:07:52 +0100 Subject: [PATCH 11/19] A one line change for testing purposes. I'm curious whether another addition commit gets pushed. --- CHANGELOG.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 1b772672..59c4042a 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -2,6 +2,8 @@ Changes for PolyVox version 0.3 =============================== This release has focused on... (some introduction here). +This line was added just for testing. + LargeVolume ----------- It is now possible to provide custom compressors for the data which is stored in a LargeVolume. Two compressor implementation are provided with PolyVox - RLECompressor which is suitable for cubic-style terrain, and MinizCompressor which uses the 'miniz' zlib implementation for smooth terrain or general purpose use. Users can provide their own implementation of the compressor interface if they wish. From c28fa9a0cadde3f0e5fce7378cc1c7d5d57602d3 Mon Sep 17 00:00:00 2001 From: David Williams Date: Thu, 28 Feb 2013 13:42:02 +0100 Subject: [PATCH 12/19] It seems that GCC 4.3.5 (on the CDash machine) doesn't recognise ignoring the 'enum-compare' warning. This commit should switch of all warnings instead (just for this file). --- library/PolyVoxCore/source/MinizCompressor.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index 5fb1b7b5..827c2817 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -12,11 +12,12 @@ #define MINIZ_NO_MALLOC #include "PolyVoxCore/Impl/ErrorHandling.h" + // For some unknown reason the miniz library is supplied only as a // single .c file without a header. Apparently the only way to use // it is then to #include it directly which is what the examples do. // We also disable some warnings as I don't want to fix external code. -#pragma GCC diagnostic ignored "-Wenum-compare" +#pragma GCC diagnostic ignored "-Wall" #include "PolyVoxCore/Impl/miniz.c" From a7e49a1394b634a6659666eab4378e5f397ad130 Mon Sep 17 00:00:00 2001 From: David Williams Date: Fri, 1 Mar 2013 16:07:50 +0100 Subject: [PATCH 13/19] Still trying to get rid of the warnings on the CDash machine. It's tricky because I'm not seeing the locally and the warning supression seems to vary bewteen vesions of GCC (See: http://dbp-consulting.com/tutorials/SuppressingGCCWarnings.html). --- library/PolyVoxCore/include/PolyVoxCore/Impl/miniz.c | 11 +++++++++++ library/PolyVoxCore/source/MinizCompressor.cpp | 2 -- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/Impl/miniz.c b/library/PolyVoxCore/include/PolyVoxCore/Impl/miniz.c index 39a17524..4b2608e1 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/Impl/miniz.c +++ b/library/PolyVoxCore/include/PolyVoxCore/Impl/miniz.c @@ -1,3 +1,14 @@ +/* + * CHANGES FOR POLYVOX + * ------------------- + * This file gave compiler warnings on certain versions of GCC (at least version 4.3.5 used by out build machine) + * and I did not want to risk tampering with the code to fix them.Therefore the only difference between this file + * and the official 'miniz.c' is the pragma below which disables warnings for this file in GCC. + */ +#ifdef __GNUC__ +#pragma GCC system_header +#endif + /* miniz.c v1.14 - public domain deflate/inflate, zlib-subset, ZIP reading/writing/appending, PNG writing See "unlicense" statement at the end of this file. Rich Geldreich , last updated May 20, 2012 diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index 827c2817..b44f63d9 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -16,8 +16,6 @@ // For some unknown reason the miniz library is supplied only as a // single .c file without a header. Apparently the only way to use // it is then to #include it directly which is what the examples do. -// We also disable some warnings as I don't want to fix external code. -#pragma GCC diagnostic ignored "-Wall" #include "PolyVoxCore/Impl/miniz.c" From d12db9906f6e52f7379883185be7a6d9b6075ddd Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Fri, 1 Mar 2013 16:33:31 +0100 Subject: [PATCH 14/19] Updated note about why we #include the miniz.c file. --- library/PolyVoxCore/source/MinizCompressor.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index b44f63d9..af1af34b 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -13,9 +13,13 @@ #include "PolyVoxCore/Impl/ErrorHandling.h" -// For some unknown reason the miniz library is supplied only as a -// single .c file without a header. Apparently the only way to use -// it is then to #include it directly which is what the examples do. +// 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" From 74b4caba6bcd4e83f4af0088b021633799547e7f Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Fri, 1 Mar 2013 17:06:51 +0100 Subject: [PATCH 15/19] Added some API docs to compressor. --- .../include/PolyVoxCore/Compressor.h | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/Compressor.h b/library/PolyVoxCore/include/PolyVoxCore/Compressor.h index f731bd68..7006b2e8 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/Compressor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/Compressor.h @@ -28,20 +28,35 @@ freely, subject to the following restrictions: 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. + */ 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. + /// 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 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. + /// Compresses the data. virtual uint32_t compress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) = 0; - // Decompresses the data. + /// Decompresses the data. virtual uint32_t decompress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) = 0; }; } From b4fcb3daf8b4fe9d2ac7d269f6a83ea8befa3a7d Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Mon, 4 Mar 2013 15:17:19 +0100 Subject: [PATCH 16/19] Added API documentation regarding compression. --- .../include/PolyVoxCore/Compressor.h | 35 ++++++++++++++++++- .../include/PolyVoxCore/MinizCompressor.h | 15 +++++++- .../include/PolyVoxCore/RLECompressor.h | 12 +++++++ .../PolyVoxCore/source/MinizCompressor.cpp | 4 ++- 4 files changed, 63 insertions(+), 3 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/Compressor.h b/library/PolyVoxCore/include/PolyVoxCore/Compressor.h index 7006b2e8..510b7e20 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/Compressor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/Compressor.h @@ -35,6 +35,10 @@ namespace PolyVox * 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 { @@ -49,14 +53,43 @@ namespace PolyVox /// 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 The size of the uncompressed input data + /// + /// \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(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(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) = 0; }; } diff --git a/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h b/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h index f6a5ed5c..fb2b2473 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/MinizCompressor.h @@ -5,12 +5,25 @@ 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(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); uint32_t decompress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); diff --git a/library/PolyVoxCore/include/PolyVoxCore/RLECompressor.h b/library/PolyVoxCore/include/PolyVoxCore/RLECompressor.h index 7e75742b..10b52b90 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/RLECompressor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/RLECompressor.h @@ -5,6 +5,15 @@ namespace PolyVox { + /** + * Performs compression of data using Run Length Encoding (RLE). + * + * This compressor is designed for voxel data which contains long runs of the same value. Minecraft-style terrain and other + * cubic-style terrains are likely to fall under this category, whereas density fields for Marching Cubes terrain will not. Please + * see the following article if you want more details of how RLE compression works: http://en.wikipedia.org/wiki/Run-length_encoding + * + * \sa MinizCompressor + */ template class RLECompressor : public Compressor { @@ -14,9 +23,12 @@ namespace PolyVox LengthType length; }; public: + /// Constructor RLECompressor(); + /// Destructor ~RLECompressor(); + // API documentation is in base class and gets inherited by Doxygen. uint32_t getMaxCompressedSize(uint32_t uUncompressedInputSize); uint32_t compress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); uint32_t decompress(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength); diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index af1af34b..4c17ae65 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -29,7 +29,9 @@ using namespace std; namespace PolyVox { - // Compression levels: 0-9 are the standard zlib-style levels, 10 is best possible compression (not zlib compatible, and may be very slow) + /// 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) { From df5c339f6445aa236fdf833b22855e8c0ec15a72 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Mon, 4 Mar 2013 15:36:11 +0100 Subject: [PATCH 17/19] Just remembered that we settled on a different formatting for the API docs so that it gets highlighted better in KDE. --- .../include/PolyVoxCore/Compressor.h | 79 ++++++++++--------- .../PolyVoxCore/source/MinizCompressor.cpp | 8 +- 2 files changed, 46 insertions(+), 41 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/Compressor.h b/library/PolyVoxCore/include/PolyVoxCore/Compressor.h index 510b7e20..8c00be5e 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/Compressor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/Compressor.h @@ -48,48 +48,51 @@ namespace PolyVox /// 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. - /// + /** + * 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. - /// + /** + * 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(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. - /// + /** + * 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(void* pSrcData, uint32_t uSrcLength, void* pDstData, uint32_t uDstLength) = 0; }; } diff --git a/library/PolyVoxCore/source/MinizCompressor.cpp b/library/PolyVoxCore/source/MinizCompressor.cpp index 4c17ae65..a341ff81 100644 --- a/library/PolyVoxCore/source/MinizCompressor.cpp +++ b/library/PolyVoxCore/source/MinizCompressor.cpp @@ -29,9 +29,11 @@ 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. + /** + * 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) { From 23042c3fcbf91f73be88f003cbd4b1d74b2b2f1d Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Mon, 4 Mar 2013 16:00:43 +0100 Subject: [PATCH 18/19] Moved warning suppression into the relevant file so that it's not global. --- library/PolyVoxCore/CMakeLists.txt | 2 +- .../include/PolyVoxCore/Impl/ErrorHandling.h | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/library/PolyVoxCore/CMakeLists.txt b/library/PolyVoxCore/CMakeLists.txt index 65492a6f..145484dd 100644 --- a/library/PolyVoxCore/CMakeLists.txt +++ b/library/PolyVoxCore/CMakeLists.txt @@ -153,7 +153,7 @@ SET_PROPERTY(TARGET PolyVoxCore PROPERTY FOLDER "Library") SET_TARGET_PROPERTIES(PolyVoxCore PROPERTIES VERSION ${POLYVOX_VERSION} SOVERSION ${POLYVOX_VERSION_MAJOR}) IF(MSVC) - SET_TARGET_PROPERTIES(PolyVoxCore PROPERTIES COMPILE_FLAGS "/W4 /wd4251 /wd4127") #Disable warning on STL exports + SET_TARGET_PROPERTIES(PolyVoxCore PROPERTIES COMPILE_FLAGS "/W4 /wd4251") #Disable warning on STL exports ENDIF(MSVC) #Install diff --git a/library/PolyVoxCore/include/PolyVoxCore/Impl/ErrorHandling.h b/library/PolyVoxCore/include/PolyVoxCore/Impl/ErrorHandling.h index 8d434c9a..3e5d9057 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/Impl/ErrorHandling.h +++ b/library/PolyVoxCore/include/PolyVoxCore/Impl/ErrorHandling.h @@ -36,6 +36,14 @@ freely, subject to the following restrictions: #define POLYVOX_HALT() std::exit(EXIT_FAILURE) #endif +// We use the do...while(0) construct in our macros (for reasons see here: http://stackoverflow.com/a/154138) +// but Visual Studio gives unhelpful 'conditional expression is constant' warnings. The recommended solution +// (http://stackoverflow.com/a/1946485) is to disable these warnings. +#if defined(_MSC_VER) + __pragma(warning(push)) + __pragma(warning(disable:4127)) +#endif + #define POLYVOX_UNUSED(x) do { (void)sizeof(x); } while(0) /* @@ -120,4 +128,9 @@ freely, subject to the following restrictions: getThrowHandler()((except), __FILE__, __LINE__) #endif +// See the corresponding 'push' above. +#if defined(_MSC_VER) + __pragma(warning(pop)) +#endif + #endif //__PolyVox_ErrorHandling_H__ From 6374ebf092c5829031deb1f22704f52cc4f82b31 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Mon, 4 Mar 2013 17:10:23 +0100 Subject: [PATCH 19/19] Apparently I didn't test my fix for warning 4127 because it didn't actually work. Now fixed :-) --- .../include/PolyVoxCore/Impl/ErrorHandling.h | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/Impl/ErrorHandling.h b/library/PolyVoxCore/include/PolyVoxCore/Impl/ErrorHandling.h index 3e5d9057..70778808 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/Impl/ErrorHandling.h +++ b/library/PolyVoxCore/include/PolyVoxCore/Impl/ErrorHandling.h @@ -36,12 +36,17 @@ freely, subject to the following restrictions: #define POLYVOX_HALT() std::exit(EXIT_FAILURE) #endif -// We use the do...while(0) construct in our macros (for reasons see here: http://stackoverflow.com/a/154138) -// but Visual Studio gives unhelpful 'conditional expression is constant' warnings. The recommended solution -// (http://stackoverflow.com/a/1946485) is to disable these warnings. +// Macros cannot contain #ifdefs, but some of our macros need to disable warnings and such warning supression is +// platform specific. But macros can contain other macros, so we create macros to control the warnings and use +// those instead. This set of warning supression macros can be extended to GCC/Clang when required. #if defined(_MSC_VER) - __pragma(warning(push)) - __pragma(warning(disable:4127)) + #define POLYVOX_MSC_WARNING_PUSH __pragma(warning(push)) + #define POLYVOX_DISABLE_MSC_WARNING(x) __pragma(warning(disable:x)) + #define POLYVOX_MSC_WARNING_POP __pragma(warning(pop)) +#else + #define POLYVOX_MSC_WARNING_PUSH + #define POLYVOX_DISABLE_MSC_WARNING(x) + #define POLYVOX_MSC_WARNING_POP #endif #define POLYVOX_UNUSED(x) do { (void)sizeof(x); } while(0) @@ -57,6 +62,11 @@ freely, subject to the following restrictions: #ifdef POLYVOX_ASSERTS_ENABLED #define POLYVOX_ASSERT(condition, message) \ + /* We use the do...while(0) construct in our macros (for reasons see here: http://stackoverflow.com/a/154138) \ + but Visual Studio gives unhelpful 'conditional expression is constant' warnings. The recommended solution \ + (http://stackoverflow.com/a/1946485) is to disable these warnings. */ \ + POLYVOX_MSC_WARNING_PUSH \ + POLYVOX_DISABLE_MSC_WARNING(4127) \ do \ { \ if (!(condition)) \ @@ -69,12 +79,19 @@ freely, subject to the following restrictions: std::cerr << " Location: " << "Line " << __LINE__ << " of " << __FILE__ << std::endl << std::endl; \ POLYVOX_HALT(); \ } \ - } while(0) + } while(0) \ + POLYVOX_MSC_WARNING_POP #else #define POLYVOX_ASSERT(condition, message) \ - do { POLYVOX_UNUSED(condition); POLYVOX_UNUSED(message); } while(0) + /* We use the do...while(0) construct in our macros (for reasons see here: http://stackoverflow.com/a/154138) \ + but Visual Studio gives unhelpful 'conditional expression is constant' warnings. The recommended solution \ + (http://stackoverflow.com/a/1946485) is to disable these warnings. */ \ + POLYVOX_MSC_WARNING_PUSH \ + POLYVOX_DISABLE_MSC_WARNING(4127) \ + do { POLYVOX_UNUSED(condition); POLYVOX_UNUSED(message); } while(0) \ + POLYVOX_MSC_WARNING_POP #endif @@ -128,9 +145,4 @@ freely, subject to the following restrictions: getThrowHandler()((except), __FILE__, __LINE__) #endif -// See the corresponding 'push' above. -#if defined(_MSC_VER) - __pragma(warning(pop)) -#endif - #endif //__PolyVox_ErrorHandling_H__