Page MenuHome

USD Fixes for building on windows.
Needs ReviewPublic

Authored by LazyDodo (LazyDodo) on Sun, Jul 28, 11:11 PM.

Details

Summary

Mostly minor stuff, due to the use of thread_local in usd, i had to delay load opengl32 since when you use mesa it'll spin off a bunch of threads before the crt on the main thread is initialized leading to a crash due to uninitialized locks.

debug_timer.h used some none ISO code which MSVC didn't support and it just stubbed it out for windows.

Diff Detail

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

Event Timeline

Sybren A. Stüvel (sybren) requested changes to this revision.Tue, Jul 30, 4:39 PM

Thanks for the patch.

due to the use of thread_local in usd, i had to delay load opengl32 since when you use mesa it'll spin off a bunch of threads before the crt on the main thread is initialized leading to a crash due to uninitialized locks.

Can you add this to a comment in the file? That way this knowledge doesn't get lost.

debug_timer.h used some none ISO code which MSVC didn't support and it just stubbed it out for windows.

That's fine, the code will probably be removed before merging to master.

This revision now requires changes to proceed.Tue, Jul 30, 4:39 PM

due to the use of thread_local in usd, i had to delay load opengl32 since when you use mesa it'll spin off a bunch of threads before the crt on the main thread is initialized leading to a crash due to uninitialized locks.

Can you add this to a comment in the file? That way this knowledge doesn't get lost.

I'd rather land that bit (with comment) of the patch in master then, the change is not USD related, any C++11 code that uses thread_local can trigger the problem. USD is just the first to run into it.

removed delayloading

separated out the delay load for master in D5387

Brecht Van Lommel (brecht) added inline comments.
source/creator/CMakeLists.txt
1053

What is this and why is it not needed for debug builds and non-Windows platforms?

source/creator/CMakeLists.txt
1053

It's a bunch of json USD insists on having, currently windows only, since i'm not sure how other platforms deal with this, debug is left out since it's a sloppy copy/paste job.

  • USD/Win: Copy usd registry for debug builds as well

Retry, fixing last update

LazyDodo (LazyDodo) marked an inline comment as done.Mon, Aug 12, 4:13 PM

Fixed debug configuration missing the registry, left it for win32 since it's unknown how other platforms deal with this

Looks good to me but this is for @Sybren A. Stüvel (sybren) to handle really.

source/creator/CMakeLists.txt
1053

Nitpick for comment style: # USD registry

source/blender/usd/CMakeLists.txt
51

Why is this necessary? Can we use something that's given to us by ${USD_PATH}/pxrConfig.cmake to configure the required definitions?

source/blender/usd/CMakeLists.txt
51

usd drags in the tbb headers and tbb in turn drags in windows.h which by default defines min and max macro's causing issues with c++'s std::min and std::max unless you define this symbol.

Given USD technically doesn't cause the issue we can't really expect ${USD_PATH}/pxrConfig.cmake to fix it.

One option would be to define it at a global scope, but there are some deps that do fix this issue them selves and that'll cause annoying NOMINMAX redefinition warns, which aren't great either.

soo yeah.. we're kinda stuck with sprinkling this nonsense wherever we need it on a case by case basis.