Reduce the size of integer representations when possible

src/app/file/ase_format.cpp

@@ -1508,10 +1508,6 @@ static void ase_file_write_tileset_chunk(FILE* f, FileOp* fop,
 static void ase_file_write_property_value(FILE* f,
                                           const UserData::Variant& value)
 {
-  // TODO reduce value type depending on the actual value we're going
-  // to save (e.g. we don't need to save a 64-bit integer if the
-  // value=30, we can use a uint8_t for that case)
-
   switch (value.type()) {
     case USER_DATA_PROPERTY_TYPE_NULLPTR:
       ASSERT(false);
@@ -1568,17 +1564,23 @@ static void ase_file_write_property_value(FILE* f,
       break;
     }
     case USER_DATA_PROPERTY_TYPE_VECTOR: {
-      auto& v = *std::get_if<UserData::Vector>(&value);
+      auto& vector = *std::get_if<UserData::Vector>(&value);
-      fputl(v.size(), f);
+      fputl(vector.size(), f);
-      const uint16_t type = doc::all_elements_of_same_type(v);
+      const uint16_t type = doc::all_elements_of_same_type(vector);
       fputw(type, f);
-      for (const auto& elem : v) {
+      for (const auto& elem : vector) {
-        // Check that all elements have the same type when mode == 0. Or just that mode == 1
+        UserData::Variant v = elem;
-        ASSERT(type != 0 && type == elem.type() || type == 0);
         if (type == 0) {
-          fputw(elem.type(), f);
+          if (IS_REDUCIBLE_INT(v.type())) {
+            v = reduce_int_type_size(v);
+          }
+          fputw(v.type(), f);
         }
-        ase_file_write_property_value(f, elem);
+        else if (IS_REDUCIBLE_INT(v.type()) && type < v.type()) {
+          // We need to cast each value to the common type.
+          v = cast_to_smaller_int_type(v, type);
+        }
+        ase_file_write_property_value(f, v);
       }
       break;
     }
@@ -1590,11 +1592,13 @@ static void ase_file_write_property_value(FILE* f,
       for (auto property : properties) {
         const std::string& name = property.first;
         ase_file_write_string(f, name);
+        UserData::Variant v = property.second;
+        if (IS_REDUCIBLE_INT(v.type())) {
+          v = reduce_int_type_size(v);
+        }
+        fputw(v.type(), f);
 
-        const UserData::Variant& value = property.second;
+        ase_file_write_property_value(f, v);
-        fputw(value.type(), f);
-
-        ase_file_write_property_value(f, value);
       }
       break;
     }

src/app/file/file_tests.cpp

@@ -226,7 +226,7 @@ TEST(File, CustomProperties)
                   }));
 
         ASSERT_EQ(doc::get_value<doc::UserData::Vector>(sprite->userData().properties("ext")["numbers"]),
-                  (doc::UserData::Vector {int32_t(11), int32_t(22), int32_t(33)}));
+                  (doc::UserData::Vector {int8_t(11), int8_t(22), int8_t(33)}));
 
         ASSERT_EQ(doc::get_value<doc::UserData::Properties>(sprite->userData().properties("ext")["player"]),
                   (doc::UserData::Properties {
@@ -235,6 +235,46 @@ TEST(File, CustomProperties)
                     {"cards", doc::UserData::Vector {int8_t(11), int8_t(6), int8_t(0), int8_t(13)}}
                   }));
       }
+    },
+    { // Test size reduction of integer properties
+      "test_props_4.ase", 50, 50, doc::ColorMode::INDEXED, 256,
+      {
+        {"", {
+               {"int16_to_int8", int16_t(127)},
+               {"int16_to_uint8", int16_t(128)},
+               {"int32_to_int8", int32_t(126)},
+               {"int32_to_uint8", int32_t(129)},
+               {"int32_to_int16", int32_t(32767)},
+               {"int32_to_uint16", int32_t(32768)},
+               {"int64_to_int8", int64_t(125)},
+               {"int64_to_uint8", int64_t(130)},
+               {"int64_to_int16", int64_t(32765)},
+               {"int64_to_uint16", int64_t(32769)},
+               {"int64_to_int32", int64_t(2147483647)},
+               {"int64_to_uint32", int64_t(2147483648)},
+               {"v1", doc::UserData::Vector {uint64_t(18446744073709551615ULL), uint64_t(6), uint64_t(0), uint64_t(13)}},
+             }
+        }
+      },
+      [](const TestCase& test, doc::Sprite* sprite){
+        sprite->userData().propertiesMaps() = test.propertiesMaps;
+      },
+      [](const TestCase& test, doc::Sprite* sprite){
+        ASSERT_EQ(doc::get_value<int8_t>(sprite->userData().properties()["int16_to_int8"]), 127);
+        ASSERT_EQ(doc::get_value<uint8_t>(sprite->userData().properties()["int16_to_uint8"]), 128);
+        ASSERT_EQ(doc::get_value<int8_t>(sprite->userData().properties()["int32_to_int8"]), 126);
+        ASSERT_EQ(doc::get_value<uint8_t>(sprite->userData().properties()["int32_to_uint8"]), 129);
+        ASSERT_EQ(doc::get_value<int16_t>(sprite->userData().properties()["int32_to_int16"]), 32767);
+        ASSERT_EQ(doc::get_value<uint16_t>(sprite->userData().properties()["int32_to_uint16"]), 32768);
+        ASSERT_EQ(doc::get_value<int8_t>(sprite->userData().properties()["int64_to_int8"]), 125);
+        ASSERT_EQ(doc::get_value<uint8_t>(sprite->userData().properties()["int64_to_uint8"]), 130);
+        ASSERT_EQ(doc::get_value<int16_t>(sprite->userData().properties()["int64_to_int16"]), 32765);
+        ASSERT_EQ(doc::get_value<uint16_t>(sprite->userData().properties()["int64_to_uint16"]), 32769);
+        ASSERT_EQ(doc::get_value<int32_t>(sprite->userData().properties()["int64_to_int32"]), 2147483647);
+        ASSERT_EQ(doc::get_value<uint32_t>(sprite->userData().properties()["int64_to_uint32"]), 2147483648);
+        ASSERT_EQ(doc::get_value<doc::UserData::Vector>(sprite->userData().properties()["v1"]),
+                  (doc::UserData::Vector {uint64_t(18446744073709551615ULL), uint64_t(6), uint64_t(0), uint64_t(13)}));
+      }
     }
   };
 

src/doc/user_data.h

@@ -42,6 +42,15 @@
 #define USER_DATA_PROPERTY_TYPE_VECTOR      0x0011
 #define USER_DATA_PROPERTY_TYPE_PROPERTIES  0x0012
 
