Various C++ optimizations for compression code

src/app/rom.cc

@@ -33,23 +33,14 @@ int GetGraphicsAddress(const uchar* data, uint8_t offset) {
   return core::SnesToPc(snes_addr);
 }
 
-char* HexString(const char* str, const uint size) {
-  char* toret = (char*)malloc(size * 3 + 1);
-
-  uint i;
-  for (i = 0; i < size; i++) {
-    sprintf(toret + i * 3, "%02X ", (unsigned char)str[i]);
-  }
-  toret[size * 3] = 0;
-  return toret;
-}
-
 void PrintCompressionPiece(const std::shared_ptr<CompressionPiece>& piece) {
   printf("Command : %d\n", piece->command);
   printf("length  : %d\n", piece->length);
-  printf("Argument length : %d\n", piece->argument_length);
+  printf("Argument :");
-  printf("Argument :%s\n",
+  for (int i = 0; i < piece->argument.size(); ++i) {
-         HexString(piece->argument.data(), piece->argument_length));
+    printf("%02X ", piece->argument.at(i));
+  }
+  printf("\nArgument length : %d\n", piece->argument_length);
 }
 
 std::shared_ptr<CompressionPiece> NewCompressionPiece(
@@ -350,9 +341,9 @@ void CompressionDirectCopy(const uchar* rom_data,
 
   // Arbitrary choice to do a 32 bytes grouping
   if (bytes_since_last_compression == 32 || u_data_pos > last_pos) {
-    char buffer[32];
+    std::string buffer;
     for (int i = 0; i < bytes_since_last_compression; ++i) {
-      buffer[i] = rom_data[i + u_data_pos - bytes_since_last_compression];
+      buffer.push_back(rom_data[i + u_data_pos - bytes_since_last_compression]);
     }
     auto new_comp_piece =
         NewCompressionPiece(kCommandDirectCopy, bytes_since_last_compression,
@@ -369,16 +360,14 @@ void CompressionCommandAlternative(
     uint& u_data_pos, uint& bytes_since_last_compression, uint& cmd_with_max,
     uint& max_win) {
   printf("- Ok we get a gain from %d\n", cmd_with_max);
-  char buffer[2];
+  std::string buffer;
-  buffer[0] = cmd_args[cmd_with_max][0];
+  buffer.push_back(cmd_args[cmd_with_max][0]);
-
   if (cmd_size[cmd_with_max] == 2) {
-    buffer[1] = cmd_args[cmd_with_max][1];
+    buffer.push_back(cmd_args[cmd_with_max][1]);
   }
 
   auto new_comp_piece = NewCompressionPiece(cmd_with_max, max_win, buffer,
                                             cmd_size[cmd_with_max]);
-  printf("Here");
   PrintCompressionPiece(new_comp_piece);
   // If we let non compressed stuff, we need to add a copy chuck before
   if (bytes_since_last_compression != 0) {
@@ -404,7 +393,6 @@ void CompressionCommandAlternative(
 // TODO TEST compressed data border for each cmd
 absl::StatusOr<Bytes> ROM::Compress(const int start, const int length,
                                     int mode) {
-  Bytes compressed_data(length + 10);
   // Worse case should be a copy of the string with extended header
   auto compressed_chain = NewCompressionPiece(1, 1, "aaa", 2);
   auto compressed_chain_start = compressed_chain;
@@ -440,24 +428,26 @@ absl::StatusOr<Bytes> ROM::Compress(const int start, const int length,
           bytes_since_last_compression, cmd_with_max, max_win);
     }
 
-    if (u_data_pos > last_pos) break;
+    if (u_data_pos > last_pos) {
+      printf("Breaking compression loop\n");
+      break;
+    }
 
     // Validate compression result
     if (compressed_chain_start->next != nullptr) {
-      // We don't call merge copy so we need more space
+      ROM temp_rom;
-      auto tmp = (uchar*)malloc(length * 2);
+      auto rom_response = temp_rom.LoadFromBytes(
-      auto compressed_size =
+          CreateCompressionStringV2(compressed_chain_start->next, mode));
-          CreateCompressionStringV2(compressed_chain_start->next, tmp, mode);
+      if (!rom_response.ok()) {
-      uint p;
+        return rom_response;
-
-      auto response = Decompress(0);
-      if (!response.ok()) {
-        return response.status();
       }
-      auto uncomp = std::move(*response);
+      auto decomp_response = temp_rom.Decompress(0, temp_rom.GetSize());
-      free(tmp);
+      if (!decomp_response.ok()) {
-      if (memcmp(uncomp.data(), rom_data_.data() + start, p) != 0) {
+        return decomp_response.status();
-        // FreeCompressionChain(compressed_chain_start);
+      }
+      auto decomp_data = std::move(*decomp_response);
+      if (!std::equal(decomp_data.begin() + start, decomp_data.end(),
+                      temp_rom.begin())) {
         return absl::InternalError(absl::StrFormat(
             "Compressed data does not match uncompressed data at %d\n",
             (uint)(u_data_pos - start)));
@@ -465,7 +455,7 @@ absl::StatusOr<Bytes> ROM::Compress(const int start, const int length,
     }
   }
 
-  MergeCopy(compressed_chain_start->next);  // First is a dumb place holder
+  MergeCopy(compressed_chain_start->next);  // Skipping compression chain header
 
   compressed_chain = compressed_chain_start->next;
   while (compressed_chain != NULL) {
@@ -474,14 +464,7 @@ absl::StatusOr<Bytes> ROM::Compress(const int start, const int length,
     compressed_chain = compressed_chain->next;
   }
 
-  uchar temporary_string[length + 10];
+  return CreateCompressionStringV2(compressed_chain_start->next, mode);
-  auto compressed_size = CreateCompressionString(compressed_chain_start->next,
-                                                 temporary_string, mode);
-  for (int i = 0; i < compressed_size; ++i) {
-    compressed_data[i] = temporary_string[i];
-  }
-
-  return compressed_data;
 }
 
 absl::StatusOr<Bytes> ROM::CompressGraphics(const int pos, const int length) {
@@ -636,13 +619,22 @@ absl::Status ROM::LoadFromFile(const absl::string_view& filename) {
 absl::Status ROM::LoadFromPointer(uchar* data, size_t length) {
   if (data == nullptr)
     return absl::InvalidArgumentError(
-        "Could not load ROM: parameter `data` is empty");
+        "Could not load ROM: parameter `data` is empty.");
 
   for (int i = 0; i < length; ++i) rom_data_.push_back(data[i]);
 
   return absl::OkStatus();
 }
 
+absl::Status ROM::LoadFromBytes(Bytes data) {
+  if (data.empty()) {
+    return absl::InvalidArgumentError(
+        "Could not load ROM: parameter `data` is empty.");
+  }
+  rom_data_ = data;
+  return absl::OkStatus();
+}
+
 // 0-112 -> compressed 3bpp bgr -> (decompressed each) 0x600 chars
 // 113-114 -> compressed 2bpp -> (decompressed each) 0x800 chars
 // 115-126 -> uncompressed 3bpp sprites -> (each) 0x600 chars

src/app/rom.h

@@ -39,23 +39,13 @@ constexpr int kTile32Num = 4432;
 constexpr uchar kGraphicsBitmap[8] = {0x80, 0x40, 0x20, 0x10,
                                       0x08, 0x04, 0x02, 0x01};
 
-using OWBlockset = std::vector<std::vector<ushort>>;
-struct OWMapTiles {
-  OWBlockset light_world;    // 64 maps
-  OWBlockset dark_world;     // 64 maps
-  OWBlockset special_world;  // 32 maps
-} typedef OWMapTiles;
-
 using CommandArgumentArray = std::array<std::array<char, 2>, 5>;
 using CommandSizeArray = std::array<uint, 5>;
 using DataSizeArray = std::array<uint, 5>;
 struct CompressionPiece {
   char command;
   int length;
-  // char* argument;
   int argument_length;
-  // CompressionPiece* next;
-
   std::string argument;
   std::shared_ptr<CompressionPiece> next;
   CompressionPiece() {}
@@ -67,6 +57,13 @@ struct CompressionPiece {
         next(nullptr) {}
 } typedef CompressionPiece;
 
+using OWBlockset = std::vector<std::vector<ushort>>;
+struct OWMapTiles {
+  OWBlockset light_world;    // 64 maps
+  OWBlockset dark_world;     // 64 maps
+  OWBlockset special_world;  // 32 maps
+} typedef OWMapTiles;
+
 class ROM {
  public:
   absl::StatusOr<Bytes> Compress(const int start, const int length,
@@ -84,16 +81,18 @@ class ROM {
   absl::Status LoadAllGraphicsData();
   absl::Status LoadFromFile(const absl::string_view& filename);
   absl::Status LoadFromPointer(uchar* data, size_t length);
+  absl::Status LoadFromBytes(Bytes data);
 
   auto GetSize() const { return size_; }
   auto GetTitle() const { return title; }
   auto GetGraphicsBin() const { return graphics_bin_; }
-  auto isLoaded() const { return is_loaded_; }
-
-  auto Renderer() { return renderer_; }
   void SetupRenderer(std::shared_ptr<SDL_Renderer> renderer) {
     renderer_ = renderer;
   }
+  auto isLoaded() const { return is_loaded_; }
+  auto begin() { return rom_data_.begin(); }
+  auto end() { return rom_data_.end(); }
+
   uchar& operator[](int i) {
     if (i > size_) {
       std::cout << "ROM: Index out of bounds" << std::endl;