From cc71ec738729364e53b2b350bcc320681d6ac723 Mon Sep 17 00:00:00 2001 From: mo Date: Tue, 3 Nov 2026 21:47:04 +0000 Subject: [PATCH] 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 --- src/ObjImage/Image/DdsLoader.cpp | 7 +- src/ObjImage/Image/DdsWriter.cpp | 11 ++- src/ObjImage/Image/Dx12TextureLoader.cpp | 4 +- src/ObjImage/Image/Dx9TextureLoader.cpp | 38 ++++++-- src/ObjImage/Image/Dx9TextureLoader.h | 8 ++ .../Image/ImageLoaderEmbedded.cpp.template | 7 +- .../Image/ImageToCommonConverter.cpp.template | 3 +- test/ObjCommonTests/Image/DdsCubeTests.cpp | 92 +++++++++++++++++++ .../Image/Dx9TextureLoaderTests.cpp | 74 +++++++++++++++ .../IW3/Image/ImageLoaderEmbeddedIW3Test.cpp | 58 ++++++++++++ 10 files changed, 281 insertions(+), 21 deletions(-) create mode 100644 test/ObjCommonTests/Image/DdsCubeTests.cpp create mode 100644 test/ObjCommonTests/Image/Dx9TextureLoaderTests.cpp diff --git a/src/ObjImage/Image/DdsLoader.cpp b/src/ObjImage/Image/DdsLoader.cpp index fc8af93ac..d675b3a23 100644 --- a/src/ObjImage/Image/DdsLoader.cpp +++ b/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(result->GetSizeOfMipLevel(mipLevel)); - - for (auto face = 0; face < faceCount; face++) + for (auto mipLevel = 0; mipLevel < mipMapCount; mipLevel++) { + const auto mipSize = static_cast(result->GetSizeOfMipLevel(mipLevel)); m_stream.read(reinterpret_cast(result->GetBufferForMipLevel(mipLevel, face)), mipSize); if (m_stream.gcount() != mipSize) diff --git a/src/ObjImage/Image/DdsWriter.cpp b/src/ObjImage/Image/DdsWriter.cpp index e4bffb62d..43c480622 100644 --- a/src/ObjImage/Image/DdsWriter.cpp +++ b/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(buffer), static_cast(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(buffer), static_cast(mipLevelSize)); + } } } diff --git a/src/ObjImage/Image/Dx12TextureLoader.cpp b/src/ObjImage/Image/Dx12TextureLoader.cpp index f132f0b78..34cb2a5dd 100644 --- a/src/ObjImage/Image/Dx12TextureLoader.cpp +++ b/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); diff --git a/src/ObjImage/Image/Dx9TextureLoader.cpp b/src/ObjImage/Image/Dx9TextureLoader.cpp index 7edf1d739..1f5908d55 100644 --- a/src/ObjImage/Image/Dx9TextureLoader.cpp +++ b/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(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); diff --git a/src/ObjImage/Image/Dx9TextureLoader.h b/src/ObjImage/Image/Dx9TextureLoader.h index 316c7f0f7..8b7fa77fc 100644 --- a/src/ObjImage/Image/Dx9TextureLoader.h +++ b/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; diff --git a/src/ObjLoading/Image/ImageLoaderEmbedded.cpp.template b/src/ObjLoading/Image/ImageLoaderEmbedded.cpp.template index bdc6f3ceb..c9acb7609 100644 --- a/src/ObjLoading/Image/ImageLoaderEmbedded.cpp.template +++ b/src/ObjLoading/Image/ImageLoaderEmbedded.cpp.template @@ -152,12 +152,11 @@ namespace loadDef->resourceSize = static_cast(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; } diff --git a/src/ObjWriting/Image/ImageToCommonConverter.cpp.template b/src/ObjWriting/Image/ImageToCommonConverter.cpp.template index c00784d35..2f3fa9b68 100644 --- a/src/ObjWriting/Image/ImageToCommonConverter.cpp.template +++ b/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(loadDef.format)); @@ -169,4 +170,4 @@ namespace image return LoadImageFromIwi(image, searchPath); } -} \ No newline at end of file +} diff --git a/test/ObjCommonTests/Image/DdsCubeTests.cpp b/test/ObjCommonTests/Image/DdsCubeTests.cpp new file mode 100644 index 000000000..2a306261a --- /dev/null +++ b/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 +#include +#include +#include +#include + +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(face * 16 + mip + 1); + std::memset(texture.GetBufferForMipLevel(mip, face), value, texture.GetSizeOfMipLevel(mip)); + } + } + } + + std::vector GetFaceMajorData(const TextureCube& texture) + { + std::vector 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); +} diff --git a/test/ObjCommonTests/Image/Dx9TextureLoaderTests.cpp b/test/ObjCommonTests/Image/Dx9TextureLoaderTests.cpp new file mode 100644 index 000000000..67ac8c94f --- /dev/null +++ b/test/ObjCommonTests/Image/Dx9TextureLoaderTests.cpp @@ -0,0 +1,74 @@ +#include "Image/Dx9TextureLoader.h" +#include "Image/Texture.h" + +#include +#include +#include + +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(face * 16 + mip + 1); + std::memset(texture.GetBufferForMipLevel(mip, face), value, texture.GetSizeOfMipLevel(mip)); + } + } + } + + std::vector GetFaceMajorData(const TextureCube& texture) + { + std::vector 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); +} diff --git a/test/ObjLoadingTests/Game/IW3/Image/ImageLoaderEmbeddedIW3Test.cpp b/test/ObjLoadingTests/Game/IW3/Image/ImageLoaderEmbeddedIW3Test.cpp index d479c727f..768d56e18 100644 --- a/test/ObjLoadingTests/Game/IW3/Image/ImageLoaderEmbeddedIW3Test.cpp +++ b/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 +#include #include #include #include +#include #include 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(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*>(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