of::filesystem::path with implicit narrow string conversion

[artificiel]

BACKGROUND

We want to support std::filesystem at is provides many cross-platform filesystem functions, including std::filesystem::path which is a representation of a thing on a filesystem (including knowing intelligently about it’s root drive (ex: C:) and the native separator, if it’s a directory or a file, permissions, etc). Being standardized means the responsability of dealing with the idiosyncrasies of diifferent OSes and filesystems is transferred to the compiler vendors.

one great feature of std::filesystem::path is the conversion support with std::(w)string. combined with the constructor taking std::(w)string as an origin path, it means they can be used interchangeably in most «in» and «out» operations: you can pass a string to a method expecting a path (and it will get constructed in place) or you can pass a path to a method expecting a string, and it will get converted by it’s operator, essentially calling path.string(). almost awesome.

std::string s = std::filesystem::path("/etc/passwd"); // works!
std::filesystem::path p = std::string("/etc/shadow"); // works!
// etc

PROBLEM

the standard stipulates that the conversion behaviour be done for the platform-native char type, which means std::string on most systems, but std::wstring on windows. so what’s written above is not totally true: on windows the std::string conversion operator is not implemented… meaning the above code does not compile. it should be written as

std::string  s = std::filesystem::path("/etc/passed"); // no compile
std::wstring s = std::filesystem::path("/etc/passed"); // works!

this is because on windows, handling file paths as narrow std::string is a blasphemy that makes it impossible to support the exotic wide unicode that might be present in a windows file path. however little to no OF code supports wstring.

the «solution» OF is providing is ofPathToString() which forces-converts and catches the exceptions, but that requires adding calls to ofPathToString(), and if you are doing that (which is just hiding the blasphemy) you might as well upgrade to support std::filesystem::path to get proper native path support.

so what is really needed is to add the conversion to std::string on msvc, essentially adding a line around here:

https://github.com/microsoft/STL/blob/0d8f517ae3828284fed594741b847db940167a59/stl/inc/filesystem#L921-L923

OBJECTIVE

we want to tolerate the blasphemy of converting wide paths to narrow paths because a lot of historical «cross-platform» OF code carries paths inside std:::string.

this must be considered as a stopgap «transparent» measure, and anywhere std::string is used to contain a path, it should be changed to std::filesystem::path.

SOLUTION

0. microsoft has no interest in deviating from the standard in order to support our buggy cross-plaform usage (properly written win32 string-as-path cases are using wstring/wchar and switches between wide and narrow interface calls, often with #ifdef WIN32).

1. it is repeated all over the place that the stdlib classes are not designed to be subclassed, so the obvious solution of doing:

class of::filesystem::path: public std::filesystem::path {};

is an upwards battle against the lib, which is discouraged.

2. contrary to other languages, C++ does not allow the refinement of classes after they are declared (for instance Objective-C has Categories, which allow you to graft methods and properties to existing classes dynamically at runtime). so there is no mecanism to specialize std::filesystem::path «in place».

3. the solution is to make a new class that composes a std::filesystem::path and implements all its interface, forwarding everything to and from the internal member:

of::filesystem::path {
	std::filesystem::path path_
public:
	// all methods of std::filesystem::path forwarding/returning with this->path_
	// PLUS
	operator std::string() { return path_.string(); }
}

which is basically this PR.

IMPACT ON EXISTING CODE

for historical reasons OF aliases std::filesystem as of::filesystem, but that reason does not hold anymore. moreover if we want to specialize of::filesystem::path we cannot «overwrite» it if it’s been imported from std::filesystem::path, nor can we selectively import std::filesystem::everyhing_but_path and sneakily slide our own impl in the path gap.

the approach taken is to stop importing std::filesystem (which is pointless anyway) meaning the core calls like of::filesystems::exists() must be corrected to std::filesystem::exists() (which has no side effect as it’s already what’s happening, and they’re easy to find as compilation will not work with them).

then in ofConstants.h:

#if defined(TARGET_WIN32)
	#include "ofFilesystemPath.h"
#else
	namespace of::filesystem { using path = std::filesystem::path; }
#endif

which means, for windows, the string conversion is now defined and other platforms behave exactly as they currently are (because they are already using std::filesystem::path even if it’s labelled of::filesystem::path).

there are no regression risks on the new functionality as currently none of this is possible. there is a small risk of not handling correctly all possible windows path uses, which can be corrected as we move forward and get feedback from windows people throwing their funny things into of::filesystem::path.

as was discussed in #8138 there is a risk of virtual interfaces being thrown off in derived classes not supporting of::filesystem::path; the solution for backwards-compatibility is to maintain but deprecate the std::string interface and define impure (not-required) of::filesystem::paths methods.

finally, there are edge cases where conversion does not kick such as expressions (presuming one wants to transduce a path to a string in order to display a filename):

std::string str {«filename: «};
str += of::filesystem("/etc/passwd"); // will not work
str += ofPathToString(of::filesystem("/etc/passwd")); // OK

(these cases such as ofDrawBitmapString() should support std::wstring (to display the funky unicode), or more probably be upgraded to std::u8string, new kid on the block (C++20), but it's outside the scope of the general mechanism proposed here, which can be expanded later)

also, passing paths as strings in vectors std::vector<std::string> my_files; — or other indirect typing will not get the conversion treatment, so these edge cases will need fixing.

QUESTIONS

• should the try-catch of exception (due to rare incompatible exotic unicode) in narrowing be handled internally by of::filesystem::path? I have conflicted opinions: spontaneously, yes, to streamline things for users; but it introduces a magic value (return empty string to mean failure) a pattern which should be avoided. this PR keeps it clean (no try-catch to hide the blasphemous behaviour) with the idea that putting it to test will show what it reveals? in theory, the cases triggering that exception (such as throwing an asian path to ofImage) would already occur in current OF, and it might be better to identify these cases and fix the root causes? (and anyhow the end user will be out of luck as the path will be empty).

• the PR implements a const char* to_narrow_cstr() const method defined around a mutable std::string cached_narrow_str_;. it's assembled from internet sources and seems to work, but it would be good to have criticism on the strategy and implementation. (this method is required to support w_char-to-char conversion (we talk mostly about strings but everything also applies to char *))

[artificiel]

ha! ok well at this point we're in unchartered territory... especially pasted text, it may have underwent UTF-8 conversion without knowing how MSVC's editor processes the encoding... maybe try an image load from a method that receives a live existing path from a WIN API? that would ensure it's actually wide. also ofImage looks like it's ready to correctly process wide paths down to the FreeImage_*U calls, but you're the first one trying it via paths...

so considering you're not crashing with the normal stuff and that you can assign a path to std::string, we can consider this PR ready to be merged, but I'd still wait post 12.1, both to ensure a thorough cleanup around the -FS calls, and allowing a bit of time for git-users to throw things at it.

[dimitre]

One good thing to test are filenames with cyrillic characters.
I remember they caused a kernel panic in this PR
#7435

[NickHardeman]

Testing with an image from this discussion: (#7435) ( also the same output with the nightly )

std::string str = "ExifTool_filename_tests/XW_₨₩₤ЖЯШΨΩΔՊՎ.jpg";
std::filesystem::path tempPath{ str };
ofLogNotice("Trying to load image from") << tempPath << "  : consolação.jpg" ;
std::wcout << "cout wide tempPath: " << tempPath.wstring() << std::endl;
bool bok = ofLoadImage(tempTex, tempPath);
ofLogNotice("Load ok") << bok;

bool bok2 = ofLoadImage(tempTex, "consolação.jpg");
ofLogNotice("Load ok2") << bok2;

Outputs:

[notice ] Trying to load image from: "ExifTool_filename_tests/XW_???????????.jpg" ]]]] : consolação.jpg
[notice ] Load ok: 0
[notice ] Load ok2: 1

The following causes a crash:

std::wstring str = L"ExifTool_filename_tests/XW_₨₩₤ЖЯШΨΩΔՊՎ.jpg";
std::filesystem::path tempPath{ str };
ofLogNotice("Trying to load image from") << tempPath << " ]]]] : consolação.jpg" ;// .u8string();
std::wcout << "cout wide tempPath: " << tempPath.wstring() << std::endl;

Crashes in ofLog(),

openFrameworks/libs/openFrameworks/utils/ofLog.h

Lines 507 to 511 in 45c3458

	template <class T>
	ofLog & operator<<(const T & value) {
	message << value << getPadding();
	return *this;
	}

[artificiel]

@NickHardeman good to know that does not crash!

@dimitre was your panic from a test typing "someunicodestring🫥" in source code, or from getting an actual filename from WIN? in any case getting your triggering data would be good.

[dimitre]

loading an actual file, Probably one from here,

https://github.com/dimitre/ofTests/tree/main/WinEncoding/bin/data
canapés-спациба.jpg

[artificiel]

@NickHardeman can you test @dimitre's file? so we have a good data point vs previous behaviour.

at this point we must consider the problem of unicode in general in OF separated from "convenient widepath-to-string conversion on windows" (the goal of this PR, which has been confirmed to be attained).

ultimately we should be able to trigger exceptions by generating broken unicode to clarify how it behaves with normal std:filesystem::path, for instance on macOS14 the below non-OF code throws an exception: locale not supported so we don't need to be in "exotic windows-only wide unicode territory" to have difficulties with unicode.

	   try {
		   std::wstring ws = std::wstring{L"\xD800\xDC00"};
		   std::filesystem::path p{ws};
		   std::string str = p.string();
		   std::cout << "Path string: " << str << std::endl;
	   }
	   catch (const std::exception& e) {
		   std::cerr << "Caught exception: " << e.what() << std::endl;
	   }

[ofTheo]

@NickHardeman I think it would be worth testing all the examples in
examples/input_output

Both with the nightly and with @artificiel's PR applied.

Also even something simple like:
std::string myPath = ofToDataPath(“blah.txt”);

As I can imagine there are a lot of projects and addons that might do something like that?

I am guessing that does break with the current nightly and would be fixed by this PR

And thanks for doing all this testing!!

[artificiel]

@ofTheo I think you need an L to ensure the string is wide: std::string myPath = ofToDataPath(L“blah.txt”);

but we know it works because this PR includes a test (line 189 would fail on the nightly):

openFrameworks/tests/utils/fileUtils/src/main.cpp

Lines 189 to 190 in 8911f00

	std::string narrow = ofToDataPath("");
	ofxTest(std::filesystem::exists(narrow), "narrow paths on windows");

why I'm suggesting not merging this now is not for the base function, but the more expanded usage (like the virtual interface problem that was raised when this topic was active in October, and certainly other edge cases like vectors).

and to be clear the snippet in my post just above will fail everywhere independently of OF as the escaped sequence is not convertible, so it's important to be specific in the tests and expectations of this PR as of::filesystem::path will not solve what the underlying std::filesystem::path cannot — it just provides implicit access on windows to the conversion.

[NickHardeman]

@NickHardeman can you test @dimitre's file? so we have a good data point vs previous behaviour.

at this point we must consider the problem of unicode in general in OF separated from "convenient widepath-to-string conversion on windows" (the goal of this PR, which has been confirmed to be attained).

ultimately we should be able to trigger exceptions by generating broken unicode to clarify how it behaves with normal std:filesystem::path, for instance on macOS14 the below non-OF code throws an exception: locale not supported so we don't need to be in "exotic windows-only wide unicode territory" to have difficulties with unicode.

	   try {
		   std::wstring ws = std::wstring{L"\xD800\xDC00"};
		   std::filesystem::path p{ws};
		   std::string str = p.string();
		   std::cout << "Path string: " << str << std::endl;
	   }
	   catch (const std::exception& e) {
		   std::cerr << "Caught exception: " << e.what() << std::endl;
	   }

Testing the above in @artificiel's branch and the nightly outputs:
Caught exception: No mapping for the Unicode character exists in the target multi-byte code page.

Testing the following

ofTexture tempTex;
bool bok = ofLoadImage(tempTex, "canapés-спациба.jpg");
ofLogNotice("image loaded") << bok;

In this branch
[notice ] image loaded: 0

In the nightly:
[notice ] image loaded: 0

std::filesystem::path p(L"canapés-спациба.jpg");
bool bok = ofLoadImage(tempTex, p);
ofLogNotice("image loaded") << bok;

This branch:
Crash

Nightly:
Crash

When I change the following in ofFileUtils.h in this branch, it avoids the crash at least and the image does not load.

std::string ofPathToString(const of::filesystem::path & path) {
	try {
		return path.string();
	} catch(std::filesystem::filesystem_error & e) {
		ofLogError("ofFileUtils") << "ofPathToString: error converting fs::path to string " << e.what();
	} catch (std::system_error& e) {
		// adding this additional catch
		ofLogError("ofFileUtils") << "ofPathToString: error converting fs::path to string " << e.what();
	}
	return {};
}

and more catches in

//--------------------------------------------------
of::filesystem::path ofToDataPathFS(const of::filesystem::path & in_path, bool makeAbsolute){
	of::filesystem::path path {in_path} ; // don't propagate constness
	if (makeAbsolute && path.is_absolute()) {
		return path;
	}

	if (!enableDataPath) {
		return path;
	}
	// **************************************************************************** //
	// added checks here to see if the path is valid, if not, then return
	
	try {
		auto testString = path.string();
	} catch (std::filesystem::filesystem_error& e) {
		ofLogError("ofFileUtils") << "ofToDataPathFS: error converting fs::path to string " << e.what();
		return path;
	} catch (std::system_error& e) {
		ofLogError("ofFileUtils") << "ofToDataPathFS: error converting fs::path to string " << e.what();
		return path;
	}

	std::string gen_string;
	try {
		gen_string = path.generic_string();
	} catch (std::filesystem::filesystem_error& e) {
		ofLogError("ofFileUtils") << "ofToDataPathFS: error converting fs::path to generic_string " << e.what();
	} catch (std::system_error& e) {
		ofLogError("ofFileUtils") << "ofToDataPathFS: error converting fs::path to generic_string " << e.what();
	}

	bool hasTrailingSlash = !path.empty() && !gen_string.empty() && gen_string.back() == '/';
	// **************************************************************************** //
	// rest of the function omitted
	......................
}

[NickHardeman]

std::string myPath = ofToDataPath("blah.txt");
ofLogNotice("my narrow path relative ") << myPath; 
std::string myPathA = ofToDataPath("blah.txt", true);
ofLogNotice("my narrow path abs ") << myPathA;

std::filesystem::path lpath(L"blah.txt");
std::string myPathLFS = ofToDataPath(lpath, true);
ofLogNotice("my narrow fs::path L ") << myPathLFS;

In this branch:

// ERROR does not work
//// no suitable constructor exists to convert from "const wchar_t [9]" to "of::filesystem::path"
//std::string myPathL = ofToDataPath(L"blah.txt", true);
//ofLogNotice("my narrow string path L ") << myPathL;

//[notice] my narrow path relative : data\blah.txt
//Exception thrown at 0x00007FFFFD70B699 in mySketch_debug.exe: Microsoft C++ exception : std::filesystem::filesystem_error at memory location 0x0000003B086FD3B0.
//[notice] my narrow path abs : [omitted]\openFrameworksfilesystempath2\apps\myApps\mySketch\bin\data\blah.txt
//Exception thrown at 0x00007FFFFD70B699 in mySketch_debug.exe: Microsoft C++ exception : std::filesystem::filesystem_error at memory location 0x0000003B086FD3B0.
//[notice] my narrow fs::path L : [omitted]\openFrameworksfilesystempath2\apps\myApps\mySketch\bin\data\blah.txt

Output from the nightly

std::string myPathL = ofToDataPath(L"blah.txt", true);
ofLogNotice("my narrow string path L ") << myPathL;

