Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VideoCommon: Fix std::filesystem::path encoding conversion #12112

Merged
merged 1 commit into from Aug 16, 2023

Conversation

JosJuice
Copy link
Member

In std::string, you can store strings using any encoding, but in Dolphin we have decided to use UTF-8. The problem is that if you convert between std::string and std::filesystem::path using the built-in methods, the standard library will make up its own assumption of what encoding you're using in the std::string. On most OSes this is UTF-8, but on Windows it's whatever the user's code page is.

What I believe is the C++ standard authors' intended solution to this is to use std::u8string instead of std::string, but that's a big hassle to move over to, because there's no convenient way to convert between std::string and std::u8string. Instead, in Dolphin, we have added helper functions that convert between std::string and std::filesystem::path in the manner we want. You always have to use these when converting between std::string and std::filesystem::path, otherwise we get these kinds of encoding problems that we've been having with custom textures.

Fixes https://bugs.dolphin-emu.org/issues/13328.

In std::string, you can store strings using any encoding, but in Dolphin
we have decided to use UTF-8. The problem is that if you convert between
std::string and std::filesystem::path using the built-in methods, the
standard library will make up its own assumption of what encoding you're
using in the std::string. On most OSes this is UTF-8, but on Windows
it's whatever the user's code page is.

What I believe is the C++ standard authors' intended solution to this is
to use std::u8string instead of std::string, but that's a big hassle to
move over to, because there's no convenient way to convert between
std::string and std::u8string. Instead, in Dolphin, we have added helper
functions that convert between std::string and std::filesystem::path in
the manner we want. You *always* have to use these when converting
between std::string and std::filesystem::path, otherwise we get these
kinds of encoding problems that we've been having with custom textures.

Fixes https://bugs.dolphin-emu.org/issues/13328.
@iwubcode
Copy link
Contributor

Thanks @JosJuice ! My original thought was to update Dolphin to actually take paths in places where it currently takes strings. This would be changing FileToString, GetTextureDirectoriesWithGameId, DoFileSearch, etc to use or return filesystem::path instead because that is cross-platform automatically. This avoids all these conversions.

However, this is admittedly simpler. I always forget about PathToString since it's not standard despite having used it a few times. I'll have to go update other places I am doing in current and future reviews.

@AdmiralCurtiss AdmiralCurtiss merged commit 23ae8c4 into dolphin-emu:master Aug 16, 2023
11 checks passed
@JosJuice JosJuice deleted the hires-texture-encoding branch August 16, 2023 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants