To get the version of the code as described in this post you can pull a copy of the code from the archive using the tag OBJ.
The writing, and (consequently) the reading of the text below is complicated by using the word "object" to mean different things. In some cases the word object is used to mean an instance of a C++ data class type; in other cases it refers to a thing to be drawn by the sample program. For example, in chapter 3 the sample programs draw the Momo object. I've tried to clarify whether I'm talking about the instantiation of a C++ class or a thing to be drawn but I may have missed places and I'm hoping this warning will help the reader to remember when "decoding" this post that "object" has more than one meaning.
The OBJ data type, unsurprisingly, is implemented in the SDK/common/obj.cpp and SDK/common/obj.h files. Starting with the header file note that the file defines the data types OBJMATERIAL, OBJTRIANGLEINDEX, OBJTRIANGLELIST, OBJVERTEXDATA, and OBJMESH, as well as the data type OBJ. I have converted all of these data types into classes. The types OBJTRIANGLEINDEX and OBJVERTEXDATA only have constructors; if needed, they depend on the compiler to create default destructors. The OBJ data type only has a constructor and a destructor. The remaining types all have at least one constructor, a destructor, a copy constructor, and an assignment operator.
Before beginning in earnest I want to point out that at the top of the header file I added a forward reference to the OBJ class by adding the line
struct OBJ;
I'll explain why I needed this forward reference later in this post.
Every data member in the header file which is declared as unsigned char, except n_objtrianglelist (it's going to be deleted below, anyway), should be changed to bool. Also, the return type for OBJ_load_mtl() should be changed to bool. Lastly, unsigned char should be changed to bool every place the type appears in a function parameter list declaration. Correspondingly, I changed uses of 0 (zero), and 1 (one) to false, and true, respectively, when those values are used to initialize bool variables.
OBJTRIANGLEINDEX, and OBJVERTEXDATA are so simple that there probably isn't any need for comment. In fact, I almost didn't make them classes at all. The reason I did make them into classes is that OBJMESH, and OBJTRIANGLELIST manage sets of these data types as instances of std::vector. By making these two data types into classes it simplifies adding entries to the std::vectors; the code uses a constructor to create and initialize these data objects at the same time the code uses the std::vector::push_back() method to add the OBJTRIANGLEINDEX or OBJVERTEXDATA objects to the array. See the methods OBJMESH::add_vertex_data() and OBJ::OBJ().
The changes to OBJTRIANGLELIST are the next simplest case. As before, I changed its declaration from being a typedef to a named struct so we can use the name OBJTRIANGLELIST as a class name. If you look at the definition of OBJTRIANGLELIST in the header file you'll see a familiar pattern. The data members n_objtriangleindex and objtriangleindex are used to implement a dynamically sized/allocated array of OBJTRIANGLEINDEX entries. Likewise, n_indice_array and indice_array are used to implement a dynamically sized/allocated array of unsigned short integers. My first impulse was to reimplement data member pairs as:
std::vector<OBJTRIANGLEINDEX> objtriangleindex;
and
std::vector<unsigned short> indice_array;
respectively, deleting n_objtriangleindex, and n_indice_array. This works fine for objtriangleindex; it introduces a bug for indice_array. The bug isn't with indice_array itself; the bug is caused by using indice_array.size() as a substitute for n_indice_array. In some of the sample programs later in the book the original code frees (malloc(3)) the space allocated for indice_array but keeps the old value of n_indice_array. The code then passes n_indice_array as a parameter to an OpenGL call. In these cases when indice_array.size() is substituted for n_indice_array, because the contents of indice_array have been cleared, indice_array.size() evaluates to 0 (zero) and chaos ensues. The two obvious solutions are to keep the old code which uses malloc(3) to allocate/deallocate space or to keep around the variable n_indice_array but still implement indice_array using std::vector. I chose the latter and explained this choice with a comment in the header file.
[Note: This is an important tenant of writing software for me. I left n_indice_array in the code on purpose. Someone else coming along behind me to maintain the code may think that what I've done is an oversight , and change the code without understanding the impact. After all, none of the other cases of replacing a counter and a pointer with an instance of std::vector to dynamically allocate space need to retain the old counter. The comment in the header file helps prevent this.]
The constructor and destructor for OBJTRIANGLELIST are unremarkable implementations. The copy constructor and the assignment operator are a bit more interesting. These two methods do a "deep copy" of the data members, i.e., the code dynamically allocates space to hold the objtriangleindex array and indice_array in the OBJTRIANGLELIST which is being constructed or assigned to. The implementation sounds more complicated that it is; the copy constructor and the assignment operator use the methods built into the std::vector template; these methods hide all of the implementation details from the programmer.
Before discussing the specifics of rewriting the OBJMATERIAL, OBJMESH, and OBJ data types let me take a step back to discuss the relationship of these data types to each other. The OBJ data type is the top level container for the 3D (graphics) objects which will be drawn in the various sample programs. Each OBJ object can own several OBJMESH and OBJMATERIAL objects. The OBJMESH data object holds a geometric description (the "mesh") for a graphics object to be draw on the screen and it also has a pointer to the OBJMATERIAL which is currently being used to render (i.e., draw) its mesh. Every OBJMESH and OBJMATERIAL (data) object is "owned" by an OBJ "parent". In the original version of the code the OBJMESH and OBJMATERIAL data objects didn't need to know which OBJ data object was their parent. As I've rewritten the code the OBJMESH and OBJMATERIAL data objects do need to know which OBJ object is their parent. This is the reason for adding the forward reference to the OBJ class near the top of the header file: I have added a new data member "OBJ *parent" to both the OBJMESH and OBJMATERIAL data objects.
[Note: When I "push" the code for my next post you'll see that I've actually changed the type of this new member in both OBJMATERIAL, and OBJMESH to "const OBJ *parent". More on this below.]
Continuing with the changes to the OBJ data type the declarations
std::vector<OBJMESH> objmesh;
std::vector<OBJMATERIAL> objmaterial;
std::vector<TEXTURE *> texture;
std::vector<PROGRAM *> program;
std::vector<vec3> indexed_vertex;
std::vector<vec3> indexed_normal;
std::vector<vec3> indexed_fnormal;
std::vector<vec3> indexed_tangent;
std::vector<vec2> indexed_uv;
replace the original declarations of these data members and eliminate the need for the data members n_objmesh, n_objmaterial, n_texture, n_program, n_indexed_vertex, and n_indexed_uv. Details of how these changes are performed can be found in an earlier post.
Now that the changes to the data members of the OBJ class are complete I'll begin on the class methods. For the previous class types (SHADER and PROGRAM) I was able to use the THING_init() functions, and/or the THING_create() function as the basis of class constructors. The original code has no OBJ_init() or OBJ_create() functions. If you look at the SDK/chapter3‑1/templateApp.cpp file for the sample program chapter3‑1 you'll see that the call to OBJ_load() is used to allocate and initialize an OBJ object. Consequently, I used OBJ_load() as the basis for a class constructor. Remember that new allocates the space for us automatically so the call to calloc(3) needs to be removed from the body of the function.
Like previous modules OBJ_free() is used as the basis of the destructor. For each of the data members which have been redeclared using the std::vector type template the destructor uses the clear() member function to release the space. This probably isn't really needed but it mimics the operation of the original code. Before applying clear() to the std::vector objmesh, the destructor looks to see if the vao and vbo data members of each OBJMESH instance are initialized, and if they are, they are released using OpenGL calls. Also, the destructor looks through each entry of the objtrianglelist data member of the OBJMESH entries to see if the OBJTRIANGLELIST's vbo data member is initialized, and, if needed, releases it. Note that these values are created using OpenGL; it's likely that we can/should change their data types to be data types declared by OpenGL. For the vectors of "TEXTURE *" and "PROGRAM *" objects each of the objects must be released before releasing the space held by the vectors themselves; the std::vector::clear() method won't (and can't) do this for us since the template library doesn't know each application's data retention policies.
[Note: If we used C++ 11 shared_ptrs we might save ourselves some work and avoid bugs. Hopefully this will be covered in a future post.]
The following functions all become OBJ member functions:
- OBJ_get_mesh()
- OBJ_get_mesh_index()
- OBJ_get_program()
- OBJ_get_material()
- OBJ_get_texture()
- OBJ_load_mtl()
- OBJ_free_vertex_data()
As before, I stripped off the "OBJ_" prefix from the names and removed the first argument from the parameter lists because the first parameter will be passed into the method as this. You might have noticed that this list omits the functions:
- OBJ_build_texture()
- OBJ_build_program()
- OBJ_build_material()
- OBJ_set_draw_callback_material()
- OBJ_update_bound_mesh()
- OBJ_build_vbo_mesh()
- OBJ_set_attributes_mesh()
- OBJ_build_mesh()
- OBJ_build_mesh2()
- OBJ_optimize_mesh()
- OBJ_draw_material()
- OBJ_draw_mesh()
- OBJ_draw_mesh2()
- OBJ_draw_mesh3()
- OBJ_free_mesh_vertex_data()
[Note: I'll admit I'm torn about whether OBJ_draw_mesh2() and OBJ_draw_mesh3() shouldn't have been left as OBJ class methods. Perhaps there will be more on that in a later post but these don't appear to be used anywhere in the sample code from the book so sorting this out was a problem for me.]
The various "get" methods (OBJ::get_mesh(), OBJ::get_mesh_index(), OBJ::get_program(), OBJ::get_material(), and OBJ::get_texture()) can be implemented more optimally. As an example I'll discuss only the OBJ::get_mesh() method; the changes to the others all follow an identical pattern. To start, I changed the code from using a while loop to using an iterated for loop. The bool parameter exact_name is passed into the function and never changes (in fact, it should be declared in the parameter list as const bool and will be so in a future revision of the code). Its value is used inside the loop to determine which match criterion is used. Since exact_name never changes during the execution of the loop, each time the loop is executed the CPU will follow the same path through the loop for every iteration. This makes testing the value of exact_match inside the loop wasted effort. The code could test the value of exact_match once outside the loop. This changes the code
for (int i=0; i != this->objmesh.size(); ++i) {
if (exact_match) {
if (!strcmp(this->objmesh[i].name, name)
return i;
} else {
if (!strstr(this->objmesh[i].name, name)
return i;
}
}
into
if (exact_match) {
for (int i=0; i != this->objmesh.size(); ++i) {
if (!strcmp(this->objmesh[i].name, name)
return i;
}
} else {
for (int i=0; i != this->objmesh.size(); ++i) {
if (!strstr(this->objmesh[i].name, name)
return i;
}
}
Notice that the new version of the code is a little longer than the original code. This is a common trade off between code speed and code size. Pick your poison.
Continuing with the implementation of the OBJ class you may have noticed that there are four functions in the file that weren't defined in the header file:
Continuing with the implementation of the OBJ class you may have noticed that there are four functions in the file that weren't defined in the header file:
- OBJ_get_texture_index(),
- OBJ_add_texture(),
- OBJ_get_program_index(), and
- OBJ_add_program().
In the header file I declared their class method analogs (OBJ::get_texture_index(), etc.) to be private. These routines are only ever used directly by other methods in this file.
The remainder of the implementation of the OBJ class consists of things which have been discussed before, and will be omitted here for brevity.
Moving on to OBJMATERIAL the changes in the header file to data members have been covered before, with the exception of adding a pointer to the OBJMATERIAL instance's parent. Before discussing that let me go back to the list of "orphan" functions which I asserted aren't really part of the OBJ class. I think it's easiest to make the case that the OBJ_draw_material() function is misnamed; I think it should have been named OBJMATERIAL_draw_material(). Ignoring the "OBJ_" prefix of its name, look at the parameter list:
OBJ_draw_material( OBJMATERIAL *objmaterial );
and then look at the body of the function. Inside the function there isn't a single reference to an OBJ data object; all of the work inside this function is done using OBJMATERIAL's data members.
The argument that OBJ_set_draw_callback_material() is also really an OBJMATERIAL member function requires only a little more work. Start by looking at its argument list:
void OBJ_set_draw_callback_material( OBJ *obj,
unsigned int material_index,
MATERIALDRAWCALLBACK *materialdrawcallback );
Even though the first argument is a pointer to an OBJ instance the second argument is used as an index into the objmaterial data member of the OBJ instance to select which OBJMATERIAL entry in the objmaterial array of the OBJ instance is operated on. Another way to state this is that these two arguments don't specify which OBJ the method should work on, but rather, which OBJMATERIAL it should work on. And when we examine the body of the function again we see that the code changes a data member of OBJMATERIAL; the data members of OBJ are referenced only so that the function can select the correct OBJMATERIAL as its operand.
[Note: This is where changing the type of parent to "const OBJ *" comes into play as mentioned above. By making parent a pointer to a constant OBJ the code emphasizes that this method is really a member function of the OBJMATERIAL class because the method can't change the contents of the OBJ via the parent pointer. This also applies below in the discussion of making OBJ_build_material() into an OBJMATERIAL member function.]
[Note: This is where changing the type of parent to "const OBJ *" comes into play as mentioned above. By making parent a pointer to a constant OBJ the code emphasizes that this method is really a member function of the OBJMATERIAL class because the method can't change the contents of the OBJ via the parent pointer. This also applies below in the discussion of making OBJ_build_material() into an OBJMATERIAL member function.]
In the sample code calls to the old OBJ_set_draw_callback_material() which look like:
OBJ_set_draw_callback_material( obj, i, material_draw );
are changed to look like:
obj->objmaterial[i].set_draw_callback_material(material_draw);
The argument that OBJ_build_material() is really a class method of the OBJMATERIAL class continues this line of thought. Data members of OBJ are referenced so the function knows how to change the instance of OBJMATERIAL (aka this once the code has been changed to a class method) but none of the OBJ data members are changed; OBJ data members are only read, not modified. Note that in the original implementation the only OBJMATERIAL objects which could be changed by the function were children of the OBJ instance passed into the function. The version of the function rewritten as a class method of the OBJMATERIAL class needs to access the OBJ instance which is this (aka this) OBJMATERIAL's parent, hence the need for the new data member parent.
Also, note that the old OBJ_build_material() function uses the old OBJ_get_texture_index(), OBJ_add_texture(), OBJ_get_program_index(), and OBJ_add_program() functions. The new class method OBJMATERIAL::build_material() still needs access to the OBJ member functions. As mentioned above, I have rewritten these functions to be private OBJ methods since they are never referenced outside of this file. I still think the new version of the OBJ methods should be private, but, to allow OBJMATERIAL access to these private functions the OBJ class needs to declare that OBJMATERIAL is a friend class. This really shouldn't be considered out of the ordinary since the OBJMATERIAL object (aka this) is accessing the data members of the OBJ which is its parent.
Before continuing to other orphan functions I want to talk about the names of the class methods I've just discussed putting in the OBJMATERIAL class. Again, their names are:
- draw_material(),
- build_material(), and
- set_draw_callback_material()
obj->objmaterial[i].build_material(NULL);
Note that "material" shows up twice in this line of code, once in "objmaterial", and again in "build_material". When the old code named the function OBJ_build_material(), but no OBJMATERIAL was to be seen in the argument list it made sense for the code to call out that what the function really did was operate on an OBJMATERIAL (data) object. Now that the new code is used as a class method to explicitly operate on an instance of OBJMATERIAL I think having "_material" in the name of the method is redundant so I have removed it and the names are now:
- draw(),
- build(), and
- set_draw_callback()
The OBJMESH data class acquires most of the remaining functions which begin with the "OBJ_" prefix. The methods added to the OBJMESH class are:
- update_bounds() (I made this name plural),
- build_vbo(),
- set_attributes(),
- build(),
- build2(),
- optimize(),
- draw(),
- draw2(),
- draw3(), and
- free_vertex_data()
Like the methods moved into the OBJMATERIAL class I've stripped "_mesh" from the names of the methods to avoid redundancy.
This should leave only two of the original functions unaccounted for:
- OBJ_build_texture(), and
- OBJ_build_program()
As you might have guessed OBJ_build_program() becomes PROGRAM::build(). This was hinted at in a previous post.
If you guessed that OBJ_build_texture() will become TEXTURE::build() you were again correct but since I haven't yet discussed creating the TEXTURE data class I've postponed that change.
As mentioned above the OBJ class uses the std::vector template to create containers to hold OBJMATERIAL, OBJMESH, "PROGRAM *", and "TEXTURE *" data. Each of of these data types start off with a name field. In a previous post, I removed the name field from the UNIFORM data type and changed the PROGRAM object class to hold its UNIFORM data in a container created using the std::map template. I don't think the elements of the vectors objmaterial, objmesh, program, or texture have an inherent order. There are OBJ methods which assume an inherent order (OBJ::get_texture_index() and OBJ::get_program_index()) but the code could just as easily retrieve items from these containers using a name (like OBJ::get_mesh(), OBJ::get_program(), OBJ::get_material() and OBJ::get_texture()). The question then becomes "Why didn't I declare objmaterial, objmesh, program, and texture using the std::map template?" (Of course, if I had done this I would have had to modify any place in the code which uses OBJ::get_texture_index() or OBJ::get_program_index() but I think that's easily done.) The PROGRAM class type dereferences its name field in several of its class methods so we can't easily remove the name data member from the class definition. If we store the collection of PROGRAM pointers using std::map we have to store the name information twice, once as data member of the class, again as a key into the std::map to retrieve the PROGRAM pointer. There's a second problem: the name field of the PROGRAM class is passed into the utility function get_file_name() and this function can modify the value stored in name. If we use the PROGRAM name as the key with std::map the key must be treated as a read-only entry so name must be read-only, but clearly, because get_file_name() changes the contents of name, name is not read-only. The TEXTURE data type (to become the TEXTURE class in a future post) suffers from the same problems. It looks like the name fields for OBJMESH and OBJMATERIAL are treated as read-only and are never dereferenced in any of their respective member functions so we can probably store the OBJMESH and OBJMATERIAL instances in their OBJ parent using std::map. I started to re-implement objmaterial as
std::map<std::string,OBJMATERIAL> objmaterial;
but ran into problems when a function would return "OBJMATERIAL *" by doing a look-up on objmaterial. For some reason, the object pointed to by the "OBJMATERIAL *" return value would get corrupted; I suspect that the problem has to do with the inner workings of std::map but haven't had a chance to verify my hypothesis. I was able to make the code work correctly if I redefined objmaterial as:
std::map<std::string,OBJMATERIAL *> objmaterial;
I have briefly tried to implement this same strategy for objmesh and ran into a problem. In one of the modules I haven't yet spoken about there is a function which passes in a pointer to an OBJ ("OBJ *") and an integer which is used by the function to index into the objmesh array of the OBJ. It may be possible to rewrite the interface to that function so that the address of the objmesh needed is passed in directly but that task will have to wait until I get to that particular module.
In the interim, I'm not committing the change to objmaterial to be an instance of std::map into the archive at GitHub until I've had more time to make sure that the change is stable.
The method OBJMESH::update_bounds() calculates:
- the Axially Aligned Bounding Box of the mesh (AABB),
- the midpoint of the AABB, and
- (the comment says that the method also calculates) the radius of the bounding sphere of the mesh.
The author has commented out what I would expect to be the correct method of computing the radius of the bounding sphere of the mesh from the dimensions of the AABB but instead
the author calculated the radius of the bounding sphere as one half of the AABB's maximum dimension. I think the description of what is being calculated is wrong but the author doesn't explain why he calculated the radius this way or how the quantity is really used so all I can say about the computation is that I don't think it calculates what the comment says it calculates. Using this method to calculate the bounding sphere radius it's entirely possible to have two meshes with their midpoints separated by a distance that exceeds the sum of the lengths of their respective bounding sphere radii yet have the two objects intersecting each other. This could lead to situations where a collision has happened on the display but the code hasn't yet detected the collision. You need to determine for your own applications which computation better describes what you need, or, possibly, you need to compute the bounding sphere radius using some other means.
the author calculated the radius of the bounding sphere as one half of the AABB's maximum dimension. I think the description of what is being calculated is wrong but the author doesn't explain why he calculated the radius this way or how the quantity is really used so all I can say about the computation is that I don't think it calculates what the comment says it calculates. Using this method to calculate the bounding sphere radius it's entirely possible to have two meshes with their midpoints separated by a distance that exceeds the sum of the lengths of their respective bounding sphere radii yet have the two objects intersecting each other. This could lead to situations where a collision has happened on the display but the code hasn't yet detected the collision. You need to determine for your own applications which computation better describes what you need, or, possibly, you need to compute the bounding sphere radius using some other means.
One other comment regarding the OBJMESH::update_bounds() method. I changed the initial values for the vec3 data members named min and max of the OBJMESH pointed to by this. The new values which I've used have also been used to initialize min and max in another module which will be covered in a future post. Feel free to search for the changes in the other module if you want to look ahead. Anyway, I think the values I've assigned to min, and max are less likely to get tripped up by pathological data cases. One never knows what kinds of models the code might import.
Another bug lurking in the original version of the code is a bit more subtle but I'm still surprised that the compiler didn't print an error message. In the OBJ class the data member offset is declared to be an array of unsigned int. This actually leads to two bugs. The first is that offset[3] is tested to see if it holds the value -1, if it does then that means that the (graphics) object doesn't have texture coordinates or bi-normal/-tangent information which needs to be passed to the programmable shaders. Since the entries in offset are unsigned int they can only hold values that are non-negative, i.e., it's never possible for offset[3] to contain the value -1. The second bug is that in the original code space was allocated for the OBJ data object using calloc(3). One of the things calloc() does is zero out the all of the memory space it returns to the caller; after the OBJ data object is allocated the only other time offset[3] is set is inside OBJMESH::build_vbo() and then the only value it is ever given (besides its initial value of zero) is the offset of the texture coordinate vertex attribute. That is, even if offset[3] could hold the value -1 the code never even tries to set offset[3] to use the flag value -1. These two problems each require a solution. I solve the problem of offset[3] not being set to a flag value by initializing the entire offset array to have all of its bits set to one (in for a penny, in for a pound!); as written today we don't need a flag value in any of the other items in the offset array but while we're making the change we might as well future-proof the code. The second fix is that offset[3] is no longer tested against -1; it is now tested against ~0. And I need to add a symbol to the header file so that ~0 has a symbolic name. Remember, I hate "magic numbers" and ~0 is a magic number.
Speaking of hating magic numbers I've added several other things to the header file as well. In particular, I've added three enum types (TextureMap, VertexAttribute, and AttributeOffset), and #define statements which give a collection of string constants symbolic names. I'll discuss AttributeOffset now but postpone talking about the others until I cover the sample code from chapter 4.
Above I referred repeatedly to the OBJ data member offset[3]. The code doesn't have any comments to tell the reader what the significance of the offset[3] is. I had to reverse engineer that the offset[3] holds the OpenGL offset for the vertex attribute which the shader code uses to hold texture coordinates. I worked it out once and I can work it out again but by replacing 3 in the expression offset[3] with a symbolic name I don't have to remember what offset[3] represents; the symbolic name will tell me. I used the symbolic name AO_TexCoord0 (AO stands for "[OpenGL Vertex] Attribute Offset"). Likewise, I reverse engineered what the other entries in the offset array represent and added symbolic names for them too. I encapsulated the various symbolic names as values of the enum data type AttributeOffset. Lastly, I modified the various uses of offset[int] to use the new symbolic names for the various indices in place of the integers 0, 1, 2, 3, and 4.
Giving the array indices symbolic names isn't the only way to solve this problem. I also considered changing offset from being an array of unsigned int to be a struct with the data fields vertex, normal, fnormal, texcoord0, and tangent0. In some ways I think this is a more natural expression of the data which offset holds. Normally, I think of using an array (or in C++ a type created from the std::vector template) as a collection of data items which I need to iterate over sequentially. The code doesn't iterate over the elements of the array (to me, iterating implies a loop statement) but the elements of the array do form the members of a set and belong together logically, and, IMHO, using a struct expresses their relationship to each other better than using an array. But, again, it's just an opinion, and, in the end, I did leave the data field as an array.
That about wraps up this post. Next time I'll cover the changes to the first three sample programs in chapter 3.
Speaking of hating magic numbers I've added several other things to the header file as well. In particular, I've added three enum types (TextureMap, VertexAttribute, and AttributeOffset), and #define statements which give a collection of string constants symbolic names. I'll discuss AttributeOffset now but postpone talking about the others until I cover the sample code from chapter 4.
Above I referred repeatedly to the OBJ data member offset[3]. The code doesn't have any comments to tell the reader what the significance of the offset[3] is. I had to reverse engineer that the offset[3] holds the OpenGL offset for the vertex attribute which the shader code uses to hold texture coordinates. I worked it out once and I can work it out again but by replacing 3 in the expression offset[3] with a symbolic name I don't have to remember what offset[3] represents; the symbolic name will tell me. I used the symbolic name AO_TexCoord0 (AO stands for "[OpenGL Vertex] Attribute Offset"). Likewise, I reverse engineered what the other entries in the offset array represent and added symbolic names for them too. I encapsulated the various symbolic names as values of the enum data type AttributeOffset. Lastly, I modified the various uses of offset[int] to use the new symbolic names for the various indices in place of the integers 0, 1, 2, 3, and 4.
Giving the array indices symbolic names isn't the only way to solve this problem. I also considered changing offset from being an array of unsigned int to be a struct with the data fields vertex, normal, fnormal, texcoord0, and tangent0. In some ways I think this is a more natural expression of the data which offset holds. Normally, I think of using an array (or in C++ a type created from the std::vector template) as a collection of data items which I need to iterate over sequentially. The code doesn't iterate over the elements of the array (to me, iterating implies a loop statement) but the elements of the array do form the members of a set and belong together logically, and, IMHO, using a struct expresses their relationship to each other better than using an array. But, again, it's just an opinion, and, in the end, I did leave the data field as an array.
That about wraps up this post. Next time I'll cover the changes to the first three sample programs in chapter 3.
No comments:
Post a Comment