Fix memory corruptions in w3dview (#1129)

TheAssemblyArmada · Jul 16, 2024 · d2cb09d · d2cb09d
1 parent 9a10575
commit d2cb09d
Show file tree

Hide file tree

Showing 9 changed files with 72 additions and 54 deletions.
diff --git a/src/game/client/w3dassetmanager.cpp b/src/game/client/w3dassetmanager.cpp
@@ -401,6 +401,7 @@ TextureClass *GameAssetManager::Recolor_Texture_One_Time(TextureClass *texture,
     char buffer[512];
     Create_Color_Texture_Name(buffer, name, color);
     new_texture->Set_Texture_Name(buffer);
+    captainslog_dbgassert(!m_textureHash.Exists(new_texture), "Texture hash collision occurred"); // Thyme specific
     m_textureHash.Insert(new_texture->Get_Name(), new_texture);
     new_texture->Add_Ref();
     Ref_Ptr_Release(surface);

diff --git a/src/tools/w3dviewer/assetinfo.cpp b/src/tools/w3dviewer/assetinfo.cpp
@@ -18,6 +18,7 @@
 AssetInfoClass::AssetInfoClass(const char *name, AssetType type, RenderObjClass *robj, TextureClass *texture) :
     m_name(name), m_type(type), m_texture(texture), m_renderObj(nullptr)
 {
+    // Note: The reference for m_texture is added and removed externally (bad).
     Ref_Ptr_Set(m_renderObj, robj);
     GetHierarchyName();
 }

diff --git a/src/tools/w3dviewer/datatreeview.cpp b/src/tools/w3dviewer/datatreeview.cpp
@@ -458,6 +458,7 @@ void CDataTreeView::AddTextures()
                 m_categoryTreeItems[CATEGORY_MATERIAL],
                 TVI_SORT);
 
+            texture->Add_Ref();
             AssetInfoClass *info = new AssetInfoClass(texture->Get_Name(), ASSET_TYPE_TEXTURE, nullptr, texture);
             GetTreeCtrl().SetItem(newitem, TVIF_PARAM, nullptr, 0, 0, 0, 0, (LPARAM)info);
         }

diff --git a/src/tools/w3dviewer/w3dviewdoc.cpp b/src/tools/w3dviewer/w3dviewdoc.cpp
@@ -467,7 +467,9 @@ void CW3DViewDoc::SetRenderObject(RenderObjClass *robj, bool useRegularCameraRes
     }
 
     robj->Set_Animation();
+    captainslog_dbgassert(m_model == nullptr, "Expected nullptr, otherwise leaks"); // Thyme specific
     m_model = robj;
+    m_model->Add_Ref();
     Matrix3D tm(true);
     m_model->Set_Transform(tm);
     SceneClass *scene;

diff --git a/src/w3d/lib/multilist.h b/src/w3d/lib/multilist.h
@@ -17,6 +17,7 @@
 
 #include "always.h"
 #include "autopool.h"
+#include "refcount.h"
 #include <captainslog.h>
 
 // #TODO investigate C casts in this file.

diff --git a/src/w3d/renderer/assetmgr.cpp b/src/w3d/renderer/assetmgr.cpp
@@ -357,44 +357,43 @@ TextureClass *W3DAssetManager::Get_Texture(const char *filename,
         mip_level_count = MipCountType::MIP_LEVELS_1;
     }
 
-    if (filename == nullptr || strlen(filename) == 0) {
-        return nullptr;
-    }
-
-    StringClass name = { filename };
-    name.To_Lower();
-    TextureClass *texture = m_textureHash.Get(name);
-
-    if (texture != nullptr) {
-        texture->Add_Ref();
-        return texture;
-    }
+    if (filename != nullptr && strlen(filename) != 0) {
+        StringClass name = { filename };
+        name.To_Lower();
+
+        TextureClass *new_texture = m_textureHash.Get(name);
+
+        if (new_texture == nullptr) {
+
+            switch (asset_type) {
+                case TexAssetType::ASSET_STANDARD:
+                    new_texture = new TextureClass{
+                        name, nullptr, mip_level_count, texture_format, allow_compression, allow_reduction
+                    };
+                    break;
+                case TexAssetType::ASSET_CUBE:
+                    captainslog_dbgassert(false, "CubeTextureClass is not used");
+                    break;
+                case TexAssetType::ASSET_VOLUME:
+                    captainslog_dbgassert(false, "VolumeTextureClass is not used");
+                    break;
+                default:
+                    break;
+            }
 
-    TextureClass *new_texture = nullptr;
+            if (new_texture == nullptr) {
+                return nullptr;
+            }
 
-    switch (asset_type) {
-        case TexAssetType::ASSET_STANDARD:
-            new_texture =
-                new TextureClass{ name, nullptr, mip_level_count, texture_format, allow_compression, allow_reduction };
-            break;
-        case TexAssetType::ASSET_CUBE:
-            captainslog_dbgassert(false, "CubeTextureClass is not used");
-            break;
-        case TexAssetType::ASSET_VOLUME:
-            captainslog_dbgassert(false, "VolumeTextureClass is not used");
-            break;
-        default:
-            break;
-    }
+            captainslog_dbgassert(!m_textureHash.Exists(new_texture), "Texture hash collision occurred"); // Thyme specific
+            m_textureHash.Insert(new_texture->Get_Name(), new_texture);
+        }
 
-    if (new_texture == nullptr) {
-        return nullptr;
+        new_texture->Add_Ref();
+        return new_texture;
     }
 
-    m_textureHash.Insert(new_texture->Get_Name(), new_texture);
-
-    new_texture->Add_Ref();
-    return new_texture;
+    return nullptr;
 }
 
 // 0x00815C90

