CDROM: Add SubQ Skew option

Fixes corrupted boss sprites in Captain Commando.

One day I'll refactor things to fix this properly.

data/resources/gamedb.yaml

@@ -22079,11 +22079,13 @@ SLUS-01476:
 SLPS-01567:
   name: "Captain Commando (Japan)"
   compatibility:
-    rating: GraphicalAudioIssues
-    versionTested: "0.1-2433-g9089c973"
-    upscalingIssues: "There's garbage in right of the screen and the first boss sprite is corrupted."
+    rating: NoIssues
   controllers:
     - DigitalController
+  traits:
+    - DisablePGXP # 2D, PGXP is not beneficial.
+    - DisableWidescreen # GTE is not used, no effect.
+    - ForceCDROMSubQSkew # Fixes boss sprites.
   settings:
     displayActiveStartOffset: -62
     displayActiveEndOffset: 51

src/core/cdrom.cpp

@@ -3067,6 +3067,32 @@ void CDROM::DoSectorRead()
   if (subq_valid)
   {
     s_state.last_subq = subq;
+    if (g_settings.cdrom_subq_skew) [[unlikely]]
+    {
+      // SubQ Skew Hack. It's horrible. Needed for Captain Commando.
+      // Here's my previous rambling about the game:
+      //
+      //   So, there's two Getloc commands on the PS1 to retrieve the most-recent-read sector:
+      //   GetlocL, which returns the timecode based on the data sector header, and GetlocP, which gets it from subq.
+      //   Captain Commando would always corrupt the first boss sprite.
+      //
+      //   What the game does, is repeat the tile/texture data throughout the audio sectors for the background
+      //   music when you reach the boss part of the level, it looks for a specific subq timecode coming in (by spamming
+      //   GetlocP) then DMA's the data sector interleaved with the audio sectors out at the last possible moment
+      //
+      //   So, they hard coded it to look for a sector timecode +2 from the sector they actually wanted, then DMA that
+      //   data out they do perform some validation on the data itself, so if you're not offsetting the timecode query,
+      //   it never gets the right sector, and just keeps reading forever. Hence why the boss tiles are broken, because
+      //   it never gets the data to upload. The most insane part is they should have just done what every other game
+      //   does: use the raw read mode (2352 instead of 2048), and look at the data sector header. Instead they do this
+      //   nonsense of repeating the data throughout the audio, and racing the DMA at the last possible minute.
+      //
+      // This hack just generates synthetic SubQ with a +2 offset. I'd planned on refactoring the CDImage interface
+      // so that multiple sectors could be read in one back, in which case we could just "look ahead" to grab the
+      // subq, but I haven't got around to it. It'll break libcrypt, but CC doesn't use it. One day I'll get around to
+      // doing the refactor.... but given this is the only game that relies on it, priorities.
+      s_reader.GetMedia()->GenerateSubChannelQ(&s_state.last_subq, s_state.current_lba + 2);
+    }
   }
   else
   {
@@ -3745,7 +3771,7 @@ void CDROM::CreateFileMap(IsoReader& iso, std::string_view dir)
     {
       DEV_LOG("{}-{} = {}", entry.location_le, entry.location_le + entry.GetSizeInSectors() - 1, path);
       s_state.file_map.emplace(entry.location_le, std::make_pair(entry.location_le + entry.GetSizeInSectors() - 1,
-                                                           fmt::format("<DIR> {}", path)));
+                                                                 fmt::format("<DIR> {}", path)));
 
       CreateFileMap(iso, path);
       continue;
@@ -3753,7 +3779,7 @@ void CDROM::CreateFileMap(IsoReader& iso, std::string_view dir)
 
     DEV_LOG("{}-{} = {}", entry.location_le, entry.location_le + entry.GetSizeInSectors() - 1, path);
     s_state.file_map.emplace(entry.location_le,
-                       std::make_pair(entry.location_le + entry.GetSizeInSectors() - 1, std::move(path)));
+                             std::make_pair(entry.location_le + entry.GetSizeInSectors() - 1, std::move(path)));
   }
 }
 

src/core/game_database.cpp

@@ -40,7 +40,7 @@ namespace GameDatabase {
 enum : u32
 {
   GAME_DATABASE_CACHE_SIGNATURE = 0x45434C48,
-  GAME_DATABASE_CACHE_VERSION = 15,
+  GAME_DATABASE_CACHE_VERSION = 16,
 };
 
 static Entry* GetMutableEntry(std::string_view serial);
@@ -101,6 +101,7 @@ static constexpr const std::array<const char*, static_cast<u32>(GameDatabase::Tr
   "ForceRecompilerMemoryExceptions",
   "ForceRecompilerICache",
   "ForceRecompilerLUTFastmem",
+  "ForceCDROMSubQSkew",
   "IsLibCryptProtected",
 }};
 
