Skip to content

Commit 12bf416

Browse files
syoyosyoyo
committed
Fix assertion when there is no normal/texcoord assigned to the face. Fixes #161.
Suppress clang warnings.
1 parent f206a56 commit 12bf416

File tree

4 files changed

+206
-110
lines changed

4 files changed

+206
-110
lines changed

‎examples/viewer/viewer.cc‎

Lines changed: 148 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@ extern "C" {
3737
#include <windows.h>
3838

3939
#ifdef max
40-
#undef max
40+
#undef max
4141
#endif
4242

4343
#ifdef min
44-
#undef min
44+
#undef min
4545
#endif
4646

4747
#include <mmsystem.h>
@@ -143,15 +143,15 @@ float eye[3], lookat[3], up[3];
143143

144144
GLFWwindow* window;
145145

146-
static std::string GetBaseDir(const std::string &filepath) {
146+
static std::string GetBaseDir(const std::string& filepath) {
147147
if (filepath.find_last_of("/\\") != std::string::npos)
148148
return filepath.substr(0, filepath.find_last_of("/\\"));
149149
return "";
150150
}
151151

152-
static bool FileExists(const std::string &abs_filename) {
152+
static bool FileExists(const std::string& abs_filename) {
153153
bool ret;
154-
FILE *fp = fopen(abs_filename.c_str(), "rb");
154+
FILE* fp = fopen(abs_filename.c_str(), "rb");
155155
if (fp) {
156156
ret = true;
157157
fclose(fp);
@@ -195,10 +195,10 @@ static void CalcNormal(float N[3], float v0[3], float v1[3], float v2[3]) {
195195
}
196196

197197
static bool LoadObjAndConvert(float bmin[3], float bmax[3],
198-
std::vector<DrawObject>* drawObjects,
199-
std::vector<tinyobj::material_t>& materials,
200-
std::map<std::string, GLuint>& textures,
201-
const char* filename) {
198+
std::vector<DrawObject>* drawObjects,
199+
std::vector<tinyobj::material_t>& materials,
200+
std::map<std::string, GLuint>& textures,
201+
const char* filename) {
202202
tinyobj::attrib_t attrib;
203203
std::vector<tinyobj::shape_t> shapes;
204204

@@ -217,8 +217,8 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
217217
#endif
218218

219219
std::string err;
220-
bool ret =
221-
tinyobj::LoadObj(&attrib, &shapes, &materials, &err, filename, base_dir.c_str());
220+
bool ret = tinyobj::LoadObj(&attrib, &shapes, &materials, &err, filename,
221+
base_dir.c_str());
222222
if (!err.empty()) {
223223
std::cerr << err << std::endl;
224224
}
@@ -242,56 +242,62 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
242242
materials.push_back(tinyobj::material_t());
243243

244244
for (size_t i = 0; i < materials.size(); i++) {
245-
printf("material[%d].diffuse_texname = %s\n", int(i), materials[i].diffuse_texname.c_str());
245+
printf("material[%d].diffuse_texname = %s\n", int(i),
246+
materials[i].diffuse_texname.c_str());
246247
}
247248

248249
// Load diffuse textures
249250
{
250-
for (size_t m = 0; m < materials.size(); m++) {
251-
tinyobj::material_t* mp = &materials[m];
252-
253-
if (mp->diffuse_texname.length() > 0) {
254-
// Only load the texture if it is not already loaded
255-
if (textures.find(mp->diffuse_texname) == textures.end()) {
256-
GLuint texture_id;
257-
int w, h;
258-
int comp;
259-
260-
std::string texture_filename = mp->diffuse_texname;
261-
if (!FileExists(texture_filename)) {
262-
// Append base dir.
263-
texture_filename = base_dir + mp->diffuse_texname;
264-
if (!FileExists(texture_filename)) {
265-
std::cerr << "Unable to find file: " << mp->diffuse_texname << std::endl;
266-
exit(1);
267-
}
268-
}
269-
270-
unsigned char* image = stbi_load(texture_filename.c_str(), &w, &h, &comp, STBI_default);
271-
if (!image) {
272-
std::cerr << "Unable to load texture: " << texture_filename << std::endl;
273-
exit(1);
274-
}
275-
std::cout << "Loaded texture: " << texture_filename << ", w = " << w << ", h = " << h << ", comp = " << comp << std::endl;
276-
277-
glGenTextures(1, &texture_id);
278-
glBindTexture(GL_TEXTURE_2D, texture_id);
279-
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
280-
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
281-
if (comp == 3) {
282-
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, w, h, 0, GL_RGB, GL_UNSIGNED_BYTE, image);
283-
}
284-
else if (comp == 4) {
285-
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, w, h, 0, GL_RGBA, GL_UNSIGNED_BYTE, image);
286-
} else {
287-
assert(0); // TODO
288-
}
289-
glBindTexture(GL_TEXTURE_2D, 0);
290-
stbi_image_free(image);
291-
textures.insert(std::make_pair(mp->diffuse_texname, texture_id));
292-
}
251+
for (size_t m = 0; m < materials.size(); m++) {
252+
tinyobj::material_t* mp = &materials[m];
253+
254+
if (mp->diffuse_texname.length() > 0) {
255+
// Only load the texture if it is not already loaded
256+
if (textures.find(mp->diffuse_texname) == textures.end()) {
257+
GLuint texture_id;
258+
int w, h;
259+
int comp;
260+
261+
std::string texture_filename = mp->diffuse_texname;
262+
if (!FileExists(texture_filename)) {
263+
// Append base dir.
264+
texture_filename = base_dir + mp->diffuse_texname;
265+
if (!FileExists(texture_filename)) {
266+
std::cerr << "Unable to find file: " << mp->diffuse_texname
267+
<< std::endl;
268+
exit(1);
269+
}
270+
}
271+
272+
unsigned char* image =
273+
stbi_load(texture_filename.c_str(), &w, &h, &comp, STBI_default);
274+
if (!image) {
275+
std::cerr << "Unable to load texture: " << texture_filename
276+
<< std::endl;
277+
exit(1);
278+
}
279+
std::cout << "Loaded texture: " << texture_filename << ", w = " << w
280+
<< ", h = " << h << ", comp = " << comp << std::endl;
281+
282+
glGenTextures(1, &texture_id);
283+
glBindTexture(GL_TEXTURE_2D, texture_id);
284+
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
285+
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
286+
if (comp == 3) {
287+
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, w, h, 0, GL_RGB,
288+
GL_UNSIGNED_BYTE, image);
289+
} else if (comp == 4) {
290+
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, w, h, 0, GL_RGBA,
291+
GL_UNSIGNED_BYTE, image);
292+
} else {
293+
assert(0); // TODO
293294
}
295+
glBindTexture(GL_TEXTURE_2D, 0);
296+
stbi_image_free(image);
297+
textures.insert(std::make_pair(mp->diffuse_texname, texture_id));
298+
}
294299
}
300+
}
295301
}
296302

297303
bmin[0] = bmin[1] = bmin[2] = std::numeric_limits<float>::max();
@@ -305,26 +311,43 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
305311
tinyobj::index_t idx0 = shapes[s].mesh.indices[3 * f + 0];
306312
tinyobj::index_t idx1 = shapes[s].mesh.indices[3 * f + 1];
307313
tinyobj::index_t idx2 = shapes[s].mesh.indices[3 * f + 2];
308-
314+
309315
int current_material_id = shapes[s].mesh.material_ids[f];
310316

311-
if ((current_material_id < 0) || (current_material_id >= static_cast<int>(materials.size()))) {
317+
if ((current_material_id < 0) ||
318+
(current_material_id >= static_cast<int>(materials.size()))) {
312319
// Invaid material ID. Use default material.
313-
current_material_id = materials.size() - 1; // Default material is added to the last item in `materials`.
320+
current_material_id =
321+
materials.size() -
322+
1; // Default material is added to the last item in `materials`.
314323
}
315-
//if (current_material_id >= materials.size()) {
316-
// std::cerr << "Invalid material index: " << current_material_id << std::endl;
324+
// if (current_material_id >= materials.size()) {
325+
// std::cerr << "Invalid material index: " << current_material_id <<
326+
// std::endl;
317327
//}
318328
//
319329
float diffuse[3];
320330
for (size_t i = 0; i < 3; i++) {
321-
diffuse[i] = materials[current_material_id].diffuse[i];
331+
diffuse[i] = materials[current_material_id].diffuse[i];
322332
}
323333
float tc[3][2];
324334
if (attrib.texcoords.size() > 0) {
325-
assert(attrib.texcoords.size() > 2 * idx0.texcoord_index + 1);
326-
assert(attrib.texcoords.size() > 2 * idx1.texcoord_index + 1);
327-
assert(attrib.texcoords.size() > 2 * idx2.texcoord_index + 1);
335+
if ((idx0.texcoord_index < 0) || (idx1.texcoord_index < 0) ||
336+
(idx2.texcoord_index < 0)) {
337+
// face does not contain valid uv index.
338+
tc[0][0] = 0.0f;
339+
tc[0][1] = 0.0f;
340+
tc[1][0] = 0.0f;
341+
tc[1][1] = 0.0f;
342+
tc[2][0] = 0.0f;
343+
tc[2][1] = 0.0f;
344+
} else {
345+
assert(attrib.texcoords.size() >
346+
size_t(2 * idx0.texcoord_index + 1));
347+
assert(attrib.texcoords.size() >
348+
size_t(2 * idx1.texcoord_index + 1));
349+
assert(attrib.texcoords.size() >
350+
size_t(2 * idx2.texcoord_index + 1));
328351

329352
// Flip Y coord.
330353
tc[0][0] = attrib.texcoords[2 * idx0.texcoord_index];
@@ -333,13 +356,14 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
333356
tc[1][1] = 1.0f - attrib.texcoords[2 * idx1.texcoord_index + 1];
334357
tc[2][0] = attrib.texcoords[2 * idx2.texcoord_index];
335358
tc[2][1] = 1.0f - attrib.texcoords[2 * idx2.texcoord_index + 1];
359+
}
336360
} else {
337-
tc[0][0] = 0.0f;
338-
tc[0][1] = 0.0f;
339-
tc[1][0] = 0.0f;
340-
tc[1][1] = 0.0f;
341-
tc[2][0] = 0.0f;
342-
tc[2][1] = 0.0f;
361+
tc[0][0] = 0.0f;
362+
tc[0][1] = 0.0f;
363+
tc[1][0] = 0.0f;
364+
tc[1][1] = 0.0f;
365+
tc[2][0] = 0.0f;
366+
tc[2][1] = 0.0f;
343367
}
344368

345369
float v[3][3];
@@ -363,27 +387,40 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
363387
}
364388

365389
float n[3][3];
366-
if (attrib.normals.size() > 0) {
367-
int f0 = idx0.normal_index;
368-
int f1 = idx1.normal_index;
369-
int f2 = idx2.normal_index;
370-
assert(f0 >= 0);
371-
assert(f1 >= 0);
372-
assert(f2 >= 0);
373-
for (int k = 0; k < 3; k++) {
374-
n[0][k] = attrib.normals[3 * f0 + k];
375-
n[1][k] = attrib.normals[3 * f1 + k];
376-
n[2][k] = attrib.normals[3 * f2 + k];
390+
{
391+
bool invalid_normal_index = false;
392+
if (attrib.normals.size() > 0) {
393+
int nf0 = idx0.normal_index;
394+
int nf1 = idx1.normal_index;
395+
int nf2 = idx2.normal_index;
396+
397+
if ((nf0 < 0) || (nf1 < 0) || (nf2 < 0)) {
398+
// normal index is missing from this face.
399+
invalid_normal_index = true;
400+
} else {
401+
for (int k = 0; k < 3; k++) {
402+
assert(size_t(3 * nf0 + k) < attrib.normals.size());
403+
assert(size_t(3 * nf1 + k) < attrib.normals.size());
404+
assert(size_t(3 * nf2 + k) < attrib.normals.size());
405+
n[0][k] = attrib.normals[3 * nf0 + k];
406+
n[1][k] = attrib.normals[3 * nf1 + k];
407+
n[2][k] = attrib.normals[3 * nf2 + k];
408+
}
409+
}
410+
} else {
411+
invalid_normal_index = true;
412+
}
413+
414+
if (invalid_normal_index) {
415+
// compute geometric normal
416+
CalcNormal(n[0], v[0], v[1], v[2]);
417+
n[1][0] = n[0][0];
418+
n[1][1] = n[0][1];
419+
n[1][2] = n[0][2];
420+
n[2][0] = n[0][0];
421+
n[2][1] = n[0][1];
422+
n[2][2] = n[0][2];
377423
}
378-
} else {
379-
// compute geometric normal
380-
CalcNormal(n[0], v[0], v[1], v[2]);
381-
n[1][0] = n[0][0];
382-
n[1][1] = n[0][1];
383-
n[1][2] = n[0][2];
384-
n[2][0] = n[0][0];
385-
n[2][1] = n[0][1];
386-
n[2][2] = n[0][2];
387424
}
388425

389426
for (int k = 0; k < 3; k++) {
@@ -396,11 +433,9 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
396433
// Combine normal and diffuse to get color.
397434
float normal_factor = 0.2;
398435
float diffuse_factor = 1 - normal_factor;
399-
float c[3] = {
400-
n[k][0] * normal_factor + diffuse[0] * diffuse_factor,
401-
n[k][1] * normal_factor + diffuse[1] * diffuse_factor,
402-
n[k][2] * normal_factor + diffuse[2] * diffuse_factor
403-
};
436+
float c[3] = {n[k][0] * normal_factor + diffuse[0] * diffuse_factor,
437+
n[k][1] * normal_factor + diffuse[1] * diffuse_factor,
438+
n[k][2] * normal_factor + diffuse[2] * diffuse_factor};
404439
float len2 = c[0] * c[0] + c[1] * c[1] + c[2] * c[2];
405440
if (len2 > 0.0f) {
406441
float len = sqrtf(len2);
@@ -412,7 +447,7 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
412447
buffer.push_back(c[0] * 0.5 + 0.5);
413448
buffer.push_back(c[1] * 0.5 + 0.5);
414449
buffer.push_back(c[2] * 0.5 + 0.5);
415-
450+
416451
buffer.push_back(tc[k][0]);
417452
buffer.push_back(tc[k][1]);
418453
}
@@ -422,19 +457,22 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
422457
o.numTriangles = 0;
423458

424459
// OpenGL viewer does not support texturing with per-face material.
425-
if (shapes[s].mesh.material_ids.size() > 0 && shapes[s].mesh.material_ids.size() > s) {
426-
o.material_id = shapes[s].mesh.material_ids[0]; // use the material ID of the first face.
460+
if (shapes[s].mesh.material_ids.size() > 0 &&
461+
shapes[s].mesh.material_ids.size() > s) {
462+
o.material_id = shapes[s].mesh.material_ids[0]; // use the material ID
463+
// of the first face.
427464
} else {
428-
o.material_id = materials.size() - 1; // = ID for default material.
465+
o.material_id = materials.size() - 1; // = ID for default material.
429466
}
430467
printf("shape[%d] material_id %d\n", int(s), int(o.material_id));
431-
468+
432469
if (buffer.size() > 0) {
433470
glGenBuffers(1, &o.vb_id);
434471
glBindBuffer(GL_ARRAY_BUFFER, o.vb_id);
435-
glBufferData(GL_ARRAY_BUFFER, buffer.size() * sizeof(float), &buffer.at(0),
436-
GL_STATIC_DRAW);
437-
o.numTriangles = buffer.size() / (3 + 3 + 3 + 2) / 3; // 3:vtx, 3:normal, 3:col, 2:texcoord
472+
glBufferData(GL_ARRAY_BUFFER, buffer.size() * sizeof(float),
473+
&buffer.at(0), GL_STATIC_DRAW);
474+
o.numTriangles = buffer.size() / (3 + 3 + 3 + 2) /
475+
3; // 3:vtx, 3:normal, 3:col, 2:texcoord
438476

439477
printf("shape[%d] # of triangles = %d\n", static_cast<int>(s),
440478
o.numTriangles);
@@ -467,7 +505,7 @@ static void reshapeFunc(GLFWwindow* window, int w, int h) {
467505
}
468506

469507
static void keyboardFunc(GLFWwindow* window, int key, int scancode, int action,
470-
int mods) {
508+
int mods) {
471509
(void)window;
472510
(void)scancode;
473511
(void)mods;
@@ -549,7 +587,9 @@ static void motionFunc(GLFWwindow* window, double mouse_x, double mouse_y) {
549587
prevMouseY = mouse_y;
550588
}
551589

552-
static void Draw(const std::vector<DrawObject>& drawObjects, std::vector<tinyobj::material_t>& materials, std::map<std::string, GLuint>& textures) {
590+
static void Draw(const std::vector<DrawObject>& drawObjects,
591+
std::vector<tinyobj::material_t>& materials,
592+
std::map<std::string, GLuint>& textures) {
553593
glPolygonMode(GL_FRONT, GL_FILL);
554594
glPolygonMode(GL_BACK, GL_FILL);
555595

@@ -572,7 +612,7 @@ static void Draw(const std::vector<DrawObject>& drawObjects, std::vector<tinyobj
572612
if ((o.material_id < materials.size())) {
573613
std::string diffuse_texname = materials[o.material_id].diffuse_texname;
574614
if (textures.find(diffuse_texname) != textures.end()) {
575-
glBindTexture(GL_TEXTURE_2D, textures[diffuse_texname]);
615+
glBindTexture(GL_TEXTURE_2D, textures[diffuse_texname]);
576616
}
577617
}
578618
glVertexPointer(3, GL_FLOAT, stride, (const void*)0);
@@ -668,7 +708,8 @@ int main(int argc, char** argv) {
668708
float bmin[3], bmax[3];
669709
std::vector<tinyobj::material_t> materials;
670710
std::map<std::string, GLuint> textures;
671-
if (false == LoadObjAndConvert(bmin, bmax, &gDrawObjects, materials, textures, argv[1])) {
711+
if (false == LoadObjAndConvert(bmin, bmax, &gDrawObjects, materials, textures,
712+
argv[1])) {
672713
return -1;
673714
}
674715

0 commit comments

Comments
 (0)