GCMemcard: Read banners according to logical data offsets instead of physical data offsets. Also gets rid of some undefined behavior.

Source/Core/Core/HW/GCMemcard/GCMemcard.cpp

@@ -1186,46 +1186,47 @@ void GCMemcard::Gcs_SavConvert(DEntry& tempDEntry, int saveType, u64 length)
   }
 }
 
-bool GCMemcard::ReadBannerRGBA8(u8 index, u32* buffer) const
+std::optional<std::vector<u32>> GCMemcard::ReadBannerRGBA8(u8 index) const
 {
   if (!m_valid || index >= DIRLEN)
-    return false;
+    return std::nullopt;
 
-  int flags = GetActiveDirectory().m_dir_entries[index].m_banner_and_icon_flags;
+  const u32 offset = GetActiveDirectory().m_dir_entries[index].m_image_offset;
-  // Timesplitters 2 is the only game that I see this in
+  if (offset == 0xFFFFFFFF)
-  // May be a hack
+    return std::nullopt;
-  if (flags == 0xFB)
-    flags = ~flags;
 
-  int bnrFormat = (flags & 3);
+  // See comment on m_banner_and_icon_flags for an explanation of these.
+  const u8 flags = GetActiveDirectory().m_dir_entries[index].m_banner_and_icon_flags;
+  const u8 format = (flags & 0b0000'0011);
+  if (format != MEMORY_CARD_BANNER_FORMAT_CI8 && format != MEMORY_CARD_BANNER_FORMAT_RGB5A3)
+    return std::nullopt;
 
-  if (bnrFormat == 0)
+  constexpr u32 pixel_count = MEMORY_CARD_BANNER_WIDTH * MEMORY_CARD_BANNER_HEIGHT;
-    return false;
+  const size_t total_bytes = format == MEMORY_CARD_BANNER_FORMAT_CI8 ?
+                                 (pixel_count + MEMORY_CARD_CI8_PALETTE_ENTRIES * 2) :
+                                 (pixel_count * 2);
+  const auto data = GetSaveDataBytes(index, offset, total_bytes);
+  if (!data || data->size() != total_bytes)
+    return std::nullopt;
 
-  u32 DataOffset = GetActiveDirectory().m_dir_entries[index].m_image_offset;
+  std::vector<u32> rgba(pixel_count);
-  u32 DataBlock = GetActiveDirectory().m_dir_entries[index].m_first_block - MC_FST_BLOCKS;
+  if (format == MEMORY_CARD_BANNER_FORMAT_CI8)
-
-  if ((DataBlock > m_size_blocks) || (DataOffset == 0xFFFFFFFF))
   {
-    return false;
+    const u8* pxdata = data->data();
-  }
+    std::array<u16, MEMORY_CARD_CI8_PALETTE_ENTRIES> paldata;
-
+    std::memcpy(paldata.data(), data->data() + pixel_count, MEMORY_CARD_CI8_PALETTE_ENTRIES * 2);
-  const int pixels = 96 * 32;
+    Common::DecodeCI8Image(rgba.data(), pxdata, paldata.data(), MEMORY_CARD_BANNER_WIDTH,
-
+                           MEMORY_CARD_BANNER_HEIGHT);
-  if (bnrFormat & 1)
-  {
-    u8* pxdata = (u8*)(m_data_blocks[DataBlock].m_block.data() + DataOffset);
-    u16* paldata = (u16*)(m_data_blocks[DataBlock].m_block.data() + DataOffset + pixels);
-
-    Common::DecodeCI8Image(buffer, pxdata, paldata, 96, 32);
   }
   else
   {
-    u16* pxdata = (u16*)(m_data_blocks[DataBlock].m_block.data() + DataOffset);
+    std::array<u16, pixel_count> pxdata;
-
+    std::memcpy(pxdata.data(), data->data(), pixel_count * 2);
-    Common::Decode5A3Image(buffer, pxdata, 96, 32);
+    Common::Decode5A3Image(rgba.data(), pxdata.data(), MEMORY_CARD_BANNER_WIDTH,
+                           MEMORY_CARD_BANNER_HEIGHT);
   }
-  return true;
+
+  return rgba;
 }
 
 u32 GCMemcard::ReadAnimRGBA8(u8 index, u32* buffer, u8* delays) const

Source/Core/Core/HW/GCMemcard/GCMemcard.h

@@ -141,6 +141,10 @@ constexpr u16 MBIT_SIZE_MEMORY_CARD_2043 = 0x80;
 constexpr u32 MEMORY_CARD_BANNER_WIDTH = 96;
 constexpr u32 MEMORY_CARD_BANNER_HEIGHT = 32;
 
+// color format of banner as stored in the lowest two bits of m_banner_and_icon_flags
+constexpr u8 MEMORY_CARD_BANNER_FORMAT_CI8 = 1;
+constexpr u8 MEMORY_CARD_BANNER_FORMAT_RGB5A3 = 2;
+
 // width and height of a save file's icon in pixels
 constexpr u32 MEMORY_CARD_ICON_WIDTH = 32;
 constexpr u32 MEMORY_CARD_ICON_HEIGHT = 32;
@@ -148,6 +152,10 @@ constexpr u32 MEMORY_CARD_ICON_HEIGHT = 32;
 // maximum number of frames a save file's icon animation can have
 constexpr u32 MEMORY_CARD_ICON_ANIMATION_MAX_FRAMES = 8;
 
+// number of palette entries in a CI8 palette of a banner or icon
+// each palette entry is 16 bits in RGB5A3 format
+constexpr u32 MEMORY_CARD_CI8_PALETTE_ENTRIES = 256;
+
 class MemoryCardBase
 {
 public:
@@ -255,15 +263,13 @@ struct DEntry
   u8 m_unused_1;
 
   // 1 byte at 0x07: banner gfx format and icon animation (Image Key)
-  //      Bit(s)  Description
+  // First two bits are used for the banner format.
-  //      2       Icon Animation 0: forward 1: ping-pong
+  // YAGCD is wrong about the meaning of these.
-  //      1       [--0: No Banner 1: Banner present--] WRONG! YAGCD LIES!
+  // '0' and '3' both mean no banner.
-  //      0       [--Banner Color 0: RGB5A3 1: CI8--]  WRONG! YAGCD LIES!
+  // '1' means paletted (8 bits per pixel palette entry + 16 bit color palette in RGB5A3)
-  //      bits 0 and 1: image format
+  // '2' means direct color (16 bits per pixel in RGB5A3)
-  //      00 no banner
+  // Third bit is icon animation frame order, 0 for loop (abcabcabc), 1 for ping-pong (abcbabcba).
-  //      01 CI8 banner
+  // Remaining bits seem unused.
-  //      10 RGB5A3 banner
-  //      11 ? maybe ==00? Time Splitters 2 and 3 have it and don't have banner
   u8 m_banner_and_icon_flags;
 
   // 0x20 bytes at 0x08: Filename
@@ -498,7 +504,7 @@ public:
   static void Gcs_SavConvert(DEntry& tempDEntry, int saveType, u64 length = BLOCK_SIZE);
 
   // reads the banner image
-  bool ReadBannerRGBA8(u8 index, u32* buffer) const;
+  std::optional<std::vector<u32>> ReadBannerRGBA8(u8 index) const;
 
   // reads the animation frames
   u32 ReadAnimRGBA8(u8 index, u32* buffer, u8* delays) const;

Source/Core/DolphinQt/GCMemcardManager.cpp

@@ -467,12 +467,12 @@ QPixmap GCMemcardManager::GetBannerFromSaveFile(int file_index, int slot)
 {
   auto& memcard = m_slot_memcard[slot];
 
-  std::vector<u32> pxdata(MEMORY_CARD_BANNER_WIDTH * MEMORY_CARD_BANNER_HEIGHT);
+  auto pxdata = memcard->ReadBannerRGBA8(file_index);
 
   QImage image;
-  if (memcard->ReadBannerRGBA8(file_index, pxdata.data()))
+  if (pxdata)
   {
-    image = QImage(reinterpret_cast<u8*>(pxdata.data()), MEMORY_CARD_BANNER_WIDTH,
+    image = QImage(reinterpret_cast<u8*>(pxdata->data()), MEMORY_CARD_BANNER_WIDTH,
                    MEMORY_CARD_BANNER_HEIGHT, QImage::Format_ARGB32);
   }