@@ -130,6 +131,7 @@ static constexpr const std::array<const char*, static_cast<u32>(GameDatabase::Tr
   TRANSLATE_DISAMBIG_NOOP("GameDatabase", "Force Recompiler Memory Exceptions", "GameDatabase::Trait"),
   TRANSLATE_DISAMBIG_NOOP("GameDatabase", "Force Recompiler ICache", "GameDatabase::Trait"),
   TRANSLATE_DISAMBIG_NOOP("GameDatabase", "Force Recompiler LUT Fastmem", "GameDatabase::Trait"),
+  TRANSLATE_DISAMBIG_NOOP("GameDatabase", "Force CD-ROM SubQ Skew", "GameDatabase::Trait"),
   TRANSLATE_DISAMBIG_NOOP("GameDatabase", "Is LibCrypt Protected", "GameDatabase::Trait"),
 }};
 
@@ -610,6 +612,12 @@ void GameDatabase::Entry::ApplySettings(Settings& settings, bool display_osd_mes
     settings.cpu_fastmem_mode = CPUFastmemMode::LUT;
   }
 
+  if (HasTrait(Trait::ForceCDROMSubQSkew))
+  {
+    WARNING_LOG("CD-ROM SubQ Skew forced by compatibility settings.");
+    settings.cdrom_subq_skew = true;
+  }
+
   if (!messages.empty())
   {
     Host::AddIconOSDMessage(

src/core/game_database.h

@@ -53,6 +53,7 @@ enum class Trait : u32
   ForceRecompilerMemoryExceptions,
   ForceRecompilerICache,
   ForceRecompilerLUTFastmem,
+  ForceCDROMSubQSkew,
   IsLibCryptProtected,
 
   Count

src/core/settings.cpp

@@ -329,6 +329,7 @@ void Settings::Load(SettingsInterface& si, SettingsInterface& controller_si)
       si.GetStringValue("CDROM", "MechaconVersion", GetCDROMMechVersionName(DEFAULT_CDROM_MECHACON_VERSION)).c_str())
       .value_or(DEFAULT_CDROM_MECHACON_VERSION);
   cdrom_region_check = si.GetBoolValue("CDROM", "RegionCheck", false);
+  cdrom_subq_skew = si.GetBoolValue("CDROM", "SubQSkew", false);
   cdrom_load_image_to_ram = si.GetBoolValue("CDROM", "LoadImageToRAM", false);
   cdrom_load_image_patches = si.GetBoolValue("CDROM", "LoadImagePatches", false);
   cdrom_mute_cd_audio = si.GetBoolValue("CDROM", "MuteCDAudio", false);
@@ -615,6 +616,7 @@ void Settings::Save(SettingsInterface& si, bool ignore_base) const
   si.SetIntValue("CDROM", "ReadaheadSectors", cdrom_readahead_sectors);
   si.SetStringValue("CDROM", "MechaconVersion", GetCDROMMechVersionName(cdrom_mechacon_version));
   si.SetBoolValue("CDROM", "RegionCheck", cdrom_region_check);
+  si.SetBoolValue("CDROM", "SubQSkew", cdrom_subq_skew);
   si.SetBoolValue("CDROM", "LoadImageToRAM", cdrom_load_image_to_ram);
   si.SetBoolValue("CDROM", "LoadImagePatches", cdrom_load_image_patches);
   si.SetBoolValue("CDROM", "MuteCDAudio", cdrom_mute_cd_audio);

src/core/settings.h

@@ -180,6 +180,7 @@ struct Settings
   u8 cdrom_readahead_sectors = DEFAULT_CDROM_READAHEAD_SECTORS;
   CDROMMechaconVersion cdrom_mechacon_version = DEFAULT_CDROM_MECHACON_VERSION;
   bool cdrom_region_check : 1 = false;
+  bool cdrom_subq_skew : 1 = false;
   bool cdrom_load_image_to_ram : 1 = false;
   bool cdrom_load_image_patches : 1 = false;
   bool cdrom_mute_cd_audio : 1 = false;

src/core/system.cpp

@@ -4697,6 +4697,9 @@ void System::WarnAboutUnsafeSettings()
            TRANSLATE_STR("System", "Compatibility settings are not enabled. Some games may not function correctly."));
   }
 
+  if (g_settings.cdrom_subq_skew)
+    append(ICON_EMOJI_WARNING, TRANSLATE_SV("System", "CD-ROM SubQ Skew is enabled. This will break games."));
+
   if (!messages.empty())
   {
     if (messages.back() == '\n')

src/duckstation-qt/advancedsettingswidget.cpp

@@ -261,6 +261,7 @@ void AdvancedSettingsWidget::addTweakOptions()
                        Settings::GetCDROMMechVersionDisplayName, static_cast<u8>(CDROMMechaconVersion::Count),
                        Settings::DEFAULT_CDROM_MECHACON_VERSION);
   addBooleanTweakOption(m_dialog, m_ui.tweakOptionTable, tr("CD-ROM Region Check"), "CDROM", "RegionCheck", false);
+  addBooleanTweakOption(m_dialog, m_ui.tweakOptionTable, tr("CD-ROM SubQ Skew"), "CDROM", "SubQSkew", false);
   addBooleanTweakOption(m_dialog, m_ui.tweakOptionTable, tr("Allow Booting Without SBI File"), "CDROM",
                         "AllowBootingWithoutSBIFile", false);
 
