Merge pull request TrenchBroom#4342 from TrenchBroom/4254-remove-pale…

…tte-size-assumptions

4254: Don't assume color format from palette size

common/src/Assets/Palette.cpp

@@ -26,6 +26,7 @@
 #include "IO/File.h"
 #include "IO/ImageLoader.h"
 #include "IO/Reader.h"
+#include "Macros.h"
 
 #include <kdl/reflection_impl.h>
 #include <kdl/result.h>
@@ -38,17 +39,23 @@
 namespace TrenchBroom::Assets
 {
 
-struct PaletteData
+kdl_reflect_impl(PaletteData);
+
+std::ostream& operator<<(std::ostream& lhs, const PaletteColorFormat rhs)
 {
-  /**
-   * 1024 bytes, RGBA order.
-   */
-  std::vector<unsigned char> opaqueData;
-  /**
-   * 1024 bytes, RGBA order.
-   */
-  std::vector<unsigned char> index255TransparentData;
-};
+  switch (rhs)
+  {
+  case PaletteColorFormat::Rgb:
+    lhs << "Rgb";
+    break;
+  case PaletteColorFormat::Rgba:
+    lhs << "Rgba";
+    break;
+    switchDefault();
+  }
+
+  return lhs;
+}
 
 Palette::Palette(std::shared_ptr<PaletteData> data)
   : m_data{std::move(data)}
@@ -107,16 +114,45 @@ bool Palette::indexedToRgba(
   return hasTransparency;
 }
 
-Result<Palette> makePalette(const std::vector<unsigned char>& data)
+bool operator==(const Palette& lhs, const Palette& rhs)
+{
+  return lhs.m_data == rhs.m_data || *lhs.m_data == *rhs.m_data;
+}
+
+bool operator!=(const Palette& lhs, const Palette& rhs)
+{
+  return !(lhs == rhs);
+}
+
+std::ostream& operator<<(std::ostream& lhs, const Palette& rhs)
+{
+  auto str = kdl::struct_stream{lhs};
+  str << "Palette"
+      << "m_data";
+  if (rhs.m_data)
+  {
+    str << *rhs.m_data;
+  }
+  else
+  {
+    str << "nullptr";
+  }
+  return lhs;
+}
+
+
+Result<Palette> makePalette(
+  const std::vector<unsigned char>& data, const PaletteColorFormat colorFormat)
 {
   auto result = std::make_shared<PaletteData>();
 
-  if (data.size() == 768)
+  switch (colorFormat)
   {
+  case PaletteColorFormat::Rgb:
     // transform data to RGBA
-    result->opaqueData.reserve(1024);
+    result->opaqueData.reserve(data.size() / 3 * 4);
 
-    for (size_t i = 0; i < 256; ++i)
+    for (size_t i = 0; i < data.size() / 3; ++i)
     {
       const auto r = data[3 * i + 0];
       const auto g = data[3 * i + 1];
@@ -128,15 +164,18 @@ Result<Palette> makePalette(const std::vector<unsigned char>& data)
       result->opaqueData.push_back(0xFF);
     }
 
-    // build index255TransparentData from opaqueData
-    result->index255TransparentData = result->opaqueData;
-    result->index255TransparentData[1023] = 0;
-  }
-  else
-  {
+    if (!result->opaqueData.empty())
+    {
+      // build index255TransparentData from opaqueData
+      result->index255TransparentData = result->opaqueData;
+      result->index255TransparentData.back() = 0;
+    }
+    break;
+  case PaletteColorFormat::Rgba:
     // The data is already in RGBA format, don't process it
     result->opaqueData = data;
     result->index255TransparentData = data;
+    break;
   }
 
   return Palette{std::move(result)};
@@ -149,15 +188,15 @@ Result<Palette> loadLmp(IO::Reader& reader)
 {
   auto data = std::vector<unsigned char>(reader.size());
   reader.read(data.data(), data.size());
-  return makePalette(data);
+  return makePalette(data, PaletteColorFormat::Rgb);
 }
 
 Result<Palette> loadPcx(IO::Reader& reader)
 {
   auto data = std::vector<unsigned char>(768);
   reader.seekFromEnd(data.size());
   reader.read(data.data(), data.size());
-  return makePalette(data);
+  return makePalette(data, PaletteColorFormat::Rgb);
 }
 
 Result<Palette> loadBmp(IO::Reader& reader)
@@ -167,7 +206,7 @@ Result<Palette> loadBmp(IO::Reader& reader)
     IO::ImageLoader{IO::ImageLoader::BMP, bufferedReader.begin(), bufferedReader.end()};
   auto data = imageLoader.hasPalette() ? imageLoader.loadPalette()
                                        : imageLoader.loadPixels(IO::ImageLoader::RGB);
-  return makePalette(data);
+  return makePalette(data, PaletteColorFormat::Rgb);
 }
 
 } // namespace
@@ -202,13 +241,13 @@ Result<Palette> loadPalette(const IO::File& file, const std::filesystem::path& p
   }
 }
 