diff --git a/src/w3d/renderer/dx8renderer.cpp b/src/w3d/renderer/dx8renderer.cpp
@@ -472,6 +472,7 @@ DX8TextureCategoryClass::DX8TextureCategoryClass(
     captainslog_assert(pass < DX8FVFCategoryContainer::MAX_PASSES);
 
     for (int i = 0; i < 2; i++) {
+        m_textures[i] = nullptr;
         Ref_Ptr_Set(m_textures[i], texs[i]);
     }
 

diff --git a/src/w3d/renderer/mesh.h b/src/w3d/renderer/mesh.h
@@ -141,7 +141,11 @@ class PrimitivePrototypeClass : public W3DMPO, public PrototypeClass
         proto->Add_Ref();
     }
 
-    virtual ~PrimitivePrototypeClass() override { m_proto->Release_Ref(); }
+    virtual ~PrimitivePrototypeClass() override
+    {
+        if (m_proto != nullptr)
+            m_proto->Release_Ref();
+    }
     virtual const char *Get_Name() const override { return m_proto->Get_Name(); }
     virtual int Get_Class_ID() const override { return m_proto->Class_ID(); }
     virtual RenderObjClass *Create() override { return m_proto->Clone(); }

diff --git a/src/w3d/renderer/meshmdl.cpp b/src/w3d/renderer/meshmdl.cpp
@@ -603,14 +603,14 @@ TextureClass *Load_Texture(ChunkLoadClass &cload)
 
     while (cload.Open_Chunk()) {
         int id = cload.Cur_Chunk_ID();
-        if (id == W3D_CHUNK_TEXTURE_NAME) {
-            cload.Read(name, cload.Cur_Chunk_Length());
-        } else if (id == W3D_CHUNK_TEXTURE_INFO) {
-            if (id == 1) {
-                cload.Read(&info, 12);
-            }
-
-            texinfo = true;
+        switch (id) {
+            case W3D_CHUNK_TEXTURE_NAME:
+                cload.Read(name, cload.Cur_Chunk_Length());
+                break;
+            case W3D_CHUNK_TEXTURE_INFO:
+                cload.Read(&info, sizeof(info));
+                texinfo = true;
+                break;
         }
 
         cload.Close_Chunk();
@@ -638,6 +638,7 @@ TextureClass *Load_Texture(ChunkLoadClass &cload)
             case W3DTEXTURE_MIP_LEVELS_4:
                 mips = MipCountType::MIP_LEVELS_4;
                 break;
+            case W3DTEXTURE_MIP_LEVELS_ALL:
             default:
                 mips = MipCountType::MIP_LEVELS_ALL;
                 break;
@@ -646,20 +647,27 @@ TextureClass *Load_Texture(ChunkLoadClass &cload)
 
     WW3DFormat format = WW3D_FORMAT_UNKNOWN;
 
-    if ((info.Attributes & W3DTEXTURE_TYPE_MASK) == W3DTEXTURE_TYPE_BUMPMAP) {
-        if (DX8Wrapper::Is_Initted()) {
-            if (DX8Wrapper::Get_Current_Caps()->Support_Bump_Envmap()) {
-                mips = MipCountType::MIP_LEVELS_1;
-
-                if (DX8Wrapper::Get_Current_Caps()->Supports_Texture_Format(WW3D_FORMAT_U8V8)) {
-                    format = WW3D_FORMAT_U8V8;
-                } else if (DX8Wrapper::Get_Current_Caps()->Supports_Texture_Format(WW3D_FORMAT_X8L8V8U8)) {
-                    format = WW3D_FORMAT_X8L8V8U8;
-                } else if (DX8Wrapper::Get_Current_Caps()->Supports_Texture_Format(WW3D_FORMAT_L6V5U5)) {
-                    format = WW3D_FORMAT_L6V5U5;
+    switch (info.Attributes & W3DTEXTURE_TYPE_MASK) {
+        case W3DTEXTURE_TYPE_COLORMAP:
+            break;
+        case W3DTEXTURE_TYPE_BUMPMAP:
+            if (DX8Wrapper::Is_Initted()) {
+                if (DX8Wrapper::Get_Current_Caps()->Support_Bump_Envmap()) {
+                    mips = MipCountType::MIP_LEVELS_1;
+
+                    if (DX8Wrapper::Get_Current_Caps()->Supports_Texture_Format(WW3D_FORMAT_U8V8)) {
+                        format = WW3D_FORMAT_U8V8;
+                    } else if (DX8Wrapper::Get_Current_Caps()->Supports_Texture_Format(WW3D_FORMAT_X8L8V8U8)) {
+                        format = WW3D_FORMAT_X8L8V8U8;
+                    } else if (DX8Wrapper::Get_Current_Caps()->Supports_Texture_Format(WW3D_FORMAT_L6V5U5)) {
+                        format = WW3D_FORMAT_L6V5U5;
+                    }
                 }
             }
-        }
+            break;
+        default:
+            captainslog_assert(false);
+            break;
     }
 
     TextureClass *tex = W3DAssetManager::Get_Instance()->Get_Texture(name, mips, format);