Skip to content

Commit d59b9b9

Browse files
authored
Fix sparseindices (#742)
* accept size_t for Vec * Prepare a test case for "SparseIndices too large" core dump * fix python docstring deps * fix compilation errors * limit sdf hashtable size * another fix to hashtable * log2(0) is undefined * fix formatting * Revert "fix compilation errors" This reverts commit adfb86c. * fix size_t stuff * remove processOverlap setting in test * fix hashtable * vertex number limit * address comments
1 parent f3c5ada commit d59b9b9

20 files changed

+185
-111
lines changed

CMakeLists.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ if (MSVC)
8686
set(MANIFOLD_FLAGS ${MANIFOLD_FLAGS} /DNOMINMAX)
8787
else()
8888
if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
89-
set(WARNING_FLAGS -Werror -Wall -Wno-sign-compare -Wno-unused -Wno-array-bounds -Wno-stringop-overflow)
89+
set(WARNING_FLAGS -Werror -Wall -Wno-sign-compare -Wno-unused -Wno-array-bounds
90+
-Wno-stringop-overflow -Wno-alloc-size-larger-than)
9091
else()
9192
set(WARNING_FLAGS -Werror -Wall -Wno-sign-compare -Wno-unused)
9293
endif()

bindings/python/CMakeLists.txt

+10
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,18 @@ target_compile_features(manifold3d PUBLIC cxx_std_17)
2626
set_target_properties(manifold3d PROPERTIES OUTPUT_NAME "manifold3d")
2727

2828
message(Python_EXECUTABLE = ${Python_EXECUTABLE})
29+
# ideally we should generate a dependency file from python...
30+
set(DOCSTRING_DEPS
31+
${CMAKE_CURRENT_SOURCE_DIR}/../../src/manifold/src/manifold.cpp
32+
${CMAKE_CURRENT_SOURCE_DIR}/../../src/manifold/src/constructors.cpp
33+
${CMAKE_CURRENT_SOURCE_DIR}/../../src/manifold/src/sort.cpp
34+
${CMAKE_CURRENT_SOURCE_DIR}/../../src/cross_section/src/cross_section.cpp
35+
${CMAKE_CURRENT_SOURCE_DIR}/../../src/polygon/src/polygon.cpp
36+
${CMAKE_CURRENT_SOURCE_DIR}/../../src/utilities/include/public.h
37+
)
2938
add_custom_command(
3039
OUTPUT autogen_docstrings.inl
40+
DEPENDS ${DOCSTRING_DEPS}
3141
COMMAND ${Python_EXECUTABLE} ARGS ${CMAKE_CURRENT_SOURCE_DIR}/gen_docs.py
3242
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
3343
)

src/manifold/include/manifold.h

+10-2
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,17 @@ class CsgLeafNode;
4646
*/
4747
struct MeshGL {
4848
/// Number of property vertices
49-
uint32_t NumVert() const { return vertProperties.size() / numProp; };
49+
uint32_t NumVert() const {
50+
if (vertProperties.size() / numProp >= std::numeric_limits<int>::max())
51+
throw std::out_of_range("mesh too large");
52+
return vertProperties.size() / numProp;
53+
};
5054
/// Number of triangles
51-
uint32_t NumTri() const { return triVerts.size() / 3; };
55+
uint32_t NumTri() const {
56+
if (vertProperties.size() / numProp >= std::numeric_limits<int>::max())
57+
throw std::out_of_range("mesh too large");
58+
return triVerts.size() / 3;
59+
};
5260

5361
/// Number of properties per vertex, always >= 3.
5462
uint32_t numProp = 3;

src/manifold/src/boolean3.cpp

+23-20
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,9 @@ struct CopyFaceEdges {
7171
SparseIndices &pXq1;
7272
VecView<const Halfedge> halfedgesQ;
7373

74-
void operator()(thrust::tuple<int, int> in) {
74+
void operator()(thrust::tuple<size_t, size_t> in) {
7575
int idx = 3 * thrust::get<0>(in);
76-
int i = thrust::get<1>(in);
76+
size_t i = thrust::get<1>(in);
7777
int pX = p1q1.Get(i, inverted);
7878
int q2 = p1q1.Get(i, !inverted);
7979

@@ -83,7 +83,7 @@ struct CopyFaceEdges {
8383
int a = pX;
8484
int b = edge.IsForward() ? q1 : edge.pairedHalfedge;
8585
if (inverted) std::swap(a, b);
86-
pXq1.Set(idx + j, a, b);
86+
pXq1.Set(idx + static_cast<size_t>(j), a, b);
8787
}
8888
}
8989
};
@@ -92,9 +92,9 @@ SparseIndices Filter11(const Manifold::Impl &inP, const Manifold::Impl &inQ,
9292
const SparseIndices &p1q2, const SparseIndices &p2q1) {
9393
ZoneScoped;
9494
SparseIndices p1q1(3 * p1q2.size() + 3 * p2q1.size());
95-
for_each_n(autoPolicy(p1q2.size()), zip(countAt(0), countAt(0)), p1q2.size(),
96-
CopyFaceEdges<false>({p1q2, p1q1, inQ.halfedge_}));
97-
for_each_n(autoPolicy(p2q1.size()), zip(countAt(p1q2.size()), countAt(0)),
95+
for_each_n(autoPolicy(p1q2.size()), zip(countAt(0_z), countAt(0_z)),
96+
p1q2.size(), CopyFaceEdges<false>({p1q2, p1q1, inQ.halfedge_}));
97+
for_each_n(autoPolicy(p2q1.size()), zip(countAt(p1q2.size()), countAt(0_z)),
9898
p2q1.size(), CopyFaceEdges<true>({p2q1, p1q1, inP.halfedge_}));
9999
p1q1.Unique();
100100
return p1q1;
@@ -137,14 +137,14 @@ inline thrust::pair<int, glm::vec2> Shadow01(
137137

138138
// https://github.com/scandum/binary_search/blob/master/README.md
139139
// much faster than standard binary search on large arrays
140-
int monobound_quaternary_search(VecView<const int64_t> array, int64_t key) {
140+
size_t monobound_quaternary_search(VecView<const int64_t> array, int64_t key) {
141141
if (array.size() == 0) {
142142
return -1;
143143
}
144-
unsigned int bot = 0;
145-
unsigned int top = array.size();
144+
size_t bot = 0;
145+
size_t top = array.size();
146146
while (top >= 65536) {
147-
unsigned int mid = top / 4;
147+
size_t mid = top / 4;
148148
top -= mid * 3;
149149
if (key < array[bot + mid * 2]) {
150150
if (key >= array[bot + mid]) {
@@ -159,7 +159,7 @@ int monobound_quaternary_search(VecView<const int64_t> array, int64_t key) {
159159
}
160160

161161
while (top > 3) {
162-
unsigned int mid = top / 2;
162+
size_t mid = top / 2;
163163
if (key >= array[bot + mid]) {
164164
bot += mid;
165165
}
@@ -183,7 +183,7 @@ struct Kernel11 {
183183
VecView<const glm::vec3> normalP;
184184
const SparseIndices &p1q1;
185185

186-
void operator()(thrust::tuple<int, glm::vec4 &, int &> inout) {
186+
void operator()(thrust::tuple<size_t, glm::vec4 &, int &> inout) {
187187
const int p1 = p1q1.Get(thrust::get<0>(inout), false);
188188
const int q1 = p1q1.Get(thrust::get<0>(inout), true);
189189
glm::vec4 &xyzz11 = thrust::get<1>(inout);
@@ -261,7 +261,7 @@ std::tuple<Vec<int>, Vec<glm::vec4>> Shadow11(SparseIndices &p1q1,
261261
Vec<glm::vec4> xyzz11(p1q1.size());
262262

263263
for_each_n(autoPolicy(p1q1.size()),
264-
zip(countAt(0), xyzz11.begin(), s11.begin()), p1q1.size(),
264+
zip(countAt(0_z), xyzz11.begin(), s11.begin()), p1q1.size(),
265265
Kernel11({inP.vertPos_, inQ.vertPos_, inP.halfedge_, inQ.halfedge_,
266266
expandP, inP.vertNormal_, p1q1}));
267267

@@ -279,7 +279,7 @@ struct Kernel02 {
279279
const SparseIndices &p0q2;
280280
const bool forward;
281281

282-
void operator()(thrust::tuple<int, int &, float &> inout) {
282+
void operator()(thrust::tuple<size_t, int &, float &> inout) {
283283
const int p0 = p0q2.Get(thrust::get<0>(inout), !forward);
284284
const int q2 = p0q2.Get(thrust::get<0>(inout), forward);
285285
int &s02 = thrust::get<1>(inout);
@@ -351,8 +351,8 @@ std::tuple<Vec<int>, Vec<float>> Shadow02(const Manifold::Impl &inP,
351351
Vec<float> z02(p0q2.size());
352352

353353
auto vertNormalP = forward ? inP.vertNormal_ : inQ.vertNormal_;
354-
for_each_n(autoPolicy(p0q2.size()), zip(countAt(0), s02.begin(), z02.begin()),
355-
p0q2.size(),
354+
for_each_n(autoPolicy(p0q2.size()),
355+
zip(countAt(0_z), s02.begin(), z02.begin()), p0q2.size(),
356356
Kernel02({inP.vertPos_, inQ.halfedge_, inQ.vertPos_, expandP,
357357
vertNormalP, p0q2, forward}));
358358

@@ -374,7 +374,7 @@ struct Kernel12 {
374374
const bool forward;
375375
const SparseIndices &p1q2;
376376

377-
void operator()(thrust::tuple<int, int &, glm::vec3 &> inout) {
377+
void operator()(thrust::tuple<size_t, int &, glm::vec3 &> inout) {
378378
int p1 = p1q2.Get(thrust::get<0>(inout), !forward);
379379
int q2 = p1q2.Get(thrust::get<0>(inout), forward);
380380
int &x12 = thrust::get<1>(inout);
@@ -394,7 +394,7 @@ struct Kernel12 {
394394
for (int vert : {edge.startVert, edge.endVert}) {
395395
const int64_t key = forward ? SparseIndices::EncodePQ(vert, q2)
396396
: SparseIndices::EncodePQ(q2, vert);
397-
const int idx = monobound_quaternary_search(p0q2, key);
397+
const size_t idx = monobound_quaternary_search(p0q2, key);
398398
if (idx != -1) {
399399
const int s = s02[idx];
400400
x12 += s * ((vert == edge.startVert) == forward ? 1 : -1);
@@ -415,7 +415,7 @@ struct Kernel12 {
415415
const int q1F = edge.IsForward() ? q1 : edge.pairedHalfedge;
416416
const int64_t key = forward ? SparseIndices::EncodePQ(p1, q1F)
417417
: SparseIndices::EncodePQ(q1F, p1);
418-
const int idx = monobound_quaternary_search(p1q1, key);
418+
const size_t idx = monobound_quaternary_search(p1q1, key);
419419
if (idx != -1) { // s is implicitly zero for anything not found
420420
const int s = s11[idx];
421421
x12 -= s * (edge.IsForward() ? 1 : -1);
@@ -456,7 +456,7 @@ std::tuple<Vec<int>, Vec<glm::vec3>> Intersect12(
456456
Vec<glm::vec3> v12(p1q2.size());
457457

458458
for_each_n(
459-
autoPolicy(p1q2.size()), zip(countAt(0), x12.begin(), v12.begin()),
459+
autoPolicy(p1q2.size()), zip(countAt(0_z), x12.begin(), v12.begin()),
460460
p1q2.size(),
461461
Kernel12({p0q2.AsVec64(), s02, z02, p1q1.AsVec64(), s11, xyzz11,
462462
inP.halfedge_, inQ.halfedge_, inP.vertPos_, forward, p1q2}));
@@ -578,6 +578,9 @@ Boolean3::Boolean3(const Manifold::Impl &inP, const Manifold::Impl &inQ,
578578
Intersect12(inQ, inP, s20, p2q0, s11, p1q1, z20, xyzz11, p2q1_, false);
579579
PRINT("x21 size = " << x21_.size());
580580

581+
if (x12_.size() + x21_.size() >= std::numeric_limits<int>::max())
582+
throw std::out_of_range("mesh too large");
583+
581584
Vec<int> p0 = p0q2.Copy(false);
582585
p0q2.Resize(0);
583586
Vec<int> q0 = p2q0.Copy(true);

src/manifold/src/boolean_result.cpp

+16-7
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ std::tuple<Vec<int>, Vec<int>> SizeOutput(
103103
bool invertQ) {
104104
ZoneScoped;
105105
Vec<int> sidesPerFacePQ(inP.NumTri() + inQ.NumTri(), 0);
106+
// note: numFaceR <= facePQ2R.size() = sidesPerFacePQ.size() + 1
107+
if (sidesPerFacePQ.size() + 1 >= std::numeric_limits<int>::max())
108+
throw std::out_of_range("boolean result too large");
109+
106110
auto sidesPerFaceP = sidesPerFacePQ.view(0, inP.NumTri());
107111
auto sidesPerFaceQ = sidesPerFacePQ.view(inP.NumTri(), inQ.NumTri());
108112

@@ -175,7 +179,7 @@ void AddNewEdgeVerts(
175179
// edge. The direction and duplicity are given by i12, while v12R remaps to
176180
// the output vert index. When forward is false, all is reversed.
177181
auto process = [&](std::function<void(size_t)> lock,
178-
std::function<void(size_t)> unlock, int i) {
182+
std::function<void(size_t)> unlock, size_t i) {
179183
const int edgeP = p1q2.Get(i, !forward);
180184
const int faceQ = p1q2.Get(i, forward);
181185
const int vert = v12R[i];
@@ -218,17 +222,17 @@ void AddNewEdgeVerts(
218222
[&](size_t hash) { mutexes[hash % mutexes.size()].unlock(); },
219223
std::placeholders::_1);
220224
tbb::parallel_for(
221-
tbb::blocked_range<int>(0, p1q2.size(), 32),
222-
[&](const tbb::blocked_range<int> &range) {
223-
for (int i = range.begin(); i != range.end(); i++) processFun(i);
225+
tbb::blocked_range<size_t>(0_z, p1q2.size(), 32),
226+
[&](const tbb::blocked_range<size_t> &range) {
227+
for (size_t i = range.begin(); i != range.end(); i++) processFun(i);
224228
},
225229
ap);
226230
return;
227231
}
228232
#endif
229233
auto processFun = std::bind(
230234
process, [](size_t _) {}, [](size_t _) {}, std::placeholders::_1);
231-
for (int i = 0; i < p1q2.size(); ++i) processFun(i);
235+
for (size_t i = 0; i < p1q2.size(); ++i) processFun(i);
232236
}
233237

234238
std::vector<Halfedge> PairUp(std::vector<EdgePos> &edgePos) {
@@ -239,15 +243,15 @@ std::vector<Halfedge> PairUp(std::vector<EdgePos> &edgePos) {
239243
// a heuristic.
240244
ASSERT(edgePos.size() % 2 == 0, topologyErr,
241245
"Non-manifold edge! Not an even number of points.");
242-
int nEdges = edgePos.size() / 2;
246+
size_t nEdges = edgePos.size() / 2;
243247
auto middle = std::partition(edgePos.begin(), edgePos.end(),
244248
[](EdgePos x) { return x.isStart; });
245249
ASSERT(middle - edgePos.begin() == nEdges, topologyErr, "Non-manifold edge!");
246250
auto cmp = [](EdgePos a, EdgePos b) { return a.edgePos < b.edgePos; };
247251
std::stable_sort(edgePos.begin(), middle, cmp);
248252
std::stable_sort(middle, edgePos.end(), cmp);
249253
std::vector<Halfedge> edges;
250-
for (int i = 0; i < nEdges; ++i)
254+
for (size_t i = 0; i < nEdges; ++i)
251255
edges.push_back({edgePos[i].vert, edgePos[i + nEdges].vert, -1, -1});
252256
return edges;
253257
}
@@ -539,6 +543,11 @@ void CreateProperties(Manifold::Impl &outR, const Manifold::Impl &inP,
539543
std::vector<int> propMissIdx[2];
540544
propMissIdx[0].resize(inQ.NumPropVert(), -1);
541545
propMissIdx[1].resize(inP.NumPropVert(), -1);
546+
547+
if (static_cast<size_t>(outR.NumVert()) * static_cast<size_t>(numProp) >=
548+
std::numeric_limits<int>::max())
549+
throw std::out_of_range("too many vertices");
550+
542551
outR.meshRelation_.properties.reserve(outR.NumVert() * numProp);
543552
int idx = 0;
544553

src/manifold/src/edge_op.cpp

+11-8
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,9 @@ namespace manifold {
145145
void Manifold::Impl::SimplifyTopology() {
146146
if (!halfedge_.size()) return;
147147

148-
const int nbEdges = halfedge_.size();
148+
const size_t nbEdges = halfedge_.size();
149149
auto policy = autoPolicy(nbEdges);
150-
int numFlagged = 0;
150+
size_t numFlagged = 0;
151151
Vec<uint8_t> bflags(nbEdges);
152152

153153
// In the case of a very bad triangulation, it is possible to create pinched
@@ -191,8 +191,9 @@ void Manifold::Impl::SimplifyTopology() {
191191
ZoneScopedN("CollapseShortEdge");
192192
numFlagged = 0;
193193
ShortEdge se{halfedge_, vertPos_, precision_};
194-
for_each_n(policy, countAt(0), nbEdges, [&](int i) { bflags[i] = se(i); });
195-
for (int i = 0; i < nbEdges; ++i) {
194+
for_each_n(policy, countAt(0_z), nbEdges,
195+
[&](size_t i) { bflags[i] = se(i); });
196+
for (size_t i = 0; i < nbEdges; ++i) {
196197
if (bflags[i]) {
197198
CollapseEdge(i, scratchBuffer);
198199
scratchBuffer.resize(0);
@@ -212,8 +213,9 @@ void Manifold::Impl::SimplifyTopology() {
212213
ZoneScopedN("CollapseFlaggedEdge");
213214
numFlagged = 0;
214215
FlagEdge se{halfedge_, meshRelation_.triRef};
215-
for_each_n(policy, countAt(0), nbEdges, [&](int i) { bflags[i] = se(i); });
216-
for (int i = 0; i < nbEdges; ++i) {
216+
for_each_n(policy, countAt(0_z), nbEdges,
217+
[&](size_t i) { bflags[i] = se(i); });
218+
for (size_t i = 0; i < nbEdges; ++i) {
217219
if (bflags[i]) {
218220
CollapseEdge(i, scratchBuffer);
219221
scratchBuffer.resize(0);
@@ -233,11 +235,12 @@ void Manifold::Impl::SimplifyTopology() {
233235
ZoneScopedN("RecursiveEdgeSwap");
234236
numFlagged = 0;
235237
SwappableEdge se{halfedge_, vertPos_, faceNormal_, precision_};
236-
for_each_n(policy, countAt(0), nbEdges, [&](int i) { bflags[i] = se(i); });
238+
for_each_n(policy, countAt(0_z), nbEdges,
239+
[&](size_t i) { bflags[i] = se(i); });
237240
std::vector<int> edgeSwapStack;
238241
std::vector<int> visited(halfedge_.size(), -1);
239242
int tag = 0;
240-
for (int i = 0; i < nbEdges; ++i) {
243+
for (size_t i = 0; i < nbEdges; ++i) {
241244
if (bflags[i]) {
242245
numFlagged++;
243246
tag++;

src/manifold/src/face_op.cpp

+8-6
Original file line numberDiff line numberDiff line change
@@ -136,12 +136,12 @@ void Manifold::Impl::Face2Tri(const Vec<int>& faceEdge,
136136
tbb::task_group group;
137137
// map from face to triangle
138138
tbb::concurrent_unordered_map<int, std::vector<glm::ivec3>> results;
139-
Vec<int> triCount(faceEdge.size());
139+
Vec<size_t> triCount(faceEdge.size());
140140
triCount.back() = 0;
141141
// precompute number of triangles per face, and launch async tasks to
142142
// triangulate complex faces
143-
for_each(autoPolicy(faceEdge.size()), countAt(0),
144-
countAt(faceEdge.size() - 1), [&](int face) {
143+
for_each(autoPolicy(faceEdge.size()), countAt(0_z),
144+
countAt(faceEdge.size() - 1), [&](size_t face) {
145145
triCount[face] = faceEdge[face + 1] - faceEdge[face] - 2;
146146
ASSERT(triCount[face] >= 1, topologyErr,
147147
"face has less than three edges.");
@@ -155,13 +155,15 @@ void Manifold::Impl::Face2Tri(const Vec<int>& faceEdge,
155155
group.wait();
156156
// prefix sum computation (assign unique index to each face) and preallocation
157157
exclusive_scan(autoPolicy(triCount.size()), triCount.begin(), triCount.end(),
158-
triCount.begin(), 0);
158+
triCount.begin(), 0_z);
159+
if (triCount.back() >= std::numeric_limits<int>::max())
160+
throw std::out_of_range("too many triangles");
159161
triVerts.resize(triCount.back());
160162
triNormal.resize(triCount.back());
161163
triRef.resize(triCount.back());
162164

163165
auto processFace2 = std::bind(
164-
processFace, [&](int face) { return std::move(results[face]); },
166+
processFace, [&](size_t face) { return std::move(results[face]); },
165167
[&](int face, glm::ivec3 tri, glm::vec3 normal, TriRef r) {
166168
triVerts[triCount[face]] = tri;
167169
triNormal[triCount[face]] = normal;
@@ -170,7 +172,7 @@ void Manifold::Impl::Face2Tri(const Vec<int>& faceEdge,
170172
},
171173
std::placeholders::_1);
172174
// set triangles in parallel
173-
for_each(autoPolicy(faceEdge.size()), countAt(0),
175+
for_each(autoPolicy(faceEdge.size()), countAt(0_z),
174176
countAt(faceEdge.size() - 1), processFace2);
175177
#else
176178
triVerts.reserve(faceEdge.size());

0 commit comments

Comments
 (0)