Details

Reviewers

Brecht Van Lommel (brecht)
Sergey Sharybin (sergey)
Stefan Werner (swerner)
Bastien Montagne (mont29)

Commits

rBd3283ef1210f: Build: upgrade OpenImageDenoise to 1.2.1

Summary

Oh boy, sometimes bumping a version is as easy as changing version.cmake, this is not one of those.

Notes OIDN:

New build time dependency on ISPC, so I added the latest ISPC version
the ISPC version we used defines uint8/16/32 which OIDN also does, so that collided, fixed with an ifdef
OIDN_STATIC_LIB, I don't know where to begin here, it just doesn't work, I can't see how it would work, and can't see how it ever worked. Generally I try to do as few changes as possible and when I do them, and I'll try to stick to something that we may be able to upstream, once more not one of those, I pretty much bluntly changed what was needed to make it build, regardless if it made any sense or not. Out of the box enabling OIDN_STATIC_LIB makes cmake not even finish the configuration phrase fixed in 1.2.1
~~They changed a bunch of their library names~~ fixed in 1.2.1
Not tested with any of the other OS's, and I'm fairly it needs some further tweaks for those.

Notes ISPC

Given it is a build time requirement and is not needed at link time for blender, there is not harvest/post build install stage for it, no libs or binaries need to be added to SVN
ISPC is counting on clang and llvm sharing a root folder, which we don't, so that needed some small tweaks in their CMakeLists.txt
Needed M4 at build time, added to the mingw setup, going on a stretch here and assume every linux/mac will have it

Diff Detail

Repository

rB Blender

Branch

arcpatch-D7641_1 (branched from master)

Build Status

Buildable 8553
Build 8553: arc lint + arc unit

Event Timeline

Ray Molenkamp (LazyDodo) requested review of this revision.May 6 2020, 5:25 PM

Ray Molenkamp (LazyDodo) created this revision.

Harbormaster completed remote builds in B7922: Diff 24456.May 6 2020, 5:25 PM

Ray Molenkamp (LazyDodo) mentioned this in T76184: Library upgrades for 2.90.May 6 2020, 5:25 PM

Ray Molenkamp (LazyDodo) edited the summary of this revision. (Show Details)May 6 2020, 5:45 PM

Given the hassle it is to build static and there's very few upsides, how committed are we to the whole "link what we can statically" ?

Arian (14AUDDIN) awarded a token.May 7 2020, 2:12 PM

Evan Wilson (EAW) awarded a token.May 7 2020, 9:48 PM

noki paike (amonpaike) awarded a token.May 13 2020, 10:32 AM

I'm not particularly committed to it, I'm not even sure it's all that helpful. I'm fine with using dynamic linking more, it will help improve build times as well.

Think the only thing against dynamic linking would be clutter in the binary folder, but D6399 should deal with that nicely.

Ray Molenkamp (LazyDodo) edited the summary of this revision. (Show Details)May 21 2020, 3:32 AM

Fix Linux and macOS build

Harbormaster completed remote builds in B8200: Diff 25078.May 25 2020, 5:44 PM

B (bnzs) awarded a token.Jun 4 2020, 6:25 PM

Seems I forgot to accept this.

This revision is now accepted and ready to land.Jun 5 2020, 1:36 AM

Merge remote-tracking branch 'origin/master' into arcpatch-D7641

Harbormaster completed remote builds in B8389: Diff 25508.Jun 5 2020, 1:46 AM

@Evan Wilson (EAW) pointed me at oidn #76 seconds before i was about to commit this where they mention they are looking to get 1.2.1 out the door this week

figure we may as well wait that out

The issue with OIDN executing an illegal instruction on and crashing older architectures has been patched, which was holding back the 1.2.1 update, according to the linked thread. As the fix is in, we are now back to "We'll release v1.2.1 with the fix in a couple of days."

update to oidn 1.2.1

This revision is now accepted and ready to land.Jun 15 2020, 7:12 PM

Harbormaster completed remote builds in B8553: Diff 25934.Jun 15 2020, 7:12 PM

Ray Molenkamp (LazyDodo) retitled this revision from Update OIDN to 1.2.0 to Update OIDN to 1.2.1.Jun 15 2020, 7:13 PM

Ray Molenkamp (LazyDodo) edited the summary of this revision. (Show Details)

some small changes may be required, since the lib names changed once again, i'll hold off landing till mac/linux had a chance to test.

Stefan Werner (swerner) awarded a token.Jun 16 2020, 10:14 AM

On my Linux system (Ubuntu 1804), make deps fails for ispc with linker errors about missing llvm symbols.

Fix LLVM link errors on Linux

Harbormaster completed remote builds in B8586: Diff 25980.Jun 16 2020, 3:03 PM

Ray Molenkamp (LazyDodo) added inline comments.Jun 16 2020, 3:08 PM

