From 9883235c0c67185d9cdc9e739be8ef25532378a8 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Tue, 18 Nov 2025 23:17:03 +0100 Subject: [PATCH] Common: Treat DirectIOFile as unable to create SAF files In 405baed805, we made the assumption that whether OpenAndroidContent is able to create a new file depends on the open mode, the document provider, and the positions of the celestial bodies. However, I'm fairly sure that it can't create files at all as currently implemented. First, ContentResolver.openFileDescriptor is documented as throwing FileNotFoundException if the file doesn't exist. Now, the SAF documentation is notoriously unreliable on matters like these, and document providers can do whatever they want anyway, so we can't actually trust this to mean that FileNotFoundException will always be thrown if the file doesn't exist. But second, the Dolphin function ContentHandler.unmangle is also unable to handle files that don't already exist (unless the passed-in URI isn't mangled to begin with, but to the best of my knowledge, there's no way to get a non-mangled URI for a file that doesn't exist (unless you make assumptions about how the document provider's URI scheme works, which we don't do in Dolphin)). Summed up, it looks a lot like OpenAndroidContent can't create files, or at the very least can't do so reliably. Therefore I'm making DirectIOFile throw an assertion and return false in situations where it's being asked to create a file under SAF. For reference, there's no code in Dolphin that actually tries to create a file under SAF. In all instances where we use SAF to write to files, the file is created by the system file picker before it returns the URI of the file to us. What does this change gain us? First, if someone in the future tries to use DirectIOFile to create a file under SAF, they'll be immediately informed that this isn't supported rather than discovering it doesn't work and chalking it up to SAF being bad for unpredictable reasons. Second, we get rid of a call to File::Exists, which is a notable performance improvement for game list scanning due to SAF and Dolphin's "unmangling" being bad for reasons that unfortunately are entirely predictable to those of us who have stared into the SAF void. I've tested that game list scanning and game conversion still works. --- Source/Core/Common/DirectIOFile.cpp | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/Source/Core/Common/DirectIOFile.cpp b/Source/Core/Common/DirectIOFile.cpp index d5e805529de..d72e98fa3bb 100644 --- a/Source/Core/Common/DirectIOFile.cpp +++ b/Source/Core/Common/DirectIOFile.cpp @@ -121,29 +121,20 @@ bool DirectIOFile::Open(const std::string& path, AccessMode access_mode, OpenMod else if (access_mode == AccessMode::Write) open_mode_str = "w"; - // FYI: File::Exists can be slow on Android. - Common::Lazy file_exists{[&] { return Exists(path); }}; + if (open_mode == OpenMode::Truncate) + open_mode_str += 't'; - // A few features are emulated in a non-atomic manner. - if (open_mode == OpenMode::Existing) + if (open_mode == OpenMode::Create) { - if (access_mode != AccessMode::Read && !*file_exists) - return false; - } - else - { - if (open_mode == OpenMode::Truncate) - open_mode_str += 't'; - else if (open_mode == OpenMode::Create && *file_exists) - return false; - - // Modes other than `Existing` may create a file, but "r" won't do that automatically. - if (access_mode == AccessMode::Read && !*file_exists) - CreateEmptyFile(path); + ASSERT_MSG(COMMON, false, "DirectIOFile doesn't support creating SAF files"); + return false; } m_fd = OpenAndroidContent(path, open_mode_str); + if (!IsOpen() && (open_mode == OpenMode::Always || open_mode == OpenMode::Truncate)) + ASSERT_MSG(COMMON, Exists(path), "DirectIOFile doesn't support creating SAF files"); + return IsOpen(); } #endif