[notice] my narrow path relative : data\blah.txt
Exception thrown at 0x00007FFFFD70B699 in TestPaths_debug.exe: Microsoft C++ exception : std::filesystem::filesystem_error at memory location 0x0000005DA678D980.
Exception thrown at 0x00007FFFFD70B699 in TestPaths_debug.exe: Microsoft C++ exception : std::filesystem::filesystem_error at memory location 0x0000005DA678D980.
Exception thrown at 0x00007FFFFD70B699 in TestPaths_debug.exe: Microsoft C++ exception : std::filesystem::filesystem_error at memory location 0x0000005DA678D980.
[notice] my narrow path abs : [omitted]\of_v20250311_vs_64_release\apps\myApps\TestPaths\bin\data\blah.txt
[notice] my narrow string path L : [omitted]\of_v20250311_vs_64_release\apps\myApps\TestPaths\bin\data\blah.txt
[notice] my narrow fs::path L : [omitted]\of_v20250311_vs_64_release\apps\myApps\TestPaths\bin\data\blah.txt

[artificiel]

OK! the direct w_char constructor should deal with "const wchar_t [9]" to "of::filesystem::path"

[NickHardeman]

All of the examples in input_output seem to function as they should on both the nightly and this branch in regards to file paths, except the following:

Both the nightly and this branch have the following error, though not sure it's related to fs::path
examples / input_output / imageLoaderWebExample

[ error ] ofImage: loadImage(): couldn't load image from ofBuffer, unable to guess image format from memory
[ error ] ofImage: loadImage(): couldn't load image from "https://openframeworks.cc/about/0.jpg"
'imageLoaderWebExample_debug.exe' (Win32): Loaded 'C:\Windows\System32\TextInputFramework.dll'. Symbol loading disabled by Include/Exclude setting.
The thread 19036 has exited with code 0 (0x0).
The thread 20632 has exited with code 0 (0x0).
Exception thrown at 0x00007FF971CCB699 in imageLoaderWebExample_debug.exe: Microsoft C++ exception: std::bad_function_call at memory location 0x000000F41355F520.
[ error ] ofApp: -1 SSL peer certificate or SSH remote key was not OK for request about
Exception thrown at 0x00007FF971CCB699 in imageLoaderWebExample_debug.exe: Microsoft C++ exception: std::bad_function_call at memory location 0x000000F41355F520.
[ error ] ofApp: -1 SSL peer certificate or SSH remote key was not OK for request about
The thread 20536 has exited with code 0 (0x0).
The thread 23936 has exited with code 0 (0x0).

[NickHardeman]

@artificiel thank you, can you add the extra catches in ofPathToString and ofToDataPathFS mentioned above to this PR?
Git is borked on my Windows machine :/

[dimitre]

Great advancements @NickHardeman @artificiel
I'm wondering if it worths picking some changes from https://github.com/openframeworks/openFrameworks/pull/7435/files and adding here since it addresses some bad conversions in windows.

[artificiel]

@NickHardeman I've added the catch to ofPathToString() which is a convenience function to facilitate transition by catching conversion errors.

however ofToDataPathFS() should not touch the internal type of of::filesystem::path — I see there are calls to .string() in there, which should be reworked to be cross-platform (perhaps with auto and .native()). otherwise it breaks to purpose of supporting wide strings. so it's a separate issue, which follows the whole cleanup of ofFileUtils (all those FS functions should go, notably). but it's a larger project than this PR which is just the stepping stone enabling the rest.

the scope of this PR is (1) anything that breaks direct use of of::filesystem::path with wide unicode, as well as (2) what is required for the typical case of std::string s = ofToDataPath("non-unicode-str-as-before-but-packaged-in-wstring"); or methods like open(std:string path) (again where path contains no wide-unicode) to work for legacy projects on windows. if the wide strings don't contain wide chars, the conversion will not fail.

what is related to actual wide-narrow conversion or internal OF/libs treatment is part of a larger effort, and before that work is attempted it might be ineffective to test too deeply as many errors will just go away when the FS stuff is revised.

[artificiel]

@dimitre we crossed posts! I was just clarifying the scope of this PR, and yes to what you said, but it's a second layer of work and my PR's for this are not ready.

[ofTheo]

Thanks @artificiel - I would be in favor of merging this as soon as possible ( before 0.12.1 ) as it will prevent a good amount of issues for Windows users.

We can do some follow up PRs if needed for edge cases but might be helpful to have in the nightlies so we can ask people on the forum to test it.

[NickHardeman]

@artificiel thank you for the clarification on the scope. Thinking of the additional functionality for this PR for Windows, specifically the std::filesystem::path to std::string conversion; it seems like a benefit to include for 0.12.1. Additional PRs could build upon this one, incorporating fixes from the PR @dimitre mentioned.

[artificiel]

@ofTheo @NickHardeman in theory this PR will not fix so many true existing issues as people are currently not sending wide strings through OF (it just does not work). if they start trying funky things between "now and 12.1's release", they will hit the same blocks we know are there but not-handled-yet. another PR is required to clean FileUtils (which was a work-in-progress that stopped in the fall), and most of it is done but not completely thought-out. then an audit of every place OF processes a path should be tested with wide unicode to ensure underlying calls are doing the right things fair task which requires a clean slate of time, and a couple of separate issues/PR (ex: as was noted above ofToDataPathFS still calls .string(), and for instance what happens to wstring in ofDrawBitmapString() -> maybe a general upgrade to std::u8string makes more sense than support parallel string/wstring; design decisions that should not be taken with the idea of being quickly done. so I still think releasing this halfway done will just open a can of worms which will delay 12.1...

[ofTheo]

Thanks @artificiel for the explanation.

I see what you mean about the need to clean up ofFileUtils with the ofToDataPathFS using .string()
https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/utils/ofFileUtils.cpp#L2020-L2022

I think my priority for 0.12.1 is not as much to have unicode file paths supported ( which as you mentioned isn't working in 0.12.0 ), but to have what is in the current nightly / master work the same for Windows users as it did before. ( ie: no downgrade / errors with file paths ).

To me that should be the priority for 0.12.1, and pushing wide file paths working well on all platforms to 0.13.0

From my understanding the main issue is that currently in the nightly builds any function that returns a file path where it used to return a string will break for Windows users.

With that in mind if the goal is to have 0.12.0 like behavior can you see any minimal changes that could allow that without reverting all of the filesystem progress we have in there now?

[artificiel]

@ofTheo ah! but the current nightly does not introduce systematic of::filesystem::path! that's what why the FS-flavored functions are still laying there; there was a moment where it was enabled, then backtracked, which led to this PR to fix things properly then momentum sort of fizzled as "real world windows" was needed to test (what we're now doing).

current ofToDataPath() returns a string, as do all path-ish methods:

https://github.com/openframeworks/openFrameworks/blob/1954b2ec662f3f754209c7ca9ee1bee0424f4982/libs/openFrameworks/utils/ofFileUtils.cpp#L2055C1-L2057C2

I now get why you want to push it out, but things are currently patched for whatever worked in 12.0 to keep working the same.

[ofTheo]

@artificiel thanks!! I think I forgot about that rollback. 🤦‍♂️

So basically as it is now, on Windows, we'll only get issues if someone passes a wide string in as an argument to ofToDataPath? And in that case it will trigger an exception in ofPathToString ?

[artificiel]

@ofTheo exactly! all path methods are wrapped in ofPathToString(). next step (enabled by this PR) is to remove that but it's a fairly wide step.

[NickHardeman]

@artificiel what do you think would be the best approach for removing ofPathToString()? Keep building/submitting in this PR?

[artificiel]

@nick it would make this PR complicated and hard to validate if "further work" is started. it's really a stepping stone. because OF commits are squashed (which is good IMO) it would also make a very wide single commit as it will touch so many files. once this is committed we can go step by step. so the sooner 12.1 can be tagged, the better!

also one thing I hope is OK to do is break down ofFileUtils (it will need a bit of project template tweaks); it's a huge file and the interdependencies can be broken down with a bit of work, but after that it really make working on the classes better: