Page MenuHome
Differential D5359

USD Fixes for building on windows.
AbandonedPublic
Actions

Authored by Ray Molenkamp (LazyDodo) on Jul 28 2019, 11:11 PM.

Details

Reviewers
Sybren A. Stüvel (sybren)
Brecht Van Lommel (brecht)
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 4384
Build 4384: arc lint + arc unit

Event Timeline

Ray Molenkamp (LazyDodo) created this revision.Jul 28 2019, 11:11 PM
Harbormaster completed remote builds in B4187: Diff 16642.Jul 28 2019, 11:11 PM
Sybren A. Stüvel (sybren) requested changes to this revision.Jul 30 2019, 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.Jul 30 2019, 4:39 PM
Ray Molenkamp (LazyDodo) added a comment.Jul 30 2019, 10:55 PM
In D5359#121423, @Sybren A. Stüvel (sybren) wrote:

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.

Ray Molenkamp (LazyDodo) updated this revision to Diff 16750.Aug 1 2019, 3:34 AM

removed delayloading

Harbormaster completed remote builds in B4243: Diff 16750.Aug 1 2019, 3:34 AM
Ray Molenkamp (LazyDodo) added a comment.Aug 1 2019, 4:04 AM

separated out the delay load for master in D5387

Brecht Van Lommel (brecht) added a subscriber: Brecht Van Lommel (brecht).Aug 12 2019, 11:22 AM
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?

Ray Molenkamp (LazyDodo) added inline comments.Aug 12 2019, 3:12 PM
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.

Ray Molenkamp (LazyDodo) updated this revision to Diff 17046.Aug 12 2019, 4:10 PM
  • USD/Win: Copy usd registry for debug builds as well
Harbormaster completed remote builds in B4384: Diff 17046.Aug 12 2019, 4:11 PM
Ray Molenkamp (LazyDodo) updated this revision to Diff 17047.Aug 12 2019, 4:12 PM

Retry, fixing last update

Harbormaster completed remote builds in B4385: Diff 17047.Aug 12 2019, 4:12 PM
Ray Molenkamp (LazyDodo) marked an inline comment as done.Aug 12 2019, 4:13 PM

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

Brecht Van Lommel (brecht) accepted this revision.Aug 16 2019, 2:15 PM

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

Sybren A. Stüvel (sybren) added inline comments.Aug 16 2019, 2:19 PM
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?

Ray Molenkamp (LazyDodo) added inline comments.Aug 16 2019, 2:34 PM
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.

Sybren A. Stüvel (sybren) accepted this revision.Nov 21 2019, 5:55 PM
This revision is now accepted and ready to land.Nov 21 2019, 5:55 PM
Sybren A. Stüvel (sybren) mentioned this in D6287: USD: Introducing USD support.Nov 21 2019, 5:58 PM
Ray Molenkamp (LazyDodo) abandoned this revision.Nov 21 2019, 6:00 PM

relevant bits are in D6287

Sybren A. Stüvel (sybren) mentioned this in rBec62413f803e: USD: Introducing a simple USD Exporter.Dec 13 2019, 10:28 AM

Revision Contents
Changeset List

PathSize
5 lines
build_files/
cmake/
Modules/
70 lines
release/
scripts/
startup/
bl_ui/
2 lines
source/
blender/
3 lines
blenkernel/
2 lines
6 lines
intern/
26 lines
depsgraph/
2 lines
editors/
io/
10 lines
7 lines
31 lines
235 lines
space_file/
3 lines
makesdna/
1 line
python/
intern/
4 lines
7 lines
usd/
82 lines
intern/
169 lines
538 lines
230 lines
21 lines
48 lines
127 lines
52 lines
121 lines
16 lines
61 lines
16 lines
61 lines
15 lines
88 lines
41 lines
411 lines
20 lines
62 lines
64 lines
usd/
intern/
editors/
io/
54 lines
creator/
8 lines

Diff 17046

View Options

CMakeLists.txt

Loading...
View Options

build_files/cmake/Modules/FindUSD.cmake

Loading...
View Options

release/scripts/startup/bl_ui/space_topbar.py

Loading...
View Options

source/blender/CMakeLists.txt

Loading...
View Options

source/blender/blenkernel/BKE_anim.h

Loading...
View Options

source/blender/blenkernel/BKE_camera.h

Loading...
View Options

source/blender/blenkernel/intern/camera.c

Loading...
View Options

source/blender/depsgraph/DEG_depsgraph_query.h

Loading...
View Options

source/blender/editors/io/CMakeLists.txt

Loading...
View Options

source/blender/editors/io/io_ops.c

Loading...
View Options

source/blender/editors/io/io_usd.h

Loading...
View Options

source/blender/editors/io/io_usd.c

Loading...
View Options

source/blender/editors/space_file/filelist.c

Loading...
View Options

source/blender/makesdna/DNA_space_types.h

Loading...
View Options

source/blender/python/intern/CMakeLists.txt

Loading...
View Options

source/blender/python/intern/bpy_app_build_options.c

Loading...
View Options

source/blender/usd/CMakeLists.txt

Loading...
View Options

source/blender/usd/intern/abstract_hierarchy_iterator.h

Loading...
View Options

source/blender/usd/intern/abstract_hierarchy_iterator.cc

Loading...
View Options

source/blender/usd/intern/debug_timer.h

Loading...
View Options

source/blender/usd/intern/usd_capi.cc

Loading...
View Options

source/blender/usd/intern/usd_exporter_context.h

Loading...
View Options

source/blender/usd/intern/usd_hierarchy_iterator.h

Loading...
View Options

source/blender/usd/intern/usd_hierarchy_iterator.cc

Loading...
View Options

source/blender/usd/intern/usd_writer_abstract.h

Loading...
View Options

source/blender/usd/intern/usd_writer_abstract.cc

Loading...
View Options

source/blender/usd/intern/usd_writer_camera.h

Loading...
View Options

source/blender/usd/intern/usd_writer_camera.cc

Loading...
View Options

source/blender/usd/intern/usd_writer_hair.h

Loading...
View Options

source/blender/usd/intern/usd_writer_hair.cc

Loading...
View Options

source/blender/usd/intern/usd_writer_light.h

Loading...
View Options

source/blender/usd/intern/usd_writer_light.cc

Loading...
View Options

source/blender/usd/intern/usd_writer_mesh.h

Loading...
View Options

source/blender/usd/intern/usd_writer_mesh.cc

Loading...
View Options

source/blender/usd/intern/usd_writer_transform.h

Loading...
View Options

source/blender/usd/intern/usd_writer_transform.cc

Loading...
View Options

source/blender/usd/usd.h

Loading...
View Options

source/creator/CMakeLists.txt

Loading...