--data-path command line option not working

[SRSaunders]

It appears the --data-path command line option is not hooked up inside the code. I can successfully run the samples via the command line when my working dir is the home installation directory (i.e. Vulkan-Samples). However, if I cd to the executable dir and try to run from there using the --data-path option, the samples fail to load the shaders.

Looking at the code, --data-path does successfully set the external_storage_directory static variable inside platform.h but that's about it. When the shaders are loaded the code uses _external_storage_directory from std_filesystem.h to form the path which does not equal external_storage_directory from platform.h.

I am running on macOS, but I suspect this is also a problem on linux. Note this prevents me from using Xcode to run and debug.

[SaschaWillems]

Thanks for bringing this up. And yes, you're right, I guess this was broken with #894 or maybe never worked at all. We'll take a look.

[tomadamatkinson]

@SRSaunders I'll have a look at this as you mentioned it was still an issue. Likely not a huge fix

[SRSaunders]

@tomadamatkinson any updates?

I have prototyped a solution to this problem, but it's the opposite to what the comments in the code imply. The existing comments seem to indicate that StdFileSystem::external_storage_directory() should somehow be modified to return the value in Platform::external_storage_directory. However, the code structure and include tree make this potentially quite difficult given that you cannot easily include platform.h without creating a lot of include path problems.

Instead I took the opposite approach and added a new set_external_storage_directory() method to FileSystem and StdFileSystem (as an override). This sets the value of StdFileSystem::_external_storage_directory when called from Platform::set_external_storage_directory(). That method now looks like this...

void Platform::set_external_storage_directory(const std::string &dir)
{
    auto fs = vkb::filesystem::get();
    fs->set_external_storage_directory(dir);
}

To make this work I had to "unhide" std_filesystem.hpp and move it into the normal filesystem include tree, plus add #include "filesystem/std_filesystem.hpp" to platform.cpp. It turns out all I had to do was #include "filesystem/filesystem.hpp" to platform.cpp for FileSystem base class visibility.

By doing this, any future call to StdFileSystem::external_storage_directory() returns the updated directory path stored in StdFileSystem::_external_storage_directory. A positive side effect of this is you can simplify things and eliminate Platform::external_storage_directory and Platform::get_external_storage_directory() plus Platform::temp_directory and Platform::get_temp_directory() as they are no longer required. Actually, you can go even further and do the following, eliminating Platform::set_external_storage_directory() as well:

void DataPath::init(const vkb::CommandParser &parser)
{
    if (parser.contains(&data_path_flag))
    {
        auto fs = vkb::filesystem::get();
        fs->set_external_storage_directory(parser.as<std::string>(&data_path_flag) + "/");
    }
}

This cleans things up and completely removes the need for Platform to handle data and temp paths. Thoughts?

[SRSaunders]

I decided to create a pull request so @tomadamatkinson can review my proposed changes. I have tested on all platforms except for Android. Seems to work properly. Happy to receive comments/feedback and make changes as required.

With this change I can now debug/run under Xcode (where cwd is the bin dir and not project root), and similarly on linux. Neutral on Windows since VS automatically handles cwd so that executable appears to be running from project root. Note that Xcode *can* be manually configured with a custom working directory (under Scheme: Options), but the --data-path solution is more general and works with all platforms.