Build win32

[tksharpless]

Summary

Build facebook360-dep on Windows using cmake targeting MSVCC toolchain;
dependencies installed with vcpkg;
result is native executables not requiring Docker.

Changelog

modified cmake scripts
-- target MSVCC 14.1 (VS 2017) toolchain
-- correct some library references
-- new sub-script to build ISPC compressor libraries
source code mods addressing
-- gflags linkage differences
-- use of wchar pathnames in boost::filesystem
-- remove some incorrect win32 options

Test Plan

clone repo
install required packages with vcpkg --
configure a build with cmake using VC15 project generator
build solution with Visual Studio
verify unit tests pass
run several processing jobs, comparing output with reference Linux build

[tksharpless]

This is a complex work in progress, not expected to be production ready for some time.
The purpose of this pull request is to let facebook devs examine the proposed changes and provide feedback.

[facebook-github-bot]

@aparrapo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[tksharpless]

It appears that zlib is not installed on the test build system. This cannot be due to the code changes in the branch under test.

[aparrapo]

zlib is in Dockerfile:
https://github.com/facebook/facebook360_dep/blob/master/Dockerfile#L92

Try surrounding the include on CMakeLists.txt to avoid the include on UNIX/Apple builds. It can still work, it may just not find the package config file:

if(UNIX AND NOT APPLE):
  find_package(zlib REQUIRED)
endif()

[aparrapo]

should this be a regular else()?

[tksharpless]

Cmake allows arguments to else, endif etc as fiducial labels (used to require them). If given they must match the argument of matching bracket. So this is OK, but not necessary.
I see now that it actually is my fault that the Linux build failed with"could not find zlib". Evidently Linux packages don't need an external zlib; but Windows packages universally do, so I just reflexively added find_package(zlib). In the latest commit, which I will push soon, this line is protected by if(WIN32) so that build error should not happen again.
The new cmakelists includes the capability to create a vcpkg instance and populate it with all the required packages (except ICPC, which is not on the vcpkg repo); or alternatively to use an existing vcpkg instance. This is supported by cmake scripts in a new directory 'build_win32', which also has instructions for my windows build. All of this is WIN32-only and should have no effect on the Linux build.

[facebook-github-bot]

@tksharpless has updated the pull request. Re-import the pull request

[tksharpless]

Glad that the Linux build is working. I still have to add ispc and test to the Windows build, then will push again. I don't envision any further changes that might affect the Linux build, save possible source changes for compiler compatibility.
I hope you will be able to do some testing of the Windows build when it is complete. Note that it is not a Docker build and involves a manual step -- building the MSVC solution -- which actually could be automated with msbuild but I see no need for that at present.

[tksharpless]

Question about Dep_unit_test. In CMakelists.txt one of the target link libraries is specified as 'gtest', but there is no find_package() for gtest. How does this library reference get resolved on Linux?
On windows the generated link command has 'gtest.lib' with no path, and the link fails.
And strangely, although adding find_pakage(gtest) does find the vcpkg installation of gtest, the name in the link command does not change.

[aparrapo]

find_package forces CMake to search for a file called Find<package>.cmake, which contains information about where the package is installed, its version, and other useful details. Some packages do not add this .cmake file (e.g. sometimes via apt-get) but it doesn't mean the package is not installed. It will be found in the default installation path(s).

In Windows you can try adding the gtest source directory as a project subdirectory:
https://stackoverflow.com/a/21479008

[tksharpless]

I still don't understand what is going wrong here. The facts are: 1) your original cmakelists.txt mentions gtest only once, as a link library name. The resulting project specifies that name, without any path, as a linker input, and the linker reports a nonexistent file. (I would expect the same result on any platform). 2) gtest installed in my vcpkg tree without incident, and find_package(gtest REQUIRED) finds it, however the resulting CMake Cache shows that it has been handled differently from glog and gflags, for which there are also find_package commands. Those two packages have XXXX_DIR entries in the cache, and their library files are named correctly in the generated projects. All the other packages conform to the same pattern. But for gtest, the cache has entries like

GTEST_LIBRARY C:/Users/tommy/fb360-dep-testing/vcpkg/installed/x64-windows/lib/gtest.lib

These give the correct file names, however the reference in the link libraries command still comes out as a plain name. 3) The gtest package has a couple of anomalous features. It does not have a proper version number, but rather the string "2019-10-09" which cmake will not accept as a version. And it namespaces itself as "GTest::gtest" while all other packages I've seen spell the namespace and symbol names the same. Hoever lowercasing "GTest" does not fix the problem. 4) The unit test builds OK if I put the correct gtest library name in the project by hand. So evidently this is a cmake and/or vcpkg problem, and I am taking it up with their devs. Basically, a library called gtest has been installed in vcpkg and imported in cmake, but somehow a reference to link library 'gtest' does not target it.

…

On Mon, Oct 14, 2019 at 10:38 AM Albert Parra Pozo ***@***.***> wrote: find_package forces CMake to search for a file called Find<package>.cmake, which contains information about where the package is installed, its version, and other useful details. Some packages do not add this .cmake file (e.g. sometimes via apt-get) but it doesn't mean the package is not installed. It will be found in the default installation path(s). In Windows you can try adding the gtest source directory as a project subdirectory: https://stackoverflow.com/a/21479008 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#12?email_source=notifications&email_token=ACBEZZS5R6WO7SSWUBIVUG3QOR75BA5CNFSM4I7HPXNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBE7KFQ#issuecomment-541717782>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACBEZZUDTAGLSQHZL4UHMFDQOR75BANCNFSM4I7HPXNA> .

[tksharpless]

When I build with the hand-corrected vcxproject, -- gtest.dll gets copied to the project bin directory along with the other rq'd dlls -- DepUnitTest runs all 29 tests successfully (log attached) So I would say that I can now build a native facebook360-dep for Windows, however not quite automatically due to the glitch with naming the gtest library. On Mon, Oct 14, 2019 at 12:53 PM Thomas Sharpless <[email protected]> wrote:

I still don't understand what is going wrong here. The facts are: 1) your original cmakelists.txt mentions gtest only once, as a link library name. The resulting project specifies that name, without any path, as a linker input, and the linker reports a nonexistent file. (I would expect the same result on any platform). 2) gtest installed in my vcpkg tree without incident, and find_package(gtest REQUIRED) finds it, however the resulting CMake Cache shows that it has been handled differently from glog and gflags, for which there are also find_package commands. Those two packages have XXXX_DIR entries in the cache, and their library files are named correctly in the generated projects. All the other packages conform to the same pattern. But for gtest, the cache has entries like > GTEST_LIBRARY > C:/Users/tommy/fb360-dep-testing/vcpkg/installed/x64-windows/lib/gtest.lib > These give the correct file names, however the reference in the link libraries command still comes out as a plain name. 3) The gtest package has a couple of anomalous features. It does not have a proper version number, but rather the string "2019-10-09" which cmake will not accept as a version. And it namespaces itself as "GTest::gtest" while all other packages I've seen spell the namespace and symbol names the same. Hoever lowercasing "GTest" does not fix the problem. 4) The unit test builds OK if I put the correct gtest library name in the project by hand. So evidently this is a cmake and/or vcpkg problem, and I am taking it up with their devs. Basically, a library called gtest has been installed in vcpkg and imported in cmake, but somehow a reference to link library 'gtest' does not target it. On Mon, Oct 14, 2019 at 10:38 AM Albert Parra Pozo < ***@***.***> wrote: > find_package forces CMake to search for a file called Find<package>.cmake, > which contains information about where the package is installed, its > version, and other useful details. Some packages do not add this .cmake > file (e.g. sometimes via apt-get) but it doesn't mean the package is not > installed. It will be found in the default installation path(s). > > In Windows you can try adding the gtest source directory as a project > subdirectory: > https://stackoverflow.com/a/21479008 > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#12?email_source=notifications&email_token=ACBEZZS5R6WO7SSWUBIVUG3QOR75BA5CNFSM4I7HPXNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBE7KFQ#issuecomment-541717782>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/ACBEZZUDTAGLSQHZL4UHMFDQOR75BANCNFSM4I7HPXNA> > . >

[==========] Running 29 tests from 5 test suites. [----------] Global test environment set-up. [----------] 1 test from MatchCornersTest [ RUN ] MatchCornersTest.TestTransformationDetection [ OK ] MatchCornersTest.TestTransformationDetection (749 ms) [----------] 1 test from MatchCornersTest (749 ms total) [----------] 1 test from DerpTest [ RUN ] DerpTest.TestUtilFilterDestinations [ OK ] DerpTest.TestUtilFilterDestinations (0 ms) [----------] 1 test from DerpTest (0 ms total) [----------] 9 tests from FThetaTest [ RUN ] FThetaTest.TestInitialization [ OK ] FThetaTest.TestInitialization (0 ms) [ RUN ] FThetaTest.TestFOV [ OK ] FThetaTest.TestFOV (0 ms) [ RUN ] FThetaTest.TestUndoPixel [ OK ] FThetaTest.TestUndoPixel (0 ms) [ RUN ] FThetaTest.TestGetSetRotation [ OK ] FThetaTest.TestGetSetRotation (0 ms) [ RUN ] FThetaTest.TestDistortUndistortNOP [ OK ] FThetaTest.TestDistortUndistortNOP (0 ms) [ RUN ] FThetaTest.TestDistortUndistort [ OK ] FThetaTest.TestDistortUndistort (0 ms) [ RUN ] FThetaTest.TestUndistortMonotonic [ OK ] FThetaTest.TestUndistortMonotonic (0 ms) [ RUN ] FThetaTest.TestNormalize [ OK ] FThetaTest.TestNormalize (0 ms) [ RUN ] FThetaTest.TestRescale [ OK ] FThetaTest.TestRescale (0 ms) [----------] 9 tests from FThetaTest (1 ms total) [----------] 9 tests from RectilinearTest [ RUN ] RectilinearTest.TestInitialization [ OK ] RectilinearTest.TestInitialization (0 ms) [ RUN ] RectilinearTest.TestFOV [ OK ] RectilinearTest.TestFOV (0 ms) [ RUN ] RectilinearTest.TestUndoPixel [ OK ] RectilinearTest.TestUndoPixel (0 ms) [ RUN ] RectilinearTest.TestGetSetRotation [ OK ] RectilinearTest.TestGetSetRotation (0 ms) [ RUN ] RectilinearTest.TestDistortUndistortNOP [ OK ] RectilinearTest.TestDistortUndistortNOP (0 ms) [ RUN ] RectilinearTest.TestDistortUndistort [ OK ] RectilinearTest.TestDistortUndistort (0 ms) [ RUN ] RectilinearTest.TestUndistortMonotonic [ OK ] RectilinearTest.TestUndistortMonotonic (0 ms) [ RUN ] RectilinearTest.TestNormalize [ OK ] RectilinearTest.TestNormalize (0 ms) [ RUN ] RectilinearTest.TestRescale [ OK ] RectilinearTest.TestRescale (0 ms) [----------] 9 tests from RectilinearTest (0 ms total) [----------] 9 tests from OrthographicTest [ RUN ] OrthographicTest.TestInitialization [ OK ] OrthographicTest.TestInitialization (0 ms) [ RUN ] OrthographicTest.TestFOV [ OK ] OrthographicTest.TestFOV (0 ms) [ RUN ] OrthographicTest.TestUndoPixel [ OK ] OrthographicTest.TestUndoPixel (0 ms) [ RUN ] OrthographicTest.TestGetSetRotation [ OK ] OrthographicTest.TestGetSetRotation (0 ms) [ RUN ] OrthographicTest.TestDistortUndistortNOP [ OK ] OrthographicTest.TestDistortUndistortNOP (0 ms) [ RUN ] OrthographicTest.TestDistortUndistort [ OK ] OrthographicTest.TestDistortUndistort (0 ms) [ RUN ] OrthographicTest.TestUndistortMonotonic [ OK ] OrthographicTest.TestUndistortMonotonic (0 ms) [ RUN ] OrthographicTest.TestNormalize [ OK ] OrthographicTest.TestNormalize (0 ms) [ RUN ] OrthographicTest.TestRescale [ OK ] OrthographicTest.TestRescale (0 ms) [----------] 9 tests from OrthographicTest (0 ms total) [----------] Global test environment tear-down [==========] 29 tests from 5 test suites ran. (750 ms total) [ PASSED ] 29 tests.

[facebook-github-bot]

@tksharpless has updated the pull request. Re-import the pull request

[tksharpless]

Albert, can you supply a set of binary image/strip files for testing the GlViewer?

…

On Mon, Oct 14, 2019 at 1:50 PM Facebook Community Bot < ***@***.***> wrote: @tksharpless <https://github.com/tksharpless> has updated the pull request. Re-import the pull request <https://our.intern.facebook.com/intern/opensource/github/pull_request/confirm/import/422570605062108/> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#12?email_source=notifications&email_token=ACBEZZQ3BTRWPABXU6MN7GLQOSWPXA5CNFSM4I7HPXNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBFY42Y#issuecomment-541822571>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACBEZZUIAGJXBSCHAYRBFCDQOSWPXANCNFSM4I7HPXNA> .

[facebook-github-bot]

@tksharpless has updated the pull request. Re-import the pull request

[facebook-github-bot]

@aparrapo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[aparrapo]

@tksharpless we somehow forgot to add the fused/striped data to the single frame dataset, but it should be on the multi-frame rendered sample sequence:
https://facebook360-dep-downloads.s3-us-west-2.amazonaws.com/room_chat/50_frames_unpacked_rendered.zip

Try to use that while we re-compute the data for the single frame case.

[tksharpless]

That is an awfully big file, but I will try it.

…

On Wed, Oct 16, 2019 at 9:17 PM Albert Parra Pozo ***@***.***> wrote: @tksharpless <https://github.com/tksharpless> we somehow forgot to add the fused/striped data to the single frame dataset, but it should be on the multi-frame rendered sample sequence: https://facebook360-dep-downloads.s3-us-west-2.amazonaws.com/room_chat/50_frames_unpacked_rendered.zip Try to use that while we re-compute the data for the single frame case. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12?email_source=notifications&email_token=ACBEZZV64K3JKU55M4ILTL3QO64KNA5CNFSM4I7HPXNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBONKGI#issuecomment-542954777>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACBEZZVPDHF35SSSCVYU75LQO64KNANCNFSM4I7HPXNA> .

[aparrapo]

@tksharpless did you get a chance to test GlViewer?

[facebook-github-bot]

@aparrapo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[tksharpless]

Hi Albert I have paid no attention to FB-DEP in the past couple of months; devoting all efforts to my own depth mapping pipeline. Did I promise to test the viewer on Windows? If so I will try to do that soon. -- Tom

…

On Mon, Dec 16, 2019 at 9:14 AM Albert Parra Pozo ***@***.***> wrote: @tksharpless <https://github.com/tksharpless> did you get a chance to test GlViewer? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12?email_source=notifications&email_token=ACBEZZWR57IWC4HPIET3ITDQY6EOFA5CNFSM4I7HPXNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG623EI#issuecomment-566078865>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACBEZZUX3Z2FPH7WDBO4VALQY6EOFANCNFSM4I7HPXNA> .