pbrt-v4
pbrt-v4 copied to clipboard
Update the Ptex submodule to better support MinGW on Windows
My building environment is windows with mingw64-gcc. Now its build will fail like below:
In file included from C:/Users/x/Desktop/pbrt-v4/src/ext/ptex/src/ptex/Ptexture.h:65,
from C:/Users/x/Desktop/pbrt-v4/src/pbrt/textures.cpp:24:
C:/Users/x/Desktop/pbrt-v4/src/ext/ptex/src/ptex/PtexInt.h:49:27: error: conflicting declaration
'typedef char int8_t'
49 | typedef __int8 int8_t;
| ^~~~~~
In file included from C:/msys64/ucrt64/lib/gcc/x86_64-w64-mingw32/12.2.0/include/stdint.h:9,
from C:/Users/x/Desktop/pbrt-v4/src/pbrt/pbrt.h:8,
from C:/Users/x/Desktop/pbrt-v4/src/pbrt/textures.h:8,
from C:/Users/x/Desktop/pbrt-v4/src/pbrt/textures.cpp:5:
C:/msys64/ucrt64/include/stdint.h:35:21: note: previous declaration as 'typedef signed char int8_t'
35 | typedef signed char int8_t;
| ^~~~~~
[248/462] Building CXX object CMakeFiles/pbrt_lib.dir/src/pbrt/cpu/integrators.cpp.obj
The reason is as follow:
#if defined(_WIN32) || defined(_WINDOWS) || defined(_MSC_VER)
#if defined(_MSC_VER) && _MSC_VER >= 1600
#include <stdint.h>
#else
typedef __int8 int8_t;
typedef __int16 int16_t;
typedef __int32 int32_t;
typedef __int64 int64_t;
typedef unsigned __int8 uint8_t;
typedef unsigned __int16 uint16_t;
typedef unsigned __int32 uint32_t;
typedef unsigned __int64 uint64_t;
#endif
#else
#include <stdint.h>
#endif
When building with MinGW, it won't include <stdint.h>
and use its own int types.
The newest version of plex has fixed this problem.
#if defined(_MSC_VER) && _MSC_VER >= 1600 || defined(__MINGW64__)
Add this feature test macro then pbrt can successfully build!
Yes.. I are send a pull request a few weeks ago fixing this issue: https://github.com/wdas/ptex/commit/c138f8391c3b6555c347d25ae9b05eca778a29a1
Cheers..
It would be good to update pbrt to use a modern version of ptex, but unfortunately it's not straightforward. I've pushed a branch, update-ptex
, that updates Ptex to v2.4.2 but it causes build breakage on windows: https://github.com/mmp/pbrt-v4/actions/runs/3401367217 (though works elsewhere). I don't have the time dig into the further at the moment, but would happily accept a PR that got it all working!
This patch should fix it. I thought I upstreamed it but maybe not
diff --git a/CMakeLists.txt b/CMakeLists.txt index 4c7e0c3..a933eea 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -33,9 +33,7 @@ enable_testing()
Setup platform-specific threading flags.
find_package(Threads REQUIRED)
-# Use pkg-config to create a PkgConfig::Ptex_ZLIB imported target -find_package(PkgConfig REQUIRED) -pkg_checK_modules(Ptex_ZLIB REQUIRED zlib IMPORTED_TARGET) +find_package(ZLIB REQUIRED)
if (NOT DEFINED PTEX_SHA) diff --git a/src/build/ptex-config.cmake b/src/build/ptex-config.cmake index dfe2851..5e63f07 100644 --- a/src/build/ptex-config.cmake +++ b/src/build/ptex-config.cmake @@ -8,9 +8,7 @@ set(THREADS_PREFER_PTHREAD_FLAG ON)
find_dependency(Threads REQUIRED)
-# Provide PkgConfig::ZLIB to downstream dependents -find_dependency(PkgConfig REQUIRED) -pkg_checK_modules(Ptex_ZLIB REQUIRED zlib IMPORTED_TARGET) +find_package(ZLIB REQUIRED)
set_and_check(Ptex_DIR @PACKAGE_CMAKE_INSTALL_PREFIX@) set_and_check(Ptex_LIBRARY_DIRS @PACKAGE_CMAKE_INSTALL_LIBDIR@) diff --git a/src/ptex/CMakeLists.txt b/src/ptex/CMakeLists.txt index e2e1bfd..4dfe372 100644 --- a/src/ptex/CMakeLists.txt +++ b/src/ptex/CMakeLists.txt @@ -23,7 +23,7 @@ if(PTEX_BUILD_STATIC_LIBS) PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) target_link_libraries(Ptex_static
-
PUBLIC Threads::Threads PkgConfig::Ptex_ZLIB)
-
install(TARGETS Ptex_static EXPORT Ptex DESTINATION ${CMAKE_INSTALL_LIBDIR}) endif()PUBLIC Threads::Threads ZLIB::ZLIB)
@@ -39,7 +39,7 @@ if(PTEX_BUILD_SHARED_LIBS) ${CMAKE_CURRENT_SOURCE_DIR}) target_compile_definitions(Ptex_dynamic PRIVATE PTEX_EXPORTS) target_link_libraries(Ptex_dynamic
-
PUBLIC Threads::Threads PkgConfig::Ptex_ZLIB)
-
install(TARGETS Ptex_dynamic EXPORT Ptex DESTINATION ${CMAKE_INSTALL_LIBDIR}) endif()PUBLIC Threads::Threads ZLIB::ZLIB)
diff --git a/src/utils/CMakeLists.txt b/src/utils/CMakeLists.txt index d0295cb..f6bd83d 100644 --- a/src/utils/CMakeLists.txt +++ b/src/utils/CMakeLists.txt @@ -4,6 +4,6 @@ if (PTEX_BUILD_STATIC_LIBS) add_definitions(-DPTEX_STATIC) endif()
-target_link_libraries(ptxinfo ${PTEX_LIBRARY} PkgConfig::Ptex_ZLIB) +target_link_libraries(ptxinfo ${PTEX_LIBRARY} ZLIB::ZLIB)
install(TARGETS ptxinfo DESTINATION ${CMAKE_INSTALL_BINDIR})
On Sun, 6 Nov 2022 at 08:57, Matt Pharr @.***> wrote:
It would be good to update pbrt to use a modern version of ptex, but unfortunately it's not straightforward. I've pushed a branch, update-ptex, that updates Ptex to v2.4.2 but it causes build breakage on windows: https://github.com/mmp/pbrt-v4/actions/runs/3401367217 (though works elsewhere). I don't have the time dig into the further at the moment, but would happily accept a PR that got it all working!
— Reply to this email directly, view it on GitHub https://github.com/mmp/pbrt-v4/issues/297#issuecomment-1304624241, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOYQXJHWLCAMCHQFH5M3Q3WG23Y7ANCNFSM6AAAAAARD4OT5I . You are receiving this because you are subscribed to this thread.Message ID: @.***>
Ah no my bad, I did upstream it and the version you’re using includes this fix.
On Sun, 6 Nov 2022 at 11:32, Anders Langlands @.***> wrote:
This patch should fix it. I thought I upstreamed it but maybe not
diff --git a/CMakeLists.txt b/CMakeLists.txt index 4c7e0c3..a933eea 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -33,9 +33,7 @@ enable_testing()
Setup platform-specific threading flags.
find_package(Threads REQUIRED)
-# Use pkg-config to create a PkgConfig::Ptex_ZLIB imported target -find_package(PkgConfig REQUIRED) -pkg_checK_modules(Ptex_ZLIB REQUIRED zlib IMPORTED_TARGET) +find_package(ZLIB REQUIRED)
if (NOT DEFINED PTEX_SHA) diff --git a/src/build/ptex-config.cmake b/src/build/ptex-config.cmake index dfe2851..5e63f07 100644 --- a/src/build/ptex-config.cmake +++ b/src/build/ptex-config.cmake @@ -8,9 +8,7 @@ set(THREADS_PREFER_PTHREAD_FLAG ON)
find_dependency(Threads REQUIRED)
-# Provide PkgConfig::ZLIB to downstream dependents -find_dependency(PkgConfig REQUIRED) -pkg_checK_modules(Ptex_ZLIB REQUIRED zlib IMPORTED_TARGET) +find_package(ZLIB REQUIRED)
set_and_check(Ptex_DIR @PACKAGE_CMAKE_INSTALL_PREFIX@) set_and_check(Ptex_LIBRARY_DIRS @PACKAGE_CMAKE_INSTALL_LIBDIR@) diff --git a/src/ptex/CMakeLists.txt b/src/ptex/CMakeLists.txt index e2e1bfd..4dfe372 100644 --- a/src/ptex/CMakeLists.txt +++ b/src/ptex/CMakeLists.txt @@ -23,7 +23,7 @@ if(PTEX_BUILD_STATIC_LIBS) PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) target_link_libraries(Ptex_static
PUBLIC Threads::Threads PkgConfig::Ptex_ZLIB)
install(TARGETS Ptex_static EXPORT Ptex DESTINATION ${CMAKE_INSTALL_LIBDIR}) endif()PUBLIC Threads::Threads ZLIB::ZLIB)
@@ -39,7 +39,7 @@ if(PTEX_BUILD_SHARED_LIBS) ${CMAKE_CURRENT_SOURCE_DIR}) target_compile_definitions(Ptex_dynamic PRIVATE PTEX_EXPORTS) target_link_libraries(Ptex_dynamic
PUBLIC Threads::Threads PkgConfig::Ptex_ZLIB)
install(TARGETS Ptex_dynamic EXPORT Ptex DESTINATION ${CMAKE_INSTALL_LIBDIR}) endif()PUBLIC Threads::Threads ZLIB::ZLIB)
diff --git a/src/utils/CMakeLists.txt b/src/utils/CMakeLists.txt index d0295cb..f6bd83d 100644 --- a/src/utils/CMakeLists.txt +++ b/src/utils/CMakeLists.txt @@ -4,6 +4,6 @@ if (PTEX_BUILD_STATIC_LIBS) add_definitions(-DPTEX_STATIC) endif()
-target_link_libraries(ptxinfo ${PTEX_LIBRARY} PkgConfig::Ptex_ZLIB) +target_link_libraries(ptxinfo ${PTEX_LIBRARY} ZLIB::ZLIB)
install(TARGETS ptxinfo DESTINATION ${CMAKE_INSTALL_BINDIR})
On Sun, 6 Nov 2022 at 08:57, Matt Pharr @.***> wrote:
It would be good to update pbrt to use a modern version of ptex, but unfortunately it's not straightforward. I've pushed a branch, update-ptex, that updates Ptex to v2.4.2 but it causes build breakage on windows: https://github.com/mmp/pbrt-v4/actions/runs/3401367217 (though works elsewhere). I don't have the time dig into the further at the moment, but would happily accept a PR that got it all working!
— Reply to this email directly, view it on GitHub https://github.com/mmp/pbrt-v4/issues/297#issuecomment-1304624241, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOYQXJHWLCAMCHQFH5M3Q3WG23Y7ANCNFSM6AAAAAARD4OT5I . You are receiving this because you are subscribed to this thread.Message ID: @.***>
AFAICT this requires adding an EXPORT parameter to zlib's install command. I've made a PR to mitsuba's zlib fork to add that here: https://github.com/mitsuba-renderer/zlib/pull/1
I've tested it on your branch and the configuration succeeds, but the build fails with an unrelated(?) error:
FAILED: CMakeFiles/pbrt_lib.dir/src/pbrt/textures.cpp.obj C:\PROGRA~2\MICROS~4\2019\COMMUN~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe /nologo /TP -DNOMINMAX -DPBRT_DEBUG_BUILD -DPBRT_HAS_INTRIN_H -DPBRT_HAVE__ALIGNED_MALLOC -DPBRT_IS_MSVC -DPBRT_IS_WINDOWS -DPBRT_NOINLINE=__declspec(noinline) -DPBRT_RESTRICT=__restrict -DPTEX_STATIC -D_CRT_SECURE_NO_WARNINGS -D_ENABLE_EXTENDED_ALIGNED_STORAGE -I..\src -I..\src\ext -I..\src\ext\stb -I..\src\ext\qoi -I..\src\ext\openexr\IlmBase\Imath -I..\src\ext\openexr\IlmBase\Half -I..\src\ext\openexr\IlmBase\Iex -I..\src\ext\openexr\OpenEXR\IlmImf -Isrc\ext\openexr\IlmBase\config -Isrc\ext\openexr\OpenEXR\config -I..\src\ext\zlib -Isrc\ext\zlib -I..\src\ext\libdeflate -I..\src\ext\filesystem -I..\src\ext\ptex\src\ptex -I..\src\ext\double-conversion -I..\src\ext\openvdb\nanovdb -I. -I..\src\ext\glfw\include -I..\src\ext\glad\include /DWIN32 /D_WINDOWS /W3 /GR /EHsc /MDd /Zi /Ob0 /Od /RTC1 /wd4244 /wd4267 /wd4305 /wd4552 /wd4838 /wd4843 /wd26451 /wd26495 /wd4334 -std:c++17 /showIncludes /FoCMakeFiles\pbrt_lib.dir\src\pbrt\textures.cpp.obj /FdCMakeFiles\pbrt_lib.dir\pbrt_lib.pdb /FS -c ..\src\pbrt\textures.cpp ..\src\pbrt\textures.cpp(836): error C2679: binary '=': no operator found which takes a right-hand operand of type 'COLORREF' (or there is no acceptable conversion) C:\Users\anders\code\pbrt-v4\src\pbrt/util/color.h(152): note: could be 'pbrt::RGB &pbrt::RGB::operator =(pbrt::RGB &&)' C:\Users\anders\code\pbrt-v4\src\pbrt/util/color.h(152): note: or 'pbrt::RGB &pbrt::RGB::operator =(const pbrt::RGB &)' ..\src\pbrt\textures.cpp(836): note: while trying to match the argument list '(pbrt::RGB, COLORREF)'
On Sun, 6 Nov 2022 at 08:57, Matt Pharr @.***> wrote:
It would be good to update pbrt to use a modern version of ptex, but unfortunately it's not straightforward. I've pushed a branch, update-ptex, that updates Ptex to v2.4.2 but it causes build breakage on windows: https://github.com/mmp/pbrt-v4/actions/runs/3401367217 (though works elsewhere). I don't have the time dig into the further at the moment, but would happily accept a PR that got it all working!
— Reply to this email directly, view it on GitHub https://github.com/mmp/pbrt-v4/issues/297#issuecomment-1304624241, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOYQXJHWLCAMCHQFH5M3Q3WG23Y7ANCNFSM6AAAAAARD4OT5I . You are receiving this because you are subscribed to this thread.Message ID: @.***>
Hi.. if merge the latest Ptex repo is a problem, one possible alternative is apply only the MinGW patch changes to the current code.
diff --git "a/src/ptex/PtexInt.h" "b/src/ptex/PtexInt.h"
index cae8743..797a111 100644
--- "a/src/ptex/PtexInt.h"
+++ "b/src/ptex/PtexInt.h"
@@ -43,7 +43,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGES.
#if defined(_WIN32) || defined(_WINDOWS) || defined(_MSC_VER)
-#if defined(_MSC_VER) && _MSC_VER >= 1600
+#if defined(_MSC_VER) && _MSC_VER >= 1600 || defined(__MINGW64__)
#include <stdint.h>
#else
typedef __int8 int8_t;
I applied to my local branch and is working fine. Cheers..
@anderslanglands I bet you a dollar that if you add
#ifdef RGB
#undef RGB
#endif
after the last #include
in textures.cpp, the build will work; can you try? On Windows, the windgi.h
header has the lovely habit of #define
ing RGB
to something or other, which causes errors like that.
Assuming @wjakob merges that zlib PR soon, I'll make another try given all of the above and hopefully that will do it.
(@pbrt4bounty I'd rather get up to date with ptex in general rather than just fix this one issue, since pbrt currently is stuck on a pretty old version of it...)
Seems that the CI build fail.. I need to made more test. I'm sorry