Merge branch 'master' into hl1-mdl-fix-texture-format

code/Assbin/AssbinExporter.cpp

@@ -413,7 +413,8 @@ protected:
 
         Write<unsigned int>(&chunk,tex->mWidth);
         Write<unsigned int>(&chunk,tex->mHeight);
-        chunk.Write( tex->achFormatHint, sizeof(char), 4 );
+        // Write the texture format, but don't include the null terminator.
+        chunk.Write( tex->achFormatHint, sizeof(char), HINTMAXTEXTURELEN - 1 );
 
         if(!shortened) {
             if (!tex->mHeight) {

code/Assbin/AssbinLoader.cpp

@@ -535,7 +535,7 @@ void AssbinImporter::ReadBinaryTexture(IOStream * stream, aiTexture* tex) {
 
     tex->mWidth = Read<unsigned int>(stream);
     tex->mHeight = Read<unsigned int>(stream);
-    stream->Read( tex->achFormatHint, sizeof(char), 4 );
+    stream->Read( tex->achFormatHint, sizeof(char), HINTMAXTEXTURELEN - 1 );
 
     if(!shortened) {
         if (!tex->mHeight) {

code/MDL/HalfLife/HL1MDLLoader.cpp

@@ -124,11 +124,23 @@ void HL1MDLLoader::release_resources() {
         delete[] anim_headers_;
         anim_headers_ = nullptr;
     }
+
+    // Root has some children nodes. so let's proceed them
+    if (!rootnode_children_.empty()) {
+        // Here, it means that the nodes were not added to the
+        // scene root node. We still have to delete them.
+        for (auto it = rootnode_children_.begin(); it != rootnode_children_.end(); ++it) {
+            if (*it) {
+                delete *it;
+            }
+        }
+        // Ensure this happens only once.
+        rootnode_children_.clear();
+    }
 }
 
 // ------------------------------------------------------------------------------------------------
 void HL1MDLLoader::load_file() {
-
     try {
         header_ = (const Header_HL1 *)buffer_;
         validate_header(header_, false);
@@ -138,8 +150,9 @@ void HL1MDLLoader::load_file() {
 
         load_texture_file();
 
-        if (import_settings_.read_animations)
+        if (import_settings_.read_animations) {
             load_sequence_groups_files();
+        }
 
         read_textures();
         read_skins();
@@ -155,14 +168,17 @@ void HL1MDLLoader::load_file() {
                 read_sequence_transitions();
         }
 
-        if (import_settings_.read_attachments)
+        if (import_settings_.read_attachments) {
             read_attachments();
+        }
 
-        if (import_settings_.read_hitboxes)
+        if (import_settings_.read_hitboxes) {
             read_hitboxes();
+        }
 
-        if (import_settings_.read_bone_controllers)
+        if (import_settings_.read_bone_controllers) {
             read_bone_controllers();
+        }
 
         read_global_info();
 
@@ -171,6 +187,10 @@ void HL1MDLLoader::load_file() {
             scene_->mRootNode->addChildren(
                     static_cast<unsigned int>(rootnode_children_.size()),
                     rootnode_children_.data());
+
+            // Clear the list of nodes so they will not be destroyed
+            // when resources are released.
+            rootnode_children_.clear();
         }
 
         release_resources();
@@ -185,48 +205,60 @@ void HL1MDLLoader::load_file() {
 void HL1MDLLoader::validate_header(const Header_HL1 *header, bool is_texture_header) {
     if (is_texture_header) {
         // Every single Half-Life model is assumed to have at least one texture.
-        if (!header->numtextures)
+        if (!header->numtextures) {
             throw DeadlyImportError(MDL_HALFLIFE_LOG_HEADER "There are no textures in the file");
+        }
 
-        if (header->numtextures > AI_MDL_HL1_MAX_TEXTURES)
+        if (header->numtextures > AI_MDL_HL1_MAX_TEXTURES) {
             log_warning_limit_exceeded<AI_MDL_HL1_MAX_TEXTURES>(header->numtextures, "textures");
+        }
 
-        if (header->numskinfamilies > AI_MDL_HL1_MAX_SKIN_FAMILIES)
+        if (header->numskinfamilies > AI_MDL_HL1_MAX_SKIN_FAMILIES) {
             log_warning_limit_exceeded<AI_MDL_HL1_MAX_SKIN_FAMILIES>(header->numskinfamilies, "skin families");
+        }
 
     } else {
         // Every single Half-Life model is assumed to have at least one bodypart.
-        if (!header->numbodyparts)
+        if (!header->numbodyparts) {
             throw DeadlyImportError(MDL_HALFLIFE_LOG_HEADER "Model has no bodyparts");
+        }
 
         // Every single Half-Life model is assumed to have at least one bone.
-        if (!header->numbones)
+        if (!header->numbones) {
             throw DeadlyImportError(MDL_HALFLIFE_LOG_HEADER "Model has no bones");
+        }
 
         // Every single Half-Life model is assumed to have at least one sequence group,
         // which is the "default" sequence group.
-        if (!header->numseqgroups)
+        if (!header->numseqgroups) {
             throw DeadlyImportError(MDL_HALFLIFE_LOG_HEADER "Model has no sequence groups");
+        }
 
-        if (header->numbodyparts > AI_MDL_HL1_MAX_BODYPARTS)
+        if (header->numbodyparts > AI_MDL_HL1_MAX_BODYPARTS) {
             log_warning_limit_exceeded<AI_MDL_HL1_MAX_BODYPARTS>(header->numbodyparts, "bodyparts");
+        }
 
-        if (header->numbones > AI_MDL_HL1_MAX_BONES)
+        if (header->numbones > AI_MDL_HL1_MAX_BONES) {
             log_warning_limit_exceeded<AI_MDL_HL1_MAX_BONES>(header->numbones, "bones");
+        }
 
-        if (header->numbonecontrollers > AI_MDL_HL1_MAX_BONE_CONTROLLERS)
+        if (header->numbonecontrollers > AI_MDL_HL1_MAX_BONE_CONTROLLERS) {
             log_warning_limit_exceeded<AI_MDL_HL1_MAX_BONE_CONTROLLERS>(header->numbonecontrollers, "bone controllers");
+        }
 
-        if (header->numseq > AI_MDL_HL1_MAX_SEQUENCES)
+        if (header->numseq > AI_MDL_HL1_MAX_SEQUENCES) {
             log_warning_limit_exceeded<AI_MDL_HL1_MAX_SEQUENCES>(header->numseq, "sequences");
+        }
 
-        if (header->numseqgroups > AI_MDL_HL1_MAX_SEQUENCE_GROUPS)
+        if (header->numseqgroups > AI_MDL_HL1_MAX_SEQUENCE_GROUPS) {
             log_warning_limit_exceeded<AI_MDL_HL1_MAX_SEQUENCE_GROUPS>(header->numseqgroups, "sequence groups");
+        }
 
-        if (header->numattachments > AI_MDL_HL1_MAX_ATTACHMENTS)
+        if (header->numattachments > AI_MDL_HL1_MAX_ATTACHMENTS) {
             log_warning_limit_exceeded<AI_MDL_HL1_MAX_ATTACHMENTS>(header->numattachments, "attachments");
         }
     }
+}
 
 // ------------------------------------------------------------------------------------------------
 /*
@@ -256,8 +288,7 @@ void HL1MDLLoader::load_texture_file() {
 
         load_file_into_buffer<Header_HL1>(texture_file_path, texture_buffer_);
     } else {
-        /* Model has no external texture file. This means the texture
+        // Model has no external texture file. This means the texture is stored inside the main MDL file.
-        is stored inside the main MDL file. */
         texture_buffer_ = const_cast<unsigned char *>(buffer_);
     }
 
@@ -284,16 +315,17 @@ void HL1MDLLoader::load_texture_file() {
 
 */
 void HL1MDLLoader::load_sequence_groups_files() {
-    if (header_->numseqgroups <= 1)
+    if (header_->numseqgroups <= 1) {
         return;
+    }
 
     num_sequence_groups_ = header_->numseqgroups;
 
     anim_buffers_ = new unsigned char *[num_sequence_groups_];
     anim_headers_ = new SequenceHeader_HL1 *[num_sequence_groups_];
     for (int i = 0; i < num_sequence_groups_; ++i) {
-        anim_buffers_[i] = NULL;
+        anim_buffers_[i] = nullptr;
-        anim_headers_[i] = NULL;
+        anim_headers_[i] = nullptr;
     }
 
     std::string file_path_without_extension =
@@ -327,21 +359,24 @@ void HL1MDLLoader::read_texture(const Texture_HL1 *ptexture,
         aiColor3D &last_palette_color) {
     int outwidth, outheight;
     int i, j;
-    int row1[256], row2[256], col1[256], col2[256];
+    static const size_t BuffenLen = 256;
+    int row1[BuffenLen], row2[BuffenLen], col1[BuffenLen], col2[BuffenLen];
     unsigned char *pix1, *pix2, *pix3, *pix4;
 
     // convert texture to power of 2
     for (outwidth = 1; outwidth < ptexture->width; outwidth <<= 1)
         ;
 
-    if (outwidth > 256)
+    if ( outwidth > static_cast<int>(BuffenLen)) {
-        outwidth = 256;
+        outwidth = BuffenLen;
+    }
 
     for (outheight = 1; outheight < ptexture->height; outheight <<= 1)
         ;
 
-    if (outheight > 256)
+    if (static_cast<size_t>(outheight) > BuffenLen) {
-        outheight = 256;
+        outheight = BuffenLen;
+    }
 
     pResult->mFilename = ptexture->name;
     pResult->mWidth = outwidth;
@@ -440,8 +475,9 @@ void HL1MDLLoader::read_textures() {
 // ------------------------------------------------------------------------------------------------
 void HL1MDLLoader::read_skins() {
     // Read skins, if any.
-    if (texture_header_->numskinfamilies <= 1)
+    if (texture_header_->numskinfamilies <= 1) {
         return;
+    }
 
     // Pointer to base texture index.
     short *default_skin_ptr = (short *)((uint8_t *)texture_header_ + texture_header_->skinindex);
@@ -465,8 +501,9 @@ void HL1MDLLoader::read_bones() {
     const Bone_HL1 *pbone = (const Bone_HL1 *)((uint8_t *)header_ + header_->boneindex);
 
     std::vector<std::string> unique_bones_names(header_->numbones);
-    for (int i = 0; i < header_->numbones; ++i)
+    for (int i = 0; i < header_->numbones; ++i) {
         unique_bones_names[i] = pbone[i].name;
+    }
 
     // Ensure bones have unique names.
     unique_name_generator_.set_template_name("Bone");
@@ -583,14 +620,17 @@ void HL1MDLLoader::read_meshes() {
     }
 
     // Display limit infos.
-    if (total_verts > AI_MDL_HL1_MAX_VERTICES)
+    if (total_verts > AI_MDL_HL1_MAX_VERTICES) {
         log_warning_limit_exceeded<AI_MDL_HL1_MAX_VERTICES>(total_verts, "vertices");
+    }
 
-    if (scene_->mNumMeshes > AI_MDL_HL1_MAX_MESHES)
+    if (scene_->mNumMeshes > AI_MDL_HL1_MAX_MESHES) {
         log_warning_limit_exceeded<AI_MDL_HL1_MAX_MESHES>(scene_->mNumMeshes, "meshes");
+    }
 
-    if (total_models_ > AI_MDL_HL1_MAX_MODELS)
+    if (total_models_ > AI_MDL_HL1_MAX_MODELS) {
         log_warning_limit_exceeded<AI_MDL_HL1_MAX_MODELS>(total_models_, "models");
+    }
 
     // Ensure bodyparts have unique names.
     unique_name_generator_.set_template_name("Bodypart");
@@ -917,9 +957,10 @@ void HL1MDLLoader::read_meshes() {
         }
     }
 
-    if (total_triangles > AI_MDL_HL1_MAX_TRIANGLES)
+    if (total_triangles > AI_MDL_HL1_MAX_TRIANGLES) {
         log_warning_limit_exceeded<AI_MDL_HL1_MAX_TRIANGLES>(total_triangles, "triangles");
     }
+}
 
 // ------------------------------------------------------------------------------------------------
 void HL1MDLLoader::read_animations() {
@@ -959,10 +1000,11 @@ void HL1MDLLoader::read_animations() {
     for (int sequence = 0; sequence < header_->numseq; ++sequence, ++pseqdesc) {
         pseqgroup = (const SequenceGroup_HL1 *)((uint8_t *)header_ + header_->seqgroupindex) + pseqdesc->seqgroup;
 
-        if (pseqdesc->seqgroup == 0)
+        if (pseqdesc->seqgroup == 0) {
             panim = (const AnimValueOffset_HL1 *)((uint8_t *)header_ + pseqgroup->unused2 + pseqdesc->animindex);
-        else
+        } else {
             panim = (const AnimValueOffset_HL1 *)((uint8_t *)anim_headers_[pseqdesc->seqgroup] + pseqdesc->animindex);
+        }
 
         for (int blend = 0; blend < pseqdesc->numblends; ++blend, ++scene_animations_ptr) {
 
@@ -1035,8 +1077,9 @@ void HL1MDLLoader::read_sequence_groups_info() {
     const SequenceGroup_HL1 *pseqgroup = (const SequenceGroup_HL1 *)((uint8_t *)header_ + header_->seqgroupindex);
 
     unique_sequence_groups_names_.resize(header_->numseqgroups);
-    for (int i = 0; i < header_->numseqgroups; ++i)
+    for (int i = 0; i < header_->numseqgroups; ++i) {
         unique_sequence_groups_names_[i] = pseqgroup[i].label;
+    }
 
     // Ensure sequence groups have unique names.
     unique_name_generator_.set_template_name("SequenceGroup");
@@ -1059,8 +1102,9 @@ void HL1MDLLoader::read_sequence_groups_info() {
 
 // ------------------------------------------------------------------------------------------------
 void HL1MDLLoader::read_sequence_infos() {
-    if (!header_->numseq)
+    if (!header_->numseq) {
         return;
+    }
 
     const SequenceDesc_HL1 *pseqdesc = (const SequenceDesc_HL1 *)((uint8_t *)header_ + header_->seqindex);
 
@@ -1163,8 +1207,9 @@ void HL1MDLLoader::read_sequence_infos() {
 
 // ------------------------------------------------------------------------------------------------
 void HL1MDLLoader::read_sequence_transitions() {
-    if (!header_->numtransitions)
+    if (!header_->numtransitions) {
         return;
+    }
 
     // Read sequence transition graph.
     aiNode *transition_graph_node = new aiNode(AI_MDL_HL1_NODE_SEQUENCE_TRANSITION_GRAPH);
@@ -1177,8 +1222,9 @@ void HL1MDLLoader::read_sequence_transitions() {
 }
 
 void HL1MDLLoader::read_attachments() {
-    if (!header_->numattachments)
+    if (!header_->numattachments) {
         return;
+    }
 
     const Attachment_HL1 *pattach = (const Attachment_HL1 *)((uint8_t *)header_ + header_->attachmentindex);
 
@@ -1200,8 +1246,9 @@ void HL1MDLLoader::read_attachments() {
 
 // ------------------------------------------------------------------------------------------------
 void HL1MDLLoader::read_hitboxes() {
-    if (!header_->numhitboxes)
+    if (!header_->numhitboxes) {
         return;
+    }
 
     const Hitbox_HL1 *phitbox = (const Hitbox_HL1 *)((uint8_t *)header_ + header_->hitboxindex);
 
@@ -1226,8 +1273,9 @@ void HL1MDLLoader::read_hitboxes() {
 
 // ------------------------------------------------------------------------------------------------
 void HL1MDLLoader::read_bone_controllers() {
-    if (!header_->numbonecontrollers)
+    if (!header_->numbonecontrollers) {
         return;
+    }
 
     const BoneController_HL1 *pbonecontroller = (const BoneController_HL1 *)((uint8_t *)header_ + header_->bonecontrollerindex);
 
@@ -1306,11 +1354,12 @@ void HL1MDLLoader::extract_anim_value(
     }
 
     // Bah, missing blend!
-    if (panimvalue->num.valid > k)
+    if (panimvalue->num.valid > k) {
         value = panimvalue[k + 1].value * bone_scale;
-    else
+    } else {
         value = panimvalue[panimvalue->num.valid].value * bone_scale;
     }
+}
 
 // ------------------------------------------------------------------------------------------------
 // Get the number of blend controllers.

code/PostProcessing/ValidateDataStructure.cpp

@@ -798,7 +798,7 @@ void ValidateDSProcess::Validate( const aiTexture* pTexture)
         if (!pTexture->mWidth) {
             ReportError("aiTexture::mWidth is zero (compressed texture)");
         }
-        if ('\0' != pTexture->achFormatHint[3]) {
+        if ('\0' != pTexture->achFormatHint[HINTMAXTEXTURELEN - 1]) {
             ReportWarning("aiTexture::achFormatHint must be zero-terminated");
         }
         else if ('.'  == pTexture->achFormatHint[0])    {

include/assimp/texture.h

@@ -207,8 +207,7 @@ struct aiTexture {
     , mHeight(0)
     , pcData(nullptr)
     , mFilename() {
-        achFormatHint[0] = achFormatHint[1] = 0;
+        memset(achFormatHint, 0, sizeof(achFormatHint));
-        achFormatHint[2] = achFormatHint[3] = 0;
     }
 
     // Destruction

tools/assimp_cmd/CompareDump.cpp

@@ -800,6 +800,10 @@ void CompareOnTheFlyTexture(comparer_context& comp) {
     comp.cmp<char>("achFormatHint[1]");
     comp.cmp<char>("achFormatHint[2]");
     comp.cmp<char>("achFormatHint[3]");
+    comp.cmp<char>("achFormatHint[4]");
+    comp.cmp<char>("achFormatHint[5]");
+    comp.cmp<char>("achFormatHint[6]");
+    comp.cmp<char>("achFormatHint[7]");
 }
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////

tools/assimp_cmd/WriteDumb.cpp

@@ -312,7 +312,8 @@ uint32_t WriteBinaryTexture(const aiTexture* tex)
 
 	len += Write<unsigned int>(tex->mWidth);
 	len += Write<unsigned int>(tex->mHeight);
-	len += static_cast<uint32_t>(fwrite(tex->achFormatHint,1,4,out));
+	// Write the texture format, but don't include the null terminator.
+	len += static_cast<uint32_t>(fwrite(tex->achFormatHint,sizeof(char),HINTMAXTEXTURELEN - 1,out));
 
 	if(!shortened) {
 		if (!tex->mHeight) {