-Result<Palette> loadPalette(IO::Reader& reader)
+Result<Palette> loadPalette(IO::Reader& reader, const PaletteColorFormat colorFormat)
 {
   try
   {
     auto data = std::vector<unsigned char>(reader.size());
     reader.read(data.data(), data.size());
-    return makePalette(data);
+    return makePalette(data, colorFormat);
   }
   catch (const Exception& e)
   {

common/src/Assets/Palette.h

@@ -38,15 +38,36 @@ class Reader;
 
 namespace TrenchBroom::Assets
 {
-struct PaletteData;
 class TextureBuffer;
 
+struct PaletteData
+{
+  /**
+   * 1024 bytes, RGBA order.
+   */
+  std::vector<unsigned char> opaqueData;
+  /**
+   * 1024 bytes, RGBA order.
+   */
+  std::vector<unsigned char> index255TransparentData;
+
+  kdl_reflect_decl(PaletteData, opaqueData, index255TransparentData);
+};
+
 enum class PaletteTransparency
 {
   Opaque,
   Index255Transparent
 };
 
+enum class PaletteColorFormat
+{
+  Rgb,
+  Rgba,
+};
+
+std::ostream& operator<<(std::ostream& lhs, PaletteColorFormat rhs);
+
 class Palette
 {
 private:
@@ -79,11 +100,16 @@ class Palette
     TextureBuffer& rgbaImage,
     PaletteTransparency transparency,
     Color& averageColor) const;
+
+  friend bool operator==(const Palette& lhs, const Palette& rhs);
+  friend bool operator!=(const Palette& lhs, const Palette& rhs);
+  friend std::ostream& operator<<(std::ostream& lhs, const Palette& rhs);
 };
 
-Result<Palette> makePalette(const std::vector<unsigned char>& data);
+Result<Palette> makePalette(
+  const std::vector<unsigned char>& data, PaletteColorFormat colorFormat);
 
 Result<Palette> loadPalette(const IO::File& file, const std::filesystem::path& path);
-Result<Palette> loadPalette(IO::Reader& reader);
+Result<Palette> loadPalette(IO::Reader& reader, PaletteColorFormat colorFormat);
 
 } // namespace TrenchBroom::Assets

common/src/IO/ReadM8Texture.cpp

@@ -82,7 +82,7 @@ Result<Assets::Texture, ReadTextureError> readM8Texture(std::string name, Reader
     auto paletteReader = reader.subReaderFromCurrent(M8Layout::PaletteSize);
     reader.seekForward(M8Layout::PaletteSize);
 
-    return Assets::loadPalette(paletteReader)
+    return Assets::loadPalette(paletteReader, Assets::PaletteColorFormat::Rgb)
       .and_then([&](const auto& palette) {
         reader.seekForward(4); // flags
         reader.seekForward(4); // contents

common/src/IO/ReadMipTexture.cpp

@@ -60,7 +60,7 @@ Result<Assets::Palette> readHlMipPalette(Reader& reader)
   // palette data starts right after the color count
   auto data = std::vector<unsigned char>(colorCount * 3);
   reader.read(data.data(), data.size());
-  return Assets::makePalette(data);
+  return Assets::makePalette(data, Assets::PaletteColorFormat::Rgb);
 }
 
 Result<Assets::Texture, ReadTextureError> readMipTexture(

common/src/IO/ReadWalTexture.cpp

@@ -201,7 +201,7 @@ Result<Assets::Texture, ReadTextureError> readDkWal(std::string name, Reader& re
     const auto value = reader.readInt<int32_t>();
     const auto gameData = Assets::Q2Data{flags, contents, value};
 
-    return Assets::loadPalette(paletteReader)
+    return Assets::loadPalette(paletteReader, Assets::PaletteColorFormat::Rgb)
       .transform([&](const auto& palette) {
         auto [buffers, hasTransparency] = readMips(
           palette,

common/src/IO/SprParser.cpp

@@ -228,8 +228,8 @@ static Assets::Palette parseEmbeddedPalette(Reader& reader, const RenderMode ren
   auto data = std::vector<unsigned char>(paletteSize * 3);
   reader.read(data.data(), data.size());
   data = processGoldsourcePalette(renderMode, data);
-  return Assets::makePalette(data)
-    .if_error([](const auto& e) { throw AssetException{e.msg.c_str()}; })
+  return Assets::makePalette(data, Assets::PaletteColorFormat::Rgba)
+    .if_error([](const auto& e) { throw AssetException{e.msg}; })
     .value();
 }
 

common/test/CMakeLists.txt

@@ -25,6 +25,7 @@ set(COMMON_TEST_SOURCE
         "${COMMON_TEST_SOURCE_DIR}/Assets/tst_DecalDefinition.cpp"
         "${COMMON_TEST_SOURCE_DIR}/Assets/tst_EntityModel.cpp"
         "${COMMON_TEST_SOURCE_DIR}/Assets/tst_ModelDefinition.cpp"
+        "${COMMON_TEST_SOURCE_DIR}/Assets/tst_Palette.cpp"
         "${COMMON_TEST_SOURCE_DIR}/EL/tst_EL.cpp"
         "${COMMON_TEST_SOURCE_DIR}/EL/tst_Expression.cpp"
         "${COMMON_TEST_SOURCE_DIR}/EL/tst_Interpolator.cpp"