From a1ac90c71126e31cae8ccd413f0af6fbf051b746 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sat, 7 Jan 2012 11:46:25 +0000 Subject: [PATCH] Fixed crash due to me incorrectly deciding that the max number of quads which could share a vertex was four. I believe the correct value should actually be six. --- .../PolyVoxCore/CubicSurfaceExtractor.h | 7 +++---- .../PolyVoxCore/CubicSurfaceExtractor.inl | 20 ++++++++++++------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.h b/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.h index c6d362dd..35933a6c 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.h +++ b/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.h @@ -87,10 +87,9 @@ namespace PolyVox //is the user needs per-vertex attributes, or to perform per vertex lighting. bool m_bMergeQuads; - //Although we try to avoid creating multiple vertices at the same location, sometimes this is unavoidable - //if they have different materials. For example, four different materials next to each other would mean - //four quads (though more triangles) sharing the vertex. As far as I can tell, four is the worst case scenario. - static const uint32_t MaxQuadsSharingVertex; + //This constant defines the maximum number of quads which can share a + //vertex in a cubic style mesh. See the initialisation for more details. + static const uint32_t MaxVerticesPerPosition; }; } diff --git a/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.inl b/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.inl index 84993133..c9cff506 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.inl +++ b/library/PolyVoxCore/include/PolyVoxCore/CubicSurfaceExtractor.inl @@ -23,8 +23,14 @@ freely, subject to the following restrictions: namespace PolyVox { + // We try to avoid duplicate vertices by checking whether a vertex has already been added at a given position. + // However, it is possible that vertices have the same position but differnt materials. In this case, the + // vertices are not true duplicates and both must be added to the mesh. As far as I can tell, it is possible to have + // at most six vertices with the same position but different materials. This worst-case scenario happens when we + // have a 2x2x2 group of voxels (all with different materials) and then we delete two voxels from opposing corners. + // The vertex position at the center of this group is then going to be used by six quads all with different materials. template< template class VolumeType, typename VoxelType> - const uint32_t CubicSurfaceExtractor::MaxQuadsSharingVertex = 4; + const uint32_t CubicSurfaceExtractor::MaxVerticesPerPosition = 6; template< template class VolumeType, typename VoxelType> CubicSurfaceExtractor::CubicSurfaceExtractor(VolumeType* volData, Region region, SurfaceMesh* result, bool bMergeQuads) @@ -43,7 +49,7 @@ namespace PolyVox uint32_t uArrayWidth = m_regSizeInVoxels.getUpperCorner().getX() - m_regSizeInVoxels.getLowerCorner().getX() + 2; uint32_t uArrayHeight = m_regSizeInVoxels.getUpperCorner().getY() - m_regSizeInVoxels.getLowerCorner().getY() + 2; - uint32_t arraySize[3]= {uArrayWidth, uArrayHeight, MaxQuadsSharingVertex}; + uint32_t arraySize[3]= {uArrayWidth, uArrayHeight, MaxVerticesPerPosition}; m_previousSliceVertices.resize(arraySize); m_currentSliceVertices.resize(arraySize); memset(m_previousSliceVertices.getRawData(), 0xff, m_previousSliceVertices.getNoOfElements() * sizeof(IndexAndMaterial)); @@ -241,7 +247,7 @@ namespace PolyVox uint32_t uX = static_cast(fX + 0.75f); uint32_t uY = static_cast(fY + 0.75f); - for(uint32_t ct = 0; ct < MaxQuadsSharingVertex; ct++) + for(uint32_t ct = 0; ct < MaxVerticesPerPosition; ct++) { IndexAndMaterial& rEntry = existingVertices[uX][uY][ct]; @@ -261,10 +267,10 @@ namespace PolyVox } } - //If we exit the loop here then apparently all the slots were full but none of - //them matched. I don't think this can happen so let's put an assert to make sure. + // If we exit the loop here then apparently all the slots were full but none of them matched. I don't think + // this can happen so let's put an assert to make sure. If you hit this assert then please report it to us! assert(false); - return 0; + return -1; //Should never happen. } template< template class VolumeType, typename VoxelType> @@ -301,7 +307,7 @@ namespace PolyVox bool CubicSurfaceExtractor::mergeQuads(Quad& q1, Quad& q2) { //All four vertices of a given quad have the same material, - //so just check that the first pair or vertices match. + //so just check that the first pair of vertices match. if(std::abs(m_meshCurrent->getVertices()[q1.vertices[0]].getMaterial() - m_meshCurrent->getVertices()[q2.vertices[0]].getMaterial()) < 0.001) { //Now check whether quad 2 is adjacent to quad one by comparing vertices.