@@ -297,6 +298,7 @@ void AdvancedSettingsWidget::onResetToDefaultClicked()
     setChoiceTweakOption(m_ui.tweakOptionTable, i++,
                          Settings::DEFAULT_CDROM_MECHACON_VERSION); // CDROM Mechacon Version
     setBooleanTweakOption(m_ui.tweakOptionTable, i++, false);       // CDROM Region Check
+    setBooleanTweakOption(m_ui.tweakOptionTable, i++, false);       // CDROM SubQ Skew
     setBooleanTweakOption(m_ui.tweakOptionTable, i++, false);       // Allow booting without SBI file
     setBooleanTweakOption(m_ui.tweakOptionTable, i++, false);       // Export Shared Memory
     setBooleanTweakOption(m_ui.tweakOptionTable, i++, false);       // Enable PCDRV
@@ -325,6 +327,7 @@ void AdvancedSettingsWidget::onResetToDefaultClicked()
   sif->DeleteValue("CPU", "FastmemMode");
   sif->DeleteValue("CDROM", "MechaconVersion");
   sif->DeleteValue("CDROM", "RegionCheck");
+  sif->DeleteValue("CDROM", "SubQSkew");
   sif->DeleteValue("CDROM", "AllowBootingWithoutSBIFile");
   sif->DeleteValue("PCDrv", "Enabled");
   sif->DeleteValue("PCDrv", "EnableWrites");

src/util/cd_image.cpp

@@ -456,7 +456,7 @@ void CDImage::CopyTOC(const CDImage* image)
   m_position_on_disc = 0;
 }
 
-const CDImage::Index* CDImage::GetIndexForDiscPosition(LBA pos)
+const CDImage::Index* CDImage::GetIndexForDiscPosition(LBA pos) const
 {
   for (const Index& index : m_indices)
   {
@@ -473,7 +473,7 @@ const CDImage::Index* CDImage::GetIndexForDiscPosition(LBA pos)
   return nullptr;
 }
 
-const CDImage::Index* CDImage::GetIndexForTrackPosition(u32 track_number, LBA track_pos)
+const CDImage::Index* CDImage::GetIndexForTrackPosition(u32 track_number, LBA track_pos) const
 {
   if (track_number < 1 || track_number > m_tracks.size())
     return nullptr;
@@ -485,18 +485,18 @@ const CDImage::Index* CDImage::GetIndexForTrackPosition(u32 track_number, LBA tr
   return GetIndexForDiscPosition(track.start_lba + track_pos);
 }
 
-bool CDImage::GenerateSubChannelQ(SubChannelQ* subq, LBA lba)
+bool CDImage::GenerateSubChannelQ(SubChannelQ* subq, LBA lba) const
 {
   const Index* index = GetIndexForDiscPosition(lba);
   if (!index)
     return false;
 
-  const u32 index_offset = index->start_lba_on_disc - lba;
+  const u32 index_offset = lba - index->start_lba_on_disc;
   GenerateSubChannelQ(subq, *index, index_offset);
   return true;
 }
 
-void CDImage::GenerateSubChannelQ(SubChannelQ* subq, const Index& index, u32 index_offset)
+void CDImage::GenerateSubChannelQ(SubChannelQ* subq, const Index& index, u32 index_offset) const
 {
   subq->control_bits = index.control.bits;
   subq->track_number_bcd = (index.track_number <= m_tracks.size() ? BinaryToBCD(static_cast<u8>(index.track_number)) :

src/util/cd_image.h

@@ -301,6 +301,12 @@ public:
   // Read a single raw sector, and subchannel from the current LBA.
   bool ReadRawSector(void* buffer, SubChannelQ* subq);
 
+  /// Generates sub-channel Q given the specified position.
+  bool GenerateSubChannelQ(SubChannelQ* subq, LBA lba) const;
+
+  /// Generates sub-channel Q from the given index and index-offset.
+  void GenerateSubChannelQ(SubChannelQ* subq, const Index& index, u32 index_offset) const;
+
   // Reads sub-channel Q for the specified index+LBA.
   virtual bool ReadSubChannelQ(SubChannelQ* subq, const Index& index, LBA lba_in_index);
 
@@ -340,14 +346,8 @@ protected:
   void ClearTOC();
   void CopyTOC(const CDImage* image);
 
-  const Index* GetIndexForDiscPosition(LBA pos);
-  const Index* GetIndexForTrackPosition(u32 track_number, LBA track_pos);
-
-  /// Generates sub-channel Q given the specified position.
-  bool GenerateSubChannelQ(SubChannelQ* subq, LBA lba);
-
-  /// Generates sub-channel Q from the given index and index-offset.
-  void GenerateSubChannelQ(SubChannelQ* subq, const Index& index, u32 index_offset);
+  const Index* GetIndexForDiscPosition(LBA pos) const;
+  const Index* GetIndexForTrackPosition(u32 track_number, LBA track_pos) const;
 
   /// Synthesis of lead-out data.
   void AddLeadOutIndex();