build_files/cmake/Modules/FindOpenImageDenoise.cmake
53	dnnl_cpu doesn't seem to exist on windows any more, is this bit sitll ok on mac/linux?

Brecht Van Lommel (brecht) added inline comments.Jun 24 2020, 6:09 PM

build_files/cmake/Modules/FindOpenImageDenoise.cmake
53	These are not needed anymore with latest OIDN, but will still help when building against an older version. These libraries are automatically skipped if not found, so no harm in keeping them here.

fair enough, i just didn't want to break anything by landing this early, feel free to land this if you're in a rush otherwise i'll do it in the next couple of days.

I'm not in a rush. I was just going over the various lib upgrades for 2.90 to see what remains to be done.

This seems to be the last one for which the cmake builder has not been updated yet.

This revision is now accepted and ready to land.Jun 24 2020, 6:35 PM

Think there's a few linux/mac only deps that have not been updated in the deps builder, see the table in T76184

Closed by commit rBd3283ef1210f: Build: upgrade OpenImageDenoise to 1.2.1 (authored by Ray Molenkamp (LazyDodo), committed by Brecht Van Lommel (brecht)). · Explain WhyJun 25 2020, 1:06 PM

This revision was automatically updated to reflect the committed changes.

Brecht Van Lommel (brecht) added a commit: rBd3283ef1210f: Build: upgrade OpenImageDenoise to 1.2.1.

I needed to make a few additional changes, so just committed this directly.

Ray Molenkamp (LazyDodo) edited the summary of this revision. (Show Details)Jun 25 2020, 4:41 PM

Brecht Van Lommel (brecht) edited the summary of this revision. (Show Details)Jul 9 2020, 5:58 PM

Brecht Van Lommel (brecht) edited the summary of this revision. (Show Details)Jul 10 2020, 2:13 PM

Next time, please add me when update of libs versions are needed... Discovered update of that one by mere chance. And it was closed even without validation of install_deps part???

Also, new OIDN 1.2.1 breaks lbmv tests building apparently, which I find fairly weird?

ninja: error: '/opt/lib/oidn/lib/libmkldnn.a', needed by 'bin/tests/libmv_brute_region_tracker_test', missing and no known rule to make it

Bastien Montagne (mont29) reopened this revision.Aug 10 2020, 6:32 PM

This revision is now accepted and ready to land.Aug 10 2020, 6:32 PM

Bastien Montagne (mont29) requested changes to this revision.Aug 10 2020, 6:32 PM

This revision now requires changes to proceed.Aug 10 2020, 6:32 PM

re-open

In D7641#207752, @Bastien Montagne (mont29) wrote:

Next time, please add me when update of libs versions are needed... Discovered update of that one by mere chance. And it was closed even without validation of install_deps part???

My bad for not adding you, i generally do add you, i did not close this ticket, so i take on responsibility there.

Also, new OIDN 1.2.1 breaks lbmv tests building apparently, which I find fairly weird?
ninja: error: '/opt/lib/oidn/lib/libmkldnn.a', needed by 'bin/tests/libmv_brute_region_tracker_test', missing and no known rule to make it

The library name changed, and cmake cached the old wrong library name, to fix you either toss it out of your CMakeCache.txt manually or start with a brand new build folder.

Bastien Montagne (mont29) abandoned this revision.Aug 11 2020, 6:26 PM

Bastien Montagne (mont29) edited the summary of this revision. (Show Details)

Update OIDN to 1.2.1
AbandonedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 25934

build_files/build_environment/CMakeLists.txt

build_files/build_environment/cmake/ispc.cmake

build_files/build_environment/cmake/openimagedenoise.cmake

build_files/build_environment/cmake/setup_mingw64.cmake

build_files/build_environment/cmake/tbb.cmake

build_files/build_environment/cmake/versions.cmake

build_files/build_environment/patches/ispc.diff

build_files/build_environment/patches/openimagedenoise.diff

build_files/cmake/Modules/FindOpenImageDenoise.cmake

build_files/cmake/platform/platform_win32.cmake

Update OIDN to 1.2.1AbandonedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 25934

build_files/build_environment/CMakeLists.txt

build_files/build_environment/cmake/ispc.cmake

build_files/build_environment/cmake/openimagedenoise.cmake

build_files/build_environment/cmake/setup_mingw64.cmake

build_files/build_environment/cmake/tbb.cmake

build_files/build_environment/cmake/versions.cmake

build_files/build_environment/patches/ispc.diff

build_files/build_environment/patches/openimagedenoise.diff

build_files/cmake/Modules/FindOpenImageDenoise.cmake

build_files/cmake/platform/platform_win32.cmake

Update OIDN to 1.2.1
AbandonedPublic
Actions

Revision Contents
Changeset List