+#define INT8_COMPATIBLE(i)   i >= -128        && i <= 127
+#define UINT8_COMPATIBLE(i)  i >= 128         && i <= 255
+#define INT16_COMPATIBLE(i)  i >= -32768      && i <= 32767
+#define UINT16_COMPATIBLE(i) i >= 32768       && i <= 65535
+#define INT32_COMPATIBLE(i)  i >= -2147483648 && i <= 2147483647
+#define UINT32_COMPATIBLE(i) i >= 2147483648  && i <= 4294967295
+
+#define IS_REDUCIBLE_INT(variantType) variantType >= USER_DATA_PROPERTY_TYPE_INT16 && variantType <= USER_DATA_PROPERTY_TYPE_UINT64
+
 namespace doc {
 
   class UserData {
@@ -151,7 +160,11 @@ namespace doc {
 
   // If all the vector elements are of the same type returns such type.
   // Otherwise it returns -1.
-  int all_elements_of_same_type(const UserData::Vector& vector);
+  uint16_t all_elements_of_same_type(const UserData::Vector& vector);
+
+  UserData::Variant reduce_int_type_size(const UserData::Variant& value);
+
+  UserData::Variant cast_to_smaller_int_type(const UserData::Variant& value, uint16_t type);
 
 } // namespace doc
 

src/doc/user_data_io.cpp

@@ -47,7 +47,8 @@ UserData read_user_data(std::istream& is)
   return userData;
 }
 
-size_t count_nonempty_properties_maps(const UserData::PropertiesMaps& propertiesMaps) {
+size_t count_nonempty_properties_maps(const UserData::PropertiesMaps& propertiesMaps)
+{
   size_t i = 0;
   for (const auto& it : propertiesMaps)
     if (!it.second.empty())
@@ -55,15 +56,203 @@ size_t count_nonempty_properties_maps(const UserData::PropertiesMaps& properties
   return i;
 }
 
+bool is_negative(const UserData::Variant& value)
+{
+  switch (value.type()) {
+    case USER_DATA_PROPERTY_TYPE_INT8: {
+      auto v = get_value<int8_t>(value);
+      return v < 0;
+    }
+    case USER_DATA_PROPERTY_TYPE_INT16: {
+      auto v = get_value<int16_t>(value);
+      return v < 0;
+    }
+    case USER_DATA_PROPERTY_TYPE_INT32: {
+      auto v = get_value<int32_t>(value);
+      return v < 0;
+    }
+    case USER_DATA_PROPERTY_TYPE_INT64: {
+      auto v = get_value<int64_t>(value);
+      return v < 0;
+    }
+  }
+  return false;
+}
 
-int all_elements_of_same_type(const UserData::Vector& vector) {
+
-  int type = vector.empty() ? 0 : vector.front().type();
+// If all the elements of vector have the same type, returns that type, also
+// if this type is an integer, it tries to reduce it to the minimum int type
+// capable of storing all the vector values.
+// If all the elements of vector doesn't have the same type, returns 0.
+uint16_t all_elements_of_same_type(const UserData::Vector& vector)
+{
+  uint16_t type = vector.empty() ? 0 : vector.front().type();
+  uint16_t commonReducedType = 0;
+  bool hasNegativeNumbers = false;
   for (auto value : vector) {
     if (type != value.type()) {
       return 0;
     }
+    else if (IS_REDUCIBLE_INT(value.type())) {
+      auto t = reduce_int_type_size(value).type();
+      hasNegativeNumbers |= is_negative(value);
+      if (t > commonReducedType) {
+        commonReducedType = t;
+      }
+    }
+  }
+
+  // TODO: The following check probably is not useful right now, I believe this could
+  // become useful if at some point we want to try to find a common integer type for vectors
+  // that contains elements of different integer types only.
+
+  // If our common reduced type is unsigned and we have negative numbers
+  // in our vector we should select the next signed type that includes it.
+  if (commonReducedType != 0 &&
+      (commonReducedType & 1) &&
+      hasNegativeNumbers) {
+    commonReducedType++;
+    // We couldn't find one type that satisfies all the integers. This shouldn't ever happen.
+    if (commonReducedType >= USER_DATA_PROPERTY_TYPE_UINT64) commonReducedType = 0;
+  }
+
+  return commonReducedType ? commonReducedType : type;
+}
+
+UserData::Variant cast_to_smaller_int_type(const UserData::Variant& value, uint16_t type)
+{
+  ASSERT(type < value.type());
+  switch (value.type()) {
+    case USER_DATA_PROPERTY_TYPE_INT16: {
+      auto v = get_value<int16_t>(value);
+      if (type == USER_DATA_PROPERTY_TYPE_INT8)
+        return static_cast<int8_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_UINT8)
+        return static_cast<uint8_t>(v);
+      break;
+    }
+    case USER_DATA_PROPERTY_TYPE_UINT16: {
+      auto v = get_value<uint16_t>(value);
+      if (type == USER_DATA_PROPERTY_TYPE_INT8)
+        return static_cast<int8_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_UINT8)
+        return static_cast<uint8_t>(v);
+      break;
+    }
+    case USER_DATA_PROPERTY_TYPE_INT32: {
+      auto v = get_value<int32_t>(value);
+      if (type == USER_DATA_PROPERTY_TYPE_INT8)
+        return static_cast<int8_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_UINT8)
+        return static_cast<uint8_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_INT16)
+        return static_cast<int16_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_UINT16)
+        return static_cast<uint16_t>(v);
+      break;
+    }
+    case USER_DATA_PROPERTY_TYPE_UINT32: {
+      auto v = get_value<uint32_t>(value);
+      if (type == USER_DATA_PROPERTY_TYPE_INT8)
+        return static_cast<int8_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_UINT8)
+        return static_cast<uint8_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_INT16)
+        return static_cast<int16_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_UINT16)
+        return static_cast<uint16_t>(v);
+      break;
+    }
+    case USER_DATA_PROPERTY_TYPE_INT64: {
+      auto v = get_value<int64_t>(value);
+      if (type == USER_DATA_PROPERTY_TYPE_INT8)
+        return static_cast<int8_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_UINT8)
+        return static_cast<uint8_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_INT16)
+        return static_cast<int16_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_UINT16)
+        return static_cast<uint16_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_INT32)
+        return static_cast<int32_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_UINT32)
+        return static_cast<uint32_t>(v);
+      break;
+    }
+    case USER_DATA_PROPERTY_TYPE_UINT64: {
+      auto v = get_value<uint64_t>(value);
+      if (type == USER_DATA_PROPERTY_TYPE_INT8)
+        return static_cast<int8_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_UINT8)
+        return static_cast<uint8_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_INT16)
+        return static_cast<int16_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_UINT16)
+        return static_cast<uint16_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_INT32)
+        return static_cast<int32_t>(v);
+      else if (type == USER_DATA_PROPERTY_TYPE_UINT32)
+        return static_cast<uint32_t>(v);
+      break;
+    }
+  }
+  return value;
+}
+
+UserData::Variant reduce_int_type_size(const UserData::Variant& value)
+{
+  switch (value.type()) {
+    case USER_DATA_PROPERTY_TYPE_INT16: {
+      auto v = get_value<int16_t>(value);
+      if (INT8_COMPATIBLE(v)) return static_cast<int8_t>(v);
+      else if (UINT8_COMPATIBLE(v)) return static_cast<uint8_t>(v);
+      return v;
+    }
+    case USER_DATA_PROPERTY_TYPE_UINT16: {
+      auto v = get_value<uint16_t>(value);
+      if (INT8_COMPATIBLE(v)) return static_cast<int8_t>(v);
+      else if (UINT8_COMPATIBLE(v)) return static_cast<uint8_t>(v);
+      return v;
+    }
+    case USER_DATA_PROPERTY_TYPE_INT32: {
+      auto v = get_value<int32_t>(value);
+      if (INT8_COMPATIBLE(v)) return static_cast<int8_t>(v);
+      else if (UINT8_COMPATIBLE(v)) return static_cast<uint8_t>(v);
+      else if (INT16_COMPATIBLE(v)) return static_cast<int16_t>(v);
+      else if (UINT16_COMPATIBLE(v)) return static_cast<uint16_t>(v);
+      return v;
+    }
+    case USER_DATA_PROPERTY_TYPE_UINT32: {
+      auto v = get_value<uint32_t>(value);
+      if (INT8_COMPATIBLE(v)) return static_cast<int8_t>(v);
+      else if (UINT8_COMPATIBLE(v)) return static_cast<uint8_t>(v);
+      else if (INT16_COMPATIBLE(v)) return static_cast<int16_t>(v);
+      else if (UINT16_COMPATIBLE(v)) return static_cast<uint16_t>(v);
+      return v;
+    }
+    case USER_DATA_PROPERTY_TYPE_INT64: {
+      auto v = get_value<int64_t>(value);
+      if (INT8_COMPATIBLE(v)) return static_cast<int8_t>(v);
+      else if (UINT8_COMPATIBLE(v)) return static_cast<uint8_t>(v);
+      else if (INT16_COMPATIBLE(v)) return static_cast<int16_t>(v);
+      else if (UINT16_COMPATIBLE(v)) return static_cast<uint16_t>(v);
+      else if (INT32_COMPATIBLE(v)) return static_cast<int32_t>(v);
+      else if (UINT32_COMPATIBLE(v)) return static_cast<uint32_t>(v);
+      return v;
+    }
+    case USER_DATA_PROPERTY_TYPE_UINT64: {
+      auto v = get_value<uint64_t>(value);
+      if (INT8_COMPATIBLE(v)) return static_cast<int8_t>(v);
+      else if (UINT8_COMPATIBLE(v)) return static_cast<uint8_t>(v);
+      else if (INT16_COMPATIBLE(v)) return static_cast<int16_t>(v);
+      else if (UINT16_COMPATIBLE(v)) return static_cast<uint16_t>(v);
+      else if (INT32_COMPATIBLE(v)) return static_cast<int32_t>(v);
+      else if (UINT32_COMPATIBLE(v)) return static_cast<uint32_t>(v);
+      return v;
+    }
+    default:
+      return value;
   }
-  return type;
 }
 
 void write_point(std::ostream& os, const gfx::Point& point)