Merge pull request #5687 from shuffle2/wx-cache

DolphinWX: properly sync EmuState and banner changes.

6 files changed, 242 insertions, 170 deletions

diff --git a/Source/Core/DolphinWX/Frame.h b/Source/Core/DolphinWX/Frame.h
index 094df16173..f5d0b59e06 100644
--- a/Source/Core/DolphinWX/Frame.h
+++ b/Source/Core/DolphinWX/Frame.h
@@ -99,7 +99,7 @@ public:
   void DoStop();
   void UpdateGUI();
   void GameListRefresh();
-  void GameListRescan();
+  void GameListRescan(bool purge_cache = false);
   void ToggleLogWindow(bool bShow);
   void ToggleLogConfigWindow(bool bShow);
   void StatusBarMessage(const char* format, ...);
diff --git a/Source/Core/DolphinWX/FrameTools.cpp b/Source/Core/DolphinWX/FrameTools.cpp
index 5a798f70da..46acd65278 100644
--- a/Source/Core/DolphinWX/FrameTools.cpp
+++ b/Source/Core/DolphinWX/FrameTools.cpp
@@ -1616,10 +1616,11 @@ void CFrame::GameListRefresh()
   wxPostEvent(m_game_list_ctrl, event);
 }
 
-void CFrame::GameListRescan()
+void CFrame::GameListRescan(bool purge_cache)
 {
   wxCommandEvent event{DOLPHIN_EVT_RESCAN_GAMELIST, GetId()};
   event.SetEventObject(this);
+  event.SetInt(purge_cache ? 1 : 0);
   wxPostEvent(m_game_list_ctrl, event);
 }
 
@@ -1693,7 +1694,7 @@ void CFrame::GameListChanged(wxCommandEvent& event)
       File::Delete(filename);
     }
     // Do rescan after cache has been cleared
-    GameListRescan();
+    GameListRescan(true);
     return;
   }
 
diff --git a/Source/Core/DolphinWX/GameListCtrl.cpp b/Source/Core/DolphinWX/GameListCtrl.cpp
index 4e89a7051c..5b68857c01 100644
--- a/Source/Core/DolphinWX/GameListCtrl.cpp
+++ b/Source/Core/DolphinWX/GameListCtrl.cpp
@@ -269,27 +269,11 @@ GameListCtrl::GameListCtrl(bool disable_scanning, wxWindow* parent, const wxWind
       Common::SetCurrentThreadName("gamelist scanner");
 
       if (SyncCacheFile(false))
-      {
-        // Account for changes outside the cache (just wii banners atm; could scan Ini here)
-        // Note this is on the initial load path. Further improvements could be made if updates
-        // from scan->ui threads described small changelists instead of updating the entire list
-        // at once.
-        bool cache_modified = false;
-        for (auto& file : m_cached_files)
-        {
-          if (file->ReloadBannerIfNeeded())
-            cache_modified = true;
-        }
-
         QueueEvent(new wxCommandEvent(DOLPHIN_EVT_REFRESH_GAMELIST));
 
-        if (cache_modified)
-          SyncCacheFile(true);
-      }
-      else
-      {
-        RescanList();
-      }
+      // Always do an initial scan to catch new files and perform the more expensive per-file
+      // checks. TODO Make this safely cancellable if it becomes too slow?
+      RescanList();
 
       m_scan_trigger.Wait();
       while (!m_scan_exiting.IsSet())
@@ -421,20 +405,23 @@ void GameListCtrl::RefreshList()
     return;
 
   m_shown_files.clear();
-  for (auto& item : m_cached_files)
   {
-    if (ShouldDisplayGameListItem(*item))
-      m_shown_files.push_back(item);
+    std::unique_lock<std::mutex> lk(m_cache_mutex);
+    for (auto& item : m_cached_files)
+    {
+      if (ShouldDisplayGameListItem(*item))
+        m_shown_files.push_back(item);
+    }
   }
 
   // Drives are not cached. Not sure if this is required, but better to err on the
   // side of caution if cross-platform issues could come into play.
   if (SConfig::GetInstance().m_ListDrives)
   {
-    const Core::TitleDatabase title_database;
+    std::unique_lock<std::mutex> lk(m_title_database_mutex);
     for (const auto& drive : cdio_get_devices())
     {
-      auto file = std::make_shared<GameListItem>(drive, title_database);
+      auto file = std::make_shared<GameListItem>(drive, m_title_database);
       if (file->IsValid())
         m_shown_files.push_back(file);
     }
@@ -768,33 +755,75 @@ void GameListCtrl::RescanList()
   std::set_difference(search_results.cbegin(), search_results.cend(), cached_paths.cbegin(),
                       cached_paths.cend(), std::back_inserter(new_paths));
 
-  const Core::TitleDatabase title_database;
+  // Reload the TitleDatabase
+  {
+    std::unique_lock<std::mutex> lk(m_title_database_mutex);
+    m_title_database = {};
+  }
+
   // For now, only scan new_paths. This could cause false negatives (file actively being written),
   // but otherwise should be fine.
-  for (const auto& path : removed_paths)
+  bool cache_changed = false;
   {
-    auto it = std::find_if(
-        m_cached_files.cbegin(), m_cached_files.cend(),
-        [&path](const std::shared_ptr<GameListItem>& file) { return file->GetFileName() == path; });
-    if (it != m_cached_files.cend())
-      m_cached_files.erase(it);
+    std::unique_lock<std::mutex> lk(m_cache_mutex);
+    for (const auto& path : removed_paths)
+    {
+      auto it = std::find_if(m_cached_files.cbegin(), m_cached_files.cend(),
+                             [&path](const std::shared_ptr<GameListItem>& file) {
+                               return file->GetFileName() == path;
+                             });
+      if (it != m_cached_files.cend())
+      {
+        cache_changed = true;
+        m_cached_files.erase(it);
+      }
+    }
+    for (const auto& path : new_paths)
+    {
+      auto file = std::make_shared<GameListItem>(path, m_title_database);
+      if (file->IsValid())
+      {
+        cache_changed = true;
+        m_cached_files.push_back(std::move(file));
+      }
+    }
   }
-  for (const auto& path : new_paths)
+  // The common case is that just a file has been added/removed, so trigger a refresh ASAP with the
+  // assumption that other properties of files will not change at the same time (which will be fine
+  // and just causes a double refresh).
+  if (cache_changed)
+    QueueEvent(new wxCommandEvent(DOLPHIN_EVT_REFRESH_GAMELIST));
+
+  // If any cached files need updates, apply the updates to a copy and delete the original - this
+  // makes the UI thread's use of cached files safe. Note however, it is assumed that RefreshList
+  // will not iterate m_cached_files while the scan thread is modifying the list itself.
+  bool refresh_needed = false;
   {
-    auto file = std::make_shared<GameListItem>(path, title_database);
-    if (file->IsValid())
-      m_cached_files.push_back(std::move(file));
+    std::unique_lock<std::mutex> lk(m_cache_mutex);
+    for (auto& file : m_cached_files)
+    {
+      bool emu_state_changed = file->EmuStateChanged();
+      bool banner_changed = file->BannerChanged();
+      if (emu_state_changed || banner_changed)
+      {
+        cache_changed = refresh_needed = true;
+        auto copy = std::make_shared<GameListItem>(*file);
+        if (emu_state_changed)
+          copy->EmuStateCommit();
+        if (banner_changed)
+          copy->BannerCommit();
+        file = std::move(copy);
+      }
+    }
   }
+  // Only post UI event to update the displayed list if something actually changed
+  if (refresh_needed)
+    QueueEvent(new wxCommandEvent(DOLPHIN_EVT_REFRESH_GAMELIST));
 
-  for (auto& file : m_cached_files)
-    file->ReloadINI();
-
-  // Post UI event to update the displayed list
-  QueueEvent(new wxCommandEvent(DOLPHIN_EVT_REFRESH_GAMELIST));
+  post_status("");
 
-  post_status(_("Scan complete!"));
-
-  SyncCacheFile(true);
+  if (cache_changed)
+    SyncCacheFile(true);
 }
 
 void GameListCtrl::OnRefreshGameList(wxCommandEvent& WXUNUSED(event))
@@ -802,8 +831,14 @@ void GameListCtrl::OnRefreshGameList(wxCommandEvent& WXUNUSED(event))
   RefreshList();
 }
 
