fix: handle embedded cubemap face order correctly (#996)

* fix: handle embedded cubemap face order correctly

* chore: do not treat cubemaps as separate logic when not necessary

* chore: make sure face major image data is the default and chosen for any game other than qos

* chore: make new tests use no namespace

---------

Co-authored-by: Jan Laupetin <[email protected]>

src/ObjImage/Image/DdsLoader.cpp

@@ -251,12 +251,11 @@ namespace
 
             result->Allocate();
 
-            for (auto mipLevel = 0; mipLevel < mipMapCount; mipLevel++)
+            for (auto face = 0; face < faceCount; face++)
             {
-                const auto mipSize = static_cast<std::streamsize>(result->GetSizeOfMipLevel(mipLevel));
-
-                for (auto face = 0; face < faceCount; face++)
+                for (auto mipLevel = 0; mipLevel < mipMapCount; mipLevel++)
                 {
+                    const auto mipSize = static_cast<std::streamsize>(result->GetSizeOfMipLevel(mipLevel));
                     m_stream.read(reinterpret_cast<char*>(result->GetBufferForMipLevel(mipLevel, face)), mipSize);
 
                     if (m_stream.gcount() != mipSize)

src/ObjImage/Image/DdsWriter.cpp

@@ -37,11 +37,14 @@ namespace
             }
 
             const auto mipCount = m_texture->HasMipMaps() ? m_texture->GetMipMapCount() : 1;
-            for (auto mipLevel = 0; mipLevel < mipCount; mipLevel++)
+            for (auto face = 0; face < m_texture->GetFaceCount(); face++)
             {
-                const auto* buffer = m_texture->GetBufferForMipLevel(mipLevel);
-                const auto mipLevelSize = m_texture->GetSizeOfMipLevel(mipLevel) * m_texture->GetFaceCount();
-                m_stream.write(reinterpret_cast<const char*>(buffer), static_cast<std::streamsize>(mipLevelSize));
+                for (auto mipLevel = 0; mipLevel < mipCount; mipLevel++)
+                {
+                    const auto* buffer = m_texture->GetBufferForMipLevel(mipLevel, face);
+                    const auto mipLevelSize = m_texture->GetSizeOfMipLevel(mipLevel);
+                    m_stream.write(reinterpret_cast<const char*>(buffer), static_cast<std::streamsize>(mipLevelSize));
+                }
             }
         }
 

src/ObjImage/Image/Dx12TextureLoader.cpp

@@ -92,9 +92,9 @@ namespace image
         const auto faceCount = m_type == TextureType::T_CUBE ? 6 : 1;
         const void* currentDataOffset = data;
 
-        for (auto currentMipLevel = 0; currentMipLevel < mipMapCount; currentMipLevel++)
+        for (auto currentFace = 0; currentFace < faceCount; currentFace++)
         {
-            for (auto currentFace = 0; currentFace < faceCount; currentFace++)
+            for (auto currentMipLevel = 0; currentMipLevel < mipMapCount; currentMipLevel++)
             {
                 const auto mipSize = texture->GetSizeOfMipLevel(currentMipLevel);
                 memcpy(texture->GetBufferForMipLevel(currentMipLevel, currentFace), currentDataOffset, mipSize);

src/ObjImage/Image/Dx9TextureLoader.cpp

@@ -4,14 +4,17 @@
 
 namespace
 {
+    void CopyMipLevelFace(image::Texture& texture, const int currentMipLevel, const int currentFace, const uint8_t*& currentDataOffset)
+    {
+        const auto mipSize = texture.GetSizeOfMipLevel(currentMipLevel);
+        memcpy(texture.GetBufferForMipLevel(currentMipLevel, currentFace), currentDataOffset, mipSize);
+        currentDataOffset += mipSize;
+    }
+
     void CopyMipLevel(image::Texture& texture, const int currentMipLevel, const int faceCount, const uint8_t*& currentDataOffset)
     {
         for (auto currentFace = 0; currentFace < faceCount; currentFace++)
-        {
-            const auto mipSize = texture.GetSizeOfMipLevel(currentMipLevel);
-            memcpy(texture.GetBufferForMipLevel(currentMipLevel, currentFace), currentDataOffset, mipSize);
-            currentDataOffset += mipSize;
-        }
+            CopyMipLevelFace(texture, currentMipLevel, currentFace, currentDataOffset);
     }
 } // namespace
 
@@ -22,6 +25,7 @@ namespace image
           m_type(TextureType::T_2D),
           m_has_mip_maps(false),
           m_mip_map_order(MipMapDataOrder::LargestToSmallest),
+          m_cube_map_data_order(CubeMapDataOrder::FaceMajor),
           m_width(1u),
           m_height(1u),
           m_depth(1u)
@@ -63,6 +67,12 @@ namespace image
         return *this;
     }
 
+    Dx9TextureLoader& Dx9TextureLoader::CubeMapOrder(const CubeMapDataOrder cubeMapDataOrder)
+    {
+        m_cube_map_data_order = cubeMapDataOrder;
+        return *this;
+    }
+
     Dx9TextureLoader& Dx9TextureLoader::Width(const unsigned width)
     {
         m_width = width;
@@ -112,7 +122,23 @@ namespace image
         const auto faceCount = m_type == TextureType::T_CUBE ? 6 : 1;
         auto* currentDataOffset = static_cast<const uint8_t*>(data);
 
-        if (m_mip_map_order == MipMapDataOrder::LargestToSmallest)
+        if (m_cube_map_data_order == CubeMapDataOrder::FaceMajor)
+        {
+            for (auto currentFace = 0; currentFace < faceCount; currentFace++)
+            {
+                if (m_mip_map_order == MipMapDataOrder::LargestToSmallest)
+                {
+                    for (auto currentMipLevel = 0; currentMipLevel < mipMapCount; currentMipLevel++)
+                        CopyMipLevelFace(*texture, currentMipLevel, currentFace, currentDataOffset);
+                }
+                else
+                {
+                    for (auto currentMipLevel = mipMapCount - 1; currentMipLevel >= 0; currentMipLevel--)
+                        CopyMipLevelFace(*texture, currentMipLevel, currentFace, currentDataOffset);
+                }
+            }
+        }
+        else if (m_mip_map_order == MipMapDataOrder::LargestToSmallest)
         {
             for (auto currentMipLevel = 0; currentMipLevel < mipMapCount; currentMipLevel++)
                 CopyMipLevel(*texture, currentMipLevel, faceCount, currentDataOffset);

src/ObjImage/Image/Dx9TextureLoader.h

@@ -16,12 +16,19 @@ namespace image
             SmallestToLargest,
         };
 
+        enum class CubeMapDataOrder
+        {
+            FaceMajor,
+            MipMajor,
+        };
+
         Dx9TextureLoader();
 
         Dx9TextureLoader& Format(oat::D3DFORMAT format);
         Dx9TextureLoader& Type(TextureType textureType);
         Dx9TextureLoader& HasMipMaps(bool hasMipMaps);
         Dx9TextureLoader& MipMapOrder(MipMapDataOrder mipMapOrder);
+        Dx9TextureLoader& CubeMapOrder(CubeMapDataOrder cubeMapDataOrder);
         Dx9TextureLoader& Width(unsigned width);
         Dx9TextureLoader& Height(unsigned height);
         Dx9TextureLoader& Depth(unsigned depth);
@@ -35,6 +42,7 @@ namespace image
         TextureType m_type;
         bool m_has_mip_maps;
         MipMapDataOrder m_mip_map_order;
+        CubeMapDataOrder m_cube_map_data_order;
         unsigned m_width;
         unsigned m_height;
         unsigned m_depth;

src/ObjLoading/Image/ImageLoaderEmbedded.cpp.template

@@ -152,12 +152,11 @@ namespace
             loadDef->resourceSize = static_cast<unsigned>(dataSize);
 
             char* currentDataBuffer = loadDef->data;
-            for (auto mipLevel = 0; mipLevel < mipCount; mipLevel++)
+            for (auto face = 0; face < faceCount; face++)
             {
-                const auto mipSize = texture->GetSizeOfMipLevel(mipLevel);
-
-                for (auto face = 0; face < faceCount; face++)
+                for (auto mipLevel = 0; mipLevel < mipCount; mipLevel++)
                 {
+                    const auto mipSize = texture->GetSizeOfMipLevel(mipLevel);
                     memcpy(currentDataBuffer, texture->GetBufferForMipLevel(mipLevel, face), mipSize);
                     currentDataBuffer += mipSize;
                 }

src/ObjWriting/Image/ImageToCommonConverter.cpp.template

@@ -113,6 +113,7 @@ namespace
 #ifdef FEATURE_QOS
         // The QOS executable uploads embedded load-def mip data from the smallest mip to the largest.
         textureLoader.MipMapOrder(Dx9TextureLoader::MipMapDataOrder::SmallestToLargest);
+        textureLoader.CubeMapOrder(Dx9TextureLoader::CubeMapDataOrder::MipMajor);
 #endif
 #else
         textureLoader.Format(static_cast<oat::DXGI_FORMAT>(loadDef.format));
@@ -169,4 +170,4 @@ namespace image
 
         return LoadImageFromIwi(image, searchPath);
     }
-}
+}

test/ObjCommonTests/Image/DdsCubeTests.cpp

@@ -0,0 +1,92 @@
+#include "Image/DdsLoader.h"
+#include "Image/DdsTypes.h"
+#include "Image/DdsWriter.h"
+#include "Image/Texture.h"
+
+#include <catch2/catch_test_macros.hpp>
+#include <cstring>
+#include <sstream>
+#include <string>
+#include <vector>
+
+using namespace image;
+
+namespace
+{
+    void FillCube(TextureCube& texture)
+    {
+        for (auto face = 0; face < texture.GetFaceCount(); face++)
+        {
+            for (auto mip = 0; mip < texture.GetMipMapCount(); mip++)
+            {
+                const auto value = static_cast<unsigned char>(face * 16 + mip + 1);
+                std::memset(texture.GetBufferForMipLevel(mip, face), value, texture.GetSizeOfMipLevel(mip));
+            }
+        }
+    }
+
+    std::vector<uint8_t> GetFaceMajorData(const TextureCube& texture)
+    {
+        std::vector<uint8_t> data;
+        for (auto face = 0; face < texture.GetFaceCount(); face++)
+        {
+            for (auto mip = 0; mip < texture.GetMipMapCount(); mip++)
+            {
+                const auto* buffer = texture.GetBufferForMipLevel(mip, face);
+                const auto mipSize = texture.GetSizeOfMipLevel(mip);
+                data.insert(data.end(), buffer, buffer + mipSize);
+            }
+        }
+
+        return data;
+    }
+
+    void RequireEqualCubeTextures(const TextureCube& expected, const Texture& actual)
+    {
+        REQUIRE(actual.GetTextureType() == TextureType::T_CUBE);
+        REQUIRE(actual.GetMipMapCount() == expected.GetMipMapCount());
+
+        for (auto face = 0; face < expected.GetFaceCount(); face++)
+        {
+            for (auto mip = 0; mip < expected.GetMipMapCount(); mip++)
+            {
+                const auto mipSize = expected.GetSizeOfMipLevel(mip);
+                REQUIRE(std::memcmp(actual.GetBufferForMipLevel(mip, face), expected.GetBufferForMipLevel(mip, face), mipSize) == 0);
+            }
+        }
+    }
+} // namespace
+
+TEST_CASE("DdsWriter stores cubemap mip chains face-major", "[image][dds]")
+{
+    TextureCube source(&format::A8, 8, 8, true);
+    source.Allocate();
+    FillCube(source);
+
+    std::stringstream stream(std::ios::in | std::ios::out | std::ios::binary);
+    DdsWriter writer;
+    writer.DumpImage(stream, &source);
+
+    const auto data = stream.str();
+    const auto expectedData = GetFaceMajorData(source);
+    constexpr auto dataOffset = sizeof(uint32_t) + sizeof(DDS_HEADER);
+
+    REQUIRE(data.size() == dataOffset + expectedData.size());
+    REQUIRE(std::memcmp(data.data() + dataOffset, expectedData.data(), expectedData.size()) == 0);
+}
+
+TEST_CASE("DdsLoader reads face-major cubemap mip chains", "[image][dds]")
+{
+    TextureCube source(&format::A8, 8, 8, true);
+    source.Allocate();
+    FillCube(source);
+
+    std::stringstream stream(std::ios::in | std::ios::out | std::ios::binary);
+    DdsWriter writer;
+    writer.DumpImage(stream, &source);
+    stream.seekg(0);
+
+    const auto loaded = LoadDds(stream);
+    REQUIRE(loaded);
+    RequireEqualCubeTextures(source, *loaded);
+}

test/ObjCommonTests/Image/Dx9TextureLoaderTests.cpp

@@ -0,0 +1,74 @@
+#include "Image/Dx9TextureLoader.h"
+#include "Image/Texture.h"
+
+#include <catch2/catch_test_macros.hpp>
+#include <cstring>
+#include <vector>
+
+using namespace image;
+
+namespace
+{
+    void FillCube(TextureCube& texture)
+    {
+        for (auto face = 0; face < texture.GetFaceCount(); face++)
+        {
+            for (auto mip = 0; mip < texture.GetMipMapCount(); mip++)
+            {
+                const auto value = static_cast<unsigned char>(face * 16 + mip + 1);
+                std::memset(texture.GetBufferForMipLevel(mip, face), value, texture.GetSizeOfMipLevel(mip));
+            }
+        }
+    }
+
+    std::vector<uint8_t> GetFaceMajorData(const TextureCube& texture)
+    {
+        std::vector<uint8_t> data;
+        for (auto face = 0; face < texture.GetFaceCount(); face++)
+        {
+            for (auto mip = 0; mip < texture.GetMipMapCount(); mip++)
+            {
+                const auto* buffer = texture.GetBufferForMipLevel(mip, face);
+                const auto mipSize = texture.GetSizeOfMipLevel(mip);
+                data.insert(data.end(), buffer, buffer + mipSize);
+            }
+        }
+
+        return data;
+    }
+
+    void RequireEqualCubeTextures(const TextureCube& expected, const Texture& actual)
+    {
+        REQUIRE(actual.GetTextureType() == TextureType::T_CUBE);
+        REQUIRE(actual.GetMipMapCount() == expected.GetMipMapCount());
+
+        for (auto face = 0; face < expected.GetFaceCount(); face++)
+        {
+            for (auto mip = 0; mip < expected.GetMipMapCount(); mip++)
+            {
+                const auto mipSize = expected.GetSizeOfMipLevel(mip);
+                REQUIRE(std::memcmp(actual.GetBufferForMipLevel(mip, face), expected.GetBufferForMipLevel(mip, face), mipSize) == 0);
+            }
+        }
+    }
+} // namespace
+
+TEST_CASE("Dx9TextureLoader reads face-major cubemap mip chains", "[image][dx9]")
+{
+    TextureCube source(&format::A8, 8, 8, true);
+    source.Allocate();
+    FillCube(source);
+
+    const auto data = GetFaceMajorData(source);
+    Dx9TextureLoader loader;
+    const auto loaded = loader.Format(source.GetFormat()->GetD3DFormat())
+                            .Type(TextureType::T_CUBE)
+                            .HasMipMaps(true)
+                            .CubeMapOrder(Dx9TextureLoader::CubeMapDataOrder::FaceMajor)
+                            .Width(source.GetWidth())
+                            .Height(source.GetHeight())
+                            .LoadTexture(data.data());
+
+    REQUIRE(loaded);
+    RequireEqualCubeTextures(source, *loaded);
+}

test/ObjLoadingTests/Game/IW3/Image/ImageLoaderEmbeddedIW3Test.cpp

@@ -1,14 +1,18 @@
 #include "Game/IW3/Image/ImageLoaderEmbeddedIW3.h"
 
 #include "Game/IW3/GameIW3.h"
+#include "Image/DdsWriter.h"
+#include "Image/Texture.h"
 #include "OatTestPaths.h"
 #include "SearchPath/MockSearchPath.h"
 #include "Utils/MemoryManager.h"
 
 #include <catch2/catch_test_macros.hpp>
+#include <cstring>
 #include <filesystem>
 #include <fstream>
 #include <memory>
+#include <sstream>
 #include <string>
 
 namespace fs = std::filesystem;
@@ -17,6 +21,18 @@ using namespace std::literals;
 
 namespace
 {
+    void FillCube(image::TextureCube& texture)
+    {
+        for (auto face = 0; face < texture.GetFaceCount(); face++)
+        {
+            for (auto mip = 0; mip < texture.GetMipMapCount(); mip++)
+            {
+                const auto value = static_cast<unsigned char>(face * 16 + mip + 1);
+                std::memset(texture.GetBufferForMipLevel(mip, face), value, texture.GetSizeOfMipLevel(mip));
+            }
+        }
+    }
+
     TEST_CASE("ImageLoaderEmbeddedIW3: Can parse dds", "[iw3][image]")
     {
         MockSearchPath searchPath;
@@ -54,4 +70,46 @@ namespace
         REQUIRE(image->texture.loadDef);
         REQUIRE(image->texture.loadDef->resourceSize > 0);
     }
+
+    TEST_CASE("ImageLoaderEmbeddedIW3: Stores cubemap mip chains face-major", "[iw3][image]")
+    {
+        image::TextureCube source(&image::format::A8, 8, 8, true);
+        source.Allocate();
+        FillCube(source);
+
+        std::stringstream ddsStream(std::ios::in | std::ios::out | std::ios::binary);
+        image::DdsWriter writer;
+        writer.DumpImage(ddsStream, &source);
+
+        MockSearchPath searchPath;
+        searchPath.AddFileData("images/_testcube.dds", ddsStream.str());
+
+        Zone zone("MockZone", 0, GameId::IW3, GamePlatform::PC);
+
+        MemoryManager memory;
+        AssetCreatorCollection creatorCollection(zone);
+        IgnoredAssetLookup ignoredAssetLookup;
+        AssetCreationContext context(zone, &creatorCollection, &ignoredAssetLookup);
+
+        auto loader = image::CreateLoaderEmbeddedIW3(memory, searchPath);
+        const auto result = loader->CreateAsset("*testcube", context);
+        REQUIRE(result.HasBeenSuccessful());
+
+        const auto* assetInfo = reinterpret_cast<XAssetInfo<GfxImage>*>(result.GetAssetInfo());
+        const auto* loadDef = assetInfo->Asset()->texture.loadDef;
+        REQUIRE(loadDef);
+
+        auto offset = 0uz;
+        for (auto face = 0; face < source.GetFaceCount(); face++)
+        {
+            for (auto mip = 0; mip < source.GetMipMapCount(); mip++)
+            {
+                const auto mipSize = source.GetSizeOfMipLevel(mip);
+                REQUIRE(std::memcmp(loadDef->data + offset, source.GetBufferForMipLevel(mip, face), mipSize) == 0);
+                offset += mipSize;
+            }
+        }
+
+        REQUIRE(offset == loadDef->resourceSize);
+    }
 } // namespace