Add support for globbing on Windows (when needed)

[trevordblack]

I'm getting moana's island scene to render on gpu.

At present, there's only a single cli option of --upgrade for converting a single pbrt-v3 scene to pbrt-v4, and this is dumped to stdout.

The Moana island scene has a nested collection of pbrt-v3 scenes. Going through and converting them one by one to pbrt-v4 is a mild annoyance. Does it make sense to spin up a recursive --upgrade-in-place option to upgrade all scenes in place at once?

It looks like v4 isn't accepting of wildcards in it's input

C:\Users\Trevor\pbrt-v4>build_vs2019\Release\pbrt.exe --upgrade ..\island-pbrt-v1.1\island\pbrt\isBayCedarA1\*
←[1m←[31mError←[0m: ..\island-pbrt-v1.1\island\pbrt\isBayCedarA1\: The parameter is incorrect.

So this might not be a trivial addition. Not sure if value, I can go off and implement if interest. Can also include a are you really sure you want to override existing files... ? prompt.

This might also just be a one-time annoyance for most scenes of interest.

[mmp]

Ah, that's interesting about wildcards. ISTR that on Windows applications have to expand those themselves, while pbrt assumes that the shell does it. I can look into that at some point, though patches for that would be welcome. :-)

For mass upgrades like that on Linux/Mac, I've done the equivalent of (untested): for x in `find -name \*.pbrt -type f`; do pbrt --upgrade $x > $x.tmp; mv $x.tmp $x; done. Moana is tricky since the current --upgrade doesn't handle the Disney material well and basically makes everything diffuse.

However... I have taken that automatic conversion of Moana Island as a starting point and then massaged it until it worked and then done some work on optimizing startup time; here is a link to the result. Note that you need to have the Moana island textures directory next to that directory for everything to work. I've sent this along to folks at Disney and think it will end up on their Moana Island page at some point, though certainly not before SIGGRAPH is over.

[trevordblack]

I've been reading through your blogs on the matter, they were the motivation I needed to pick the renderer back up. I'll investigate what's causing problems on windows. I haven't used the windows command prompt in ages, and try to avoid it where possible. It might specific to that cli, and powershell may not have the same problem.

Thank you for the tgz, that'll come in handy

[trevordblack]

Dropping work for a recursive --upgrade-in-place If there is interest, I can quickly finish it up, but I question if there is interest.

As for the problems of wildcards on windows, it appears to be specific to visual studio's compiler https://docs.microsoft.com/en-us/cpp/cpp/main-function-command-line-args?view=msvc-160#wildcard-expansion

From there I got to (CMakeLists.txt):

847 if (WIN32)
848   # Avoid a name clash when building on Visual Studio
849   set_target_properties (pbrt_lib PROPERTIES OUTPUT_NAME libpbrt)
850   # Enable wildcard expansion on Visual Studio
851   set_target_properties (pbrt_lib PROPERTIES STATIC_LIBRARY_FLAGS "/link setargv.obj")
852 endif()

Which didn't work.

[pierremoreau]

Maybe trying adding that property to pbrt_exe instead, as that is the target containing the entry point and doing the argument parsing.

if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
  # Enable wildcard expansion on Visual Studio
  set_target_properties (pbrt_exe PROPERTIES LINK_FLAGS "/link setargv.obj")
endif ()

Too bad target_link_options() was introduced only in 3.13, and PBRT’s minimum required version is 3.12.

[trevordblack]

No dice. Both of

923
924 if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
925     message("in the taco bell, in the msvc")
926     set_target_properties(pbrt_exe PROPERTIES LINK_FLAGS "/link setargv.obj")
927 endif()

and

923
924 if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
925     message("in the taco bell, in the msvc")
926     set_target_properties(pbrt_exe PROPERTIES LINK_FLAGS "setargv.obj")
927 endif()

Did not work. Still hitting the The parameter is incorrect. error.

Modifying slightly to

113 if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
114     message("in the taco bell, in the msvc")
115     string(APPEND CMAKE_EXE_LINKER_FLAGS " setargv.obj")
116 endif()

also did not work.

Nor did trying wsetargv.obj instead. I'm definitely missing something more fundamental.

[mmp]

I'm no expert on Windows command line processing, but the issue may be that pbrt gets the arguments on Windows via GetCommandLineW() (https://github.com/mmp/pbrt-v4/blob/07d4749cf1cb0244d32a030819dbf6fce93cdd6b/src/pbrt/util/args.cpp#L18); maybe the setargv.obj/wsetargv.obj trick doesn't apply in that case?

After a bit of googling, this seems to suggest that it may be necessary to manually use FindFirstFileW and FindNextFileW to expand wildcards in this case..