-void GameListCtrl::OnRescanGameList(wxCommandEvent& WXUNUSED(event))
+void GameListCtrl::OnRescanGameList(wxCommandEvent& event)
 {
+  if (event.GetInt())
+  {
+    // Knock out the cache on a purge event
+    std::unique_lock<std::mutex> lk(m_cache_mutex);
+    m_cached_files.clear();
+  }
   m_scan_trigger.Set();
 }
 
diff --git a/Source/Core/DolphinWX/GameListCtrl.h b/Source/Core/DolphinWX/GameListCtrl.h
index 2fd89cd9d5..6dbcf8aa3e 100644
--- a/Source/Core/DolphinWX/GameListCtrl.h
+++ b/Source/Core/DolphinWX/GameListCtrl.h
@@ -125,8 +125,12 @@ private:
   } m_image_indexes;
 
   // Actual backing GameListItems are maintained in a background thread and cached to file
-  static constexpr u32 CACHE_REVISION = 1;  // Last changed in PR 5680
+  static constexpr u32 CACHE_REVISION = 2;  // Last changed in PR 5687
   std::list<std::shared_ptr<GameListItem>> m_cached_files;
+  // Locks the list, not the contents
+  std::mutex m_cache_mutex;
+  Core::TitleDatabase m_title_database;
+  std::mutex m_title_database_mutex;
   std::thread m_scan_thread;
   Common::Event m_scan_trigger;
   Common::Flag m_scan_exiting;
diff --git a/Source/Core/DolphinWX/ISOFile.cpp b/Source/Core/DolphinWX/ISOFile.cpp
index 6d6b0a7d94..5f01889a29 100644
--- a/Source/Core/DolphinWX/ISOFile.cpp
+++ b/Source/Core/DolphinWX/ISOFile.cpp
@@ -61,17 +61,15 @@ static std::string GetLanguageString(DiscIO::Language language,
   return "";
 }
 
-GameListItem::GameListItem(const std::string& _rFileName, const Core::TitleDatabase& title_database)
-    : m_FileName(_rFileName), m_title_id(0), m_emu_state(0), m_FileSize(0),
-      m_region(DiscIO::Region::UNKNOWN_REGION), m_Country(DiscIO::Country::COUNTRY_UNKNOWN),
-      m_Revision(0), m_Valid(false), m_ImageWidth(0), m_ImageHeight(0), m_disc_number(0),
-      m_has_custom_name(false)
+GameListItem::GameListItem(const std::string& filename, const Core::TitleDatabase& title_database)
+    : m_file_name(filename), m_region(DiscIO::Region::UNKNOWN_REGION),
+      m_country(DiscIO::Country::COUNTRY_UNKNOWN)
 {
   {
-    std::unique_ptr<DiscIO::Volume> volume(DiscIO::CreateVolumeFromFilename(_rFileName));
+    std::unique_ptr<DiscIO::Volume> volume(DiscIO::CreateVolumeFromFilename(m_file_name));
     if (volume != nullptr)
     {
-      m_Platform = volume->GetVolumeType();
+      m_platform = volume->GetVolumeType();
 
       m_descriptions = volume->GetDescriptions();
       m_names = volume->GetLongNames();
@@ -82,20 +80,20 @@ GameListItem::GameListItem(const std::string& _rFileName, const Core::TitleDatab
         m_company = GetLanguageString(DiscIO::Language::LANGUAGE_ENGLISH, volume->GetShortMakers());
 
       m_region = volume->GetRegion();
-      m_Country = volume->GetCountry();
+      m_country = volume->GetCountry();
       m_blob_type = volume->GetBlobType();
-      m_FileSize = volume->GetRawSize();
-      m_VolumeSize = volume->GetSize();
+      m_file_size = volume->GetRawSize();
+      m_volume_size = volume->GetSize();
 
       m_game_id = volume->GetGameID();
       m_title_id = volume->GetTitleID().value_or(0);
       m_disc_number = volume->GetDiscNumber().value_or(0);
-      m_Revision = volume->GetRevision().value_or(0);
+      m_revision = volume->GetRevision().value_or(0);
 
-      std::vector<u32> buffer = volume->GetBanner(&m_ImageWidth, &m_ImageHeight);
-      ReadVolumeBanner(buffer, m_ImageWidth, m_ImageHeight);
+      std::vector<u32> buffer = volume->GetBanner(&m_banner.width, &m_banner.height);
+      ReadVolumeBanner(&m_banner.buffer, buffer, m_banner.width, m_banner.height);
 
-      m_Valid = true;
+      m_valid = true;
     }
   }
 
@@ -104,22 +102,21 @@ GameListItem::GameListItem(const std::string& _rFileName, const Core::TitleDatab
 
   if (IsValid())
   {
-    const auto type = m_Platform == DiscIO::Platform::WII_WAD ?
+    const auto type = m_platform == DiscIO::Platform::WII_WAD ?
                           Core::TitleDatabase::TitleType::Channel :
                           Core::TitleDatabase::TitleType::Other;
-    m_custom_name_titles_txt = title_database.GetTitleName(m_game_id, type);
-    ReloadINI();
+    m_custom_name = title_database.GetTitleName(m_game_id, type);
   }
 
   if (!IsValid() && IsElfOrDol())
   {
-    m_Valid = true;
-    m_FileSize = File::GetSize(_rFileName);
-    m_Platform = DiscIO::Platform::ELF_DOL;
+    m_valid = true;
+    m_file_size = File::GetSize(m_file_name);
+    m_platform = DiscIO::Platform::ELF_DOL;
     m_blob_type = DiscIO::BlobType::DIRECTORY;
 
     std::string path, name;
-    SplitPath(m_FileName, &path, &name, nullptr);
+    SplitPath(m_file_name, &path, &name, nullptr);
 
     // A bit like the Homebrew Channel icon, except there can be multiple files
     // in a folder with their own icons. Useful for those who don't want to have
@@ -135,88 +132,92 @@ GameListItem::GameListItem(const std::string& _rFileName, const Core::TitleDatab
   else
   {
     // Volume banner. Typical for everything that isn't a DOL or ELF.
-    SetWxBannerFromRaw();
+    SetWxBannerFromRaw(m_banner);
   }
 }
 
 bool GameListItem::IsValid() const
 {
-  if (!m_Valid)
+  if (!m_valid)
     return false;
 
-  if (m_Platform == DiscIO::Platform::WII_WAD && !IOS::ES::IsChannel(m_title_id))
+  if (m_platform == DiscIO::Platform::WII_WAD && !IOS::ES::IsChannel(m_title_id))
     return false;
 
   return true;
 }
 
-void GameListItem::ReloadINI()
+bool GameListItem::EmuStateChanged()
 {
-  if (!IsValid())
-    return;
+  IniFile ini = SConfig::LoadGameIni(m_game_id, m_revision);
+  ini.GetIfExists("EmuState", "EmulationStateId", &m_pending.emu_state.rating, 0);
+  ini.GetIfExists("EmuState", "EmulationIssues", &m_pending.emu_state.issues, std::string());
+  return m_emu_state != m_pending.emu_state;
+}
 
-  IniFile ini = SConfig::LoadGameIni(m_game_id, m_Revision);
-  ini.GetIfExists("EmuState", "EmulationStateId", &m_emu_state, 0);
-  ini.GetIfExists("EmuState", "EmulationIssues", &m_issues, std::string());
+void GameListItem::EmuStateCommit()
+{
+  m_emu_state = m_pending.emu_state;
+}
 
-  m_custom_name.clear();
-  m_has_custom_name = ini.GetIfExists("EmuState", "Title", &m_custom_name);
-  if (!m_has_custom_name && !m_custom_name_titles_txt.empty())
-  {
-    m_custom_name = m_custom_name_titles_txt;
-    m_has_custom_name = true;
-  }
+void GameListItem::EmuState::DoState(PointerWrap& p)
+{
+  p.Do(rating);
+  p.Do(issues);
+}
+
+void GameListItem::Banner::DoState(PointerWrap& p)
+{
+  p.Do(buffer);
+  p.Do(width);
+  p.Do(height);
 }
 
 void GameListItem::DoState(PointerWrap& p)
 {
-  p.Do(m_FileName);
+  p.Do(m_valid);
+  p.Do(m_file_name);
+  p.Do(m_file_size);
+  p.Do(m_volume_size);
   p.Do(m_names);
   p.Do(m_descriptions);
   p.Do(m_company);
   p.Do(m_game_id);
   p.Do(m_title_id);
-  p.Do(m_issues);
-  p.Do(m_emu_state);
-  p.Do(m_FileSize);
-  p.Do(m_VolumeSize);
   p.Do(m_region);
-  p.Do(m_Country);
-  p.Do(m_Platform);
+  p.Do(m_country);
+  p.Do(m_platform);
   p.Do(m_blob_type);
-  p.Do(m_Revision);
-  p.Do(m_pImage);
-  p.Do(m_Valid);
-  p.Do(m_ImageWidth);
-  p.Do(m_ImageHeight);
+  p.Do(m_revision);
   p.Do(m_disc_number);
-  p.Do(m_custom_name_titles_txt);
+  m_banner.DoState(p);
+  m_emu_state.DoState(p);
   p.Do(m_custom_name);
-  p.Do(m_has_custom_name);
   if (p.GetMode() == PointerWrap::MODE_READ)
   {
-    SetWxBannerFromRaw();
+    SetWxBannerFromRaw(m_banner);
   }
 }
 
 bool GameListItem::IsElfOrDol() const
 {
-  if (m_FileName.size() < 4)
+  if (m_file_name.size() < 4)
     return false;
 
-  std::string name_end = m_FileName.substr(m_FileName.size() - 4);
+  std::string name_end = m_file_name.substr(m_file_name.size() - 4);
   std::transform(name_end.begin(), name_end.end(), name_end.begin(), ::tolower);
   return name_end == ".elf" || name_end == ".dol";
 }
 
-void GameListItem::ReadVolumeBanner(const std::vector<u32>& buffer, int width, int height)
+void GameListItem::ReadVolumeBanner(std::vector<u8>* image, const std::vector<u32>& buffer,
+                                    int width, int height)
 {
-  m_pImage.resize(width * height * 3);
+  image->resize(width * height * 3);
   for (int i = 0; i < width * height; i++)
   {
-    m_pImage[i * 3 + 0] = (buffer[i] & 0xFF0000) >> 16;
-    m_pImage[i * 3 + 1] = (buffer[i] & 0x00FF00) >> 8;
-    m_pImage[i * 3 + 2] = (buffer[i] & 0x0000FF) >> 0;
+    (*image)[i * 3 + 0] = (buffer[i] & 0xFF0000) >> 16;
+    (*image)[i * 3 + 1] = (buffer[i] & 0x00FF00) >> 8;
+    (*image)[i * 3 + 2] = (buffer[i] & 0x0000FF) >> 0;
   }
 }
 
@@ -229,41 +230,49 @@ bool GameListItem::SetWxBannerFromPngFile(const std::string& path)
   if (!image.IsOk())
     return false;
 
-  m_image = image;
+  m_banner_wx = image;
   return true;
 }
 
-void GameListItem::SetWxBannerFromRaw()
+void GameListItem::SetWxBannerFromRaw(const Banner& banner)
 {
   // Need to make explicit copy as wxImage uses reference counting for copies combined with only
   // taking a pointer, not the content, when given a buffer to its constructor.
-  if (!m_pImage.empty())
+  if (!banner.empty())
   {
-    m_image.Create(m_ImageWidth, m_ImageHeight, false);
-    std::memcpy(m_image.GetData(), m_pImage.data(), m_pImage.size());
+    m_banner_wx.Create(banner.width, banner.height, false);
+    std::memcpy(m_banner_wx.GetData(), banner.buffer.data(), banner.buffer.size());
   }
 }
 
-bool GameListItem::ReloadBannerIfNeeded()
+bool GameListItem::BannerChanged()
 {
   // Wii banners can only be read if there is a savefile,
   // so sometimes caches don't contain banners. Let's check
   // if a banner has become available after the cache was made.
-  if ((m_Platform == DiscIO::Platform::WII_DISC || m_Platform == DiscIO::Platform::WII_WAD) &&
-      m_pImage.empty())
+  if ((m_platform == DiscIO::Platform::WII_DISC || m_platform == DiscIO::Platform::WII_WAD) &&
+      m_banner.empty())
   {
+    auto& banner = m_pending.banner;
     std::vector<u32> buffer =
-        DiscIO::Volume::GetWiiBanner(&m_ImageWidth, &m_ImageHeight, m_title_id);
+        DiscIO::Volume::GetWiiBanner(&banner.width, &banner.height, m_title_id);
     if (buffer.size())
     {
-      ReadVolumeBanner(buffer, m_ImageWidth, m_ImageHeight);
-      SetWxBannerFromRaw();
+      ReadVolumeBanner(&banner.buffer, buffer, banner.width, banner.height);
+      // Only reach here if m_banner was empty, so don't need to explicitly compare to see if
+      // different
       return true;
     }
   }
   return false;
 }
 
+void GameListItem::BannerCommit()
+{
+  m_banner = std::move(m_pending.banner);
+  SetWxBannerFromRaw(m_banner);
+}
+
 std::string GameListItem::GetDescription(DiscIO::Language language) const
 {
   return GetLanguageString(language, m_descriptions);
@@ -271,7 +280,7 @@ std::string GameListItem::GetDescription(DiscIO::Language language) const
 
 std::string GameListItem::GetDescription() const
 {
-  bool wii = m_Platform != DiscIO::Platform::GAMECUBE_DISC;
+  bool wii = m_platform != DiscIO::Platform::GAMECUBE_DISC;
   return GetDescription(SConfig::GetInstance().GetCurrentLanguage(wii));
 }
 
@@ -282,10 +291,10 @@ std::string GameListItem::GetName(DiscIO::Language language) const
 
 std::string GameListItem::GetName() const
 {
-  if (m_has_custom_name)
+  if (!m_custom_name.empty())
     return m_custom_name;
 
-  bool wii = m_Platform != DiscIO::Platform::GAMECUBE_DISC;
+  bool wii = m_platform != DiscIO::Platform::GAMECUBE_DISC;
   std::string name = GetName(SConfig::GetInstance().GetCurrentLanguage(wii));
   if (!name.empty())
     return name;
@@ -341,7 +350,7 @@ std::vector<DiscIO::Language> GameListItem::GetLanguages() const
 
 const std::string GameListItem::GetWiiFSPath() const
 {
-  if (m_Platform != DiscIO::Platform::WII_DISC && m_Platform != DiscIO::Platform::WII_WAD)
+  if (m_platform != DiscIO::Platform::WII_DISC && m_platform != DiscIO::Platform::WII_WAD)
     return "";
 
   const std::string path = Common::GetTitleDataPath(m_title_id, Common::FROM_CONFIGURED_ROOT);
diff --git a/Source/Core/DolphinWX/ISOFile.h b/Source/Core/DolphinWX/ISOFile.h
index 84f70c050b..a7b66c5e8e 100644
--- a/Source/Core/DolphinWX/ISOFile.h
+++ b/Source/Core/DolphinWX/ISOFile.h
@@ -36,11 +36,8 @@ public:
   GameListItem(const std::string& file_name, const Core::TitleDatabase& title_database);
   ~GameListItem() = default;
 
-  // Reload settings after INI changes
-  void ReloadINI();
-
   bool IsValid() const;
-  const std::string& GetFileName() const { return m_FileName; }
+  const std::string& GetFileName() const { return m_file_name; }
   std::string GetName(DiscIO::Language language) const;
   std::string GetName() const;
   std::string GetUniqueIdentifier() const;
@@ -48,66 +45,92 @@ public:
   std::string GetDescription() const;
   std::vector<DiscIO::Language> GetLanguages() const;
   std::string GetCompany() const { return m_company; }
-  u16 GetRevision() const { return m_Revision; }
+  u16 GetRevision() const { return m_revision; }
   const std::string& GetGameID() const { return m_game_id; }
   u64 GetTitleID() const { return m_title_id; }
   const std::string GetWiiFSPath() const;
   DiscIO::Region GetRegion() const { return m_region; }
-  DiscIO::Country GetCountry() const { return m_Country; }
-  DiscIO::Platform GetPlatform() const { return m_Platform; }
+  DiscIO::Country GetCountry() const { return m_country; }
+  DiscIO::Platform GetPlatform() const { return m_platform; }
   DiscIO::BlobType GetBlobType() const { return m_blob_type; }
-  const std::string& GetIssues() const { return m_issues; }
-  int GetEmuState() const { return m_emu_state; }
-  u64 GetFileSize() const { return m_FileSize; }
-  u64 GetVolumeSize() const { return m_VolumeSize; }
+  const std::string& GetIssues() const { return m_emu_state.issues; }
+  int GetEmuState() const { return m_emu_state.rating; }
+  u64 GetFileSize() const { return m_file_size; }
+  u64 GetVolumeSize() const { return m_volume_size; }
   // 0 is the first disc, 1 is the second disc
   u8 GetDiscNumber() const { return m_disc_number; }
   // NOTE: Banner image is at the original resolution, use WxUtils::ScaleImageToBitmap
   //   to display it
-  const wxImage& GetBannerImage() const { return m_image; }
+  const wxImage& GetBannerImage() const { return m_banner_wx; }
   void DoState(PointerWrap& p);
-  bool ReloadBannerIfNeeded();
+  bool BannerChanged();
+  void BannerCommit();
+  bool EmuStateChanged();
+  void EmuStateCommit();
 
 private:
+  struct EmuState
+  {
+    int rating{};
+    std::string issues{};
+    bool operator!=(const EmuState& rhs) const
+    {
+      return rating != rhs.rating || issues != rhs.issues;
+    }
+    void DoState(PointerWrap& p);
+  };
+  struct Banner
+  {
+    std::vector<u8> buffer{};
+    int width{};
+    int height{};
+    bool empty() const { return buffer.empty(); }
+    void DoState(PointerWrap& p);
+  };
+
   bool IsElfOrDol() const;
-  // Outputs to m_pImage
-  void ReadVolumeBanner(const std::vector<u32>& buffer, int width, int height);
-  // Outputs to m_image
+  void ReadVolumeBanner(std::vector<u8>* image, const std::vector<u32>& buffer, int width,
+                        int height);
+  // Outputs to m_banner_wx
   bool SetWxBannerFromPngFile(const std::string& path);
-  void SetWxBannerFromRaw();
+  void SetWxBannerFromRaw(const Banner& banner);
 
-  // IMPORTANT: All data members must be save/restored in DoState.
+  // IMPORTANT: Nearly all data members must be save/restored in DoState.
   // If anything is changed, make sure DoState handles it properly and
   // GameListCtrl::CACHE_REVISION is incremented.
 
-  std::string m_FileName;
-
-  std::map<DiscIO::Language, std::string> m_names;
-  std::map<DiscIO::Language, std::string> m_descriptions;
-  std::string m_company;
-
-  std::string m_game_id;
-  u64 m_title_id = 0;
-
-  std::string m_issues;
-  int m_emu_state;
-
-  u64 m_FileSize;
-  u64 m_VolumeSize;
-
-  DiscIO::Region m_region;
-  DiscIO::Country m_Country;
-  DiscIO::Platform m_Platform;
-  DiscIO::BlobType m_blob_type;
-  u16 m_Revision;
-
-  wxImage m_image;
-  bool m_Valid;
-  std::vector<u8> m_pImage;
-  int m_ImageWidth, m_ImageHeight;
-  u8 m_disc_number;
-
-  std::string m_custom_name_titles_txt;  // Custom title from titles.txt
-  std::string m_custom_name;             // Custom title from INI or titles.txt
-  bool m_has_custom_name;
+  bool m_valid{};
+  std::string m_file_name{};
+
+  u64 m_file_size{};
+  u64 m_volume_size{};
+
+  std::map<DiscIO::Language, std::string> m_names{};
+  std::map<DiscIO::Language, std::string> m_descriptions{};
+  std::string m_company{};
+  std::string m_game_id{};
+  u64 m_title_id{};
+
+  DiscIO::Region m_region{};
+  DiscIO::Country m_country{};
+  DiscIO::Platform m_platform{};
+  DiscIO::BlobType m_blob_type{};
+  u16 m_revision{};
+  u8 m_disc_number{};
+
+  Banner m_banner{};
+  EmuState m_emu_state{};
+  // Overridden name from TitleDatabase
+  std::string m_custom_name{};
+
+  // wxImage is not handled in DoState
+  wxImage m_banner_wx{};
+
+  // The following data members allow GameListCtrl to construct new GameListItems in a threadsafe
+  // way. They should not be handled in DoState.
+  struct
+  {
+    EmuState emu_state;
+    Banner banner;
+  } m_pending{};
 };