Page MenuHome

Fix T79801: openvdb cache does not support unicode paths
ClosedPublic

Authored by Ray Molenkamp (LazyDodo) on May 18 2022, 7:06 PM.

Details

Summary

Fix must be taken with a grain of salt, this will fix the
issue on win10 1903+ by specifying an active codepage in
the application manifest.

What broke? OpenVDB uses boosts memory mapped files
which call CreateFileA in the backend when you feed it a
regular string. I think there may be support for wide
strings in boost, but openvdb is not using that.

Now MS didn't implement the whole API twice, what happens
when you call the A variant, is the A variants convert the
input to wide with whatever codepage (CP_APC previously
only changable on a system wide basis) is set, and call
the W function. Win10 1903 added a way to change this
codepage on a per application basis by specifying it
in the manifest.

for stdlib io (fopen+if_stream'n'friends) it's sufficient
to call setlocale(LC_ALL, ".utf8"); but that's a story
for a different diff.

not sure how i feel only being able to fix it on 1903+ but
while BLI_get_short_name works, the number of places that
call openvdb::io::file::open is very much not 1, and some
of those places are beyond our control (ie USD/Mantaflow)

Diff Detail

Repository
rB Blender
Branch
tmp_openvdb_unicode (branched from master)
Build Status
Buildable 22154
Build 22154: arc lint + arc unit

Event Timeline

Ray Molenkamp (LazyDodo) requested review of this revision.May 18 2022, 7:06 PM
Ray Molenkamp (LazyDodo) created this revision.

I guess a potential risk here is an external library that assumes the default code page, and will now break instead? I'm not sure how likely that is.

I've wanted to fix this in OpenVDB itself and contribute it upstream but never got around to it. But this may be an ok workaround if you think the risk is acceptable.

I don't think you can make any assumptions in code about the default codepage, if code is accommodating for wide paths on windows, they are likely to call multibytetowidechar by them selves, (which also advises against relying on CP_ACP) and call the W variant, I'm a little hesitant since it's a new option not sure if we'll run into unintended consequences, we are still in bcon1 for 3.3 and it's a 5 line patch, it's an easy revert, it be more comfortable landing it for 3.3 only rather than for 3.3+3.2

i have to admit, it is nice to see both win32 and stdlib on windows are finally getting it together in the utf-8 department

Ok, looks good to me then.

This revision is now accepted and ready to land.May 20 2022, 4:28 PM