Page MenuHome
Differential D1185

Optionally use c++11 stuff instead of boost in cycles
ClosedPublic
Actions

Authored by Martijn Berger (juicyfruit) on Mar 15 2015, 11:59 AM.

Details

Reviewers
Thomas Dinges (dingto)
Sergey Sharybin (sergey)
Commits
rCde2e194b436f: Optionally use c++11 stuff instead of boost in cycles where possible. We do and…
rBSf01456aaa4a8: Optionally use c++11 stuff instead of boost in cycles where possible. We do and…
rBf01456aaa4a8: Optionally use c++11 stuff instead of boost in cycles where possible. We do and…
Summary

This is not intended to be proposed for merge in its current condition, rather as a starting point for a debate on the use of boost / c++11 in the cycles project.

Personally I would love to just rip out boost and use the C++11 stuff as:

  1. Our compilers support the things we would need ( gcc 4.6+, clang 3.0+ and msvc 2012+ )
  2. Boost is somehow ugly to me

Also OSL / OIIO do seem to bring in loads of boost stuff.

Diff Detail

Repository
rB Blender

Event Timeline

Martijn Berger (juicyfruit) updated this revision to Diff 3748.Mar 15 2015, 11:59 AM
Martijn Berger (juicyfruit) retitled this revision from to Optionally use c++11 stuff instead of boost in cycles.
Martijn Berger (juicyfruit) updated this object.
Martijn Berger (juicyfruit) added a reviewer: Sergey Sharybin (sergey).
Martijn Berger (juicyfruit) added a subscriber: Cycles.
Martijn Berger (juicyfruit) added a reviewer: Thomas Dinges (dingto).Mar 15 2015, 1:24 PM
Sergey Sharybin (sergey) requested changes to this revision.Mar 15 2015, 4:39 PM
Sergey Sharybin (sergey) edited edge metadata.
Sergey Sharybin (sergey) added inline comments.
intern/cycles/CMakeLists.txt
173

Wrong whitespace, plus what's the reason of making it non-SYSTEM?

intern/cycles/blender/blender_session.cpp
106 ↗(On Diff #3748)

Don't think placeholders should be changed at all. But read below as well.

intern/cycles/device/device.h
75

I'm not really happy with this. Should be either just functiom or we call it util_function, but obviously it should not be #ifdef around variable declaration.

intern/cycles/device/device_task.h
59

Same as above and other parts of the change. Wouldnt duplicate this.

intern/cycles/util/util_function.h
31

Would prefer eiter using std::placeholders or even better using std::placeholders::_[0-9] and zap p_n from the patch all together.

This revision now requires changes to proceed.Mar 15 2015, 4:39 PM
Martijn Berger (juicyfruit) updated this revision to Diff 3752.Mar 15 2015, 11:13 PM
Martijn Berger (juicyfruit) edited edge metadata.

Some changes as per Sergey's suggestions

Also note that on Mac the whole USE_CPP11 thing does not work due to libc++ / libstdc++ issues.

Sergey Sharybin (sergey) added inline comments.Mar 16 2015, 9:12 AM
intern/cycles/util/util_foreach.h
22

That's not totally correct i'm afraid.

Should be

#if (__cplusplus > 199711L) || (defined(_MSC_VER) && _MSC_VER >= 1800)

Now, ideally we should define CYCLES_USE_CPP11 in util_types.h in order to avoid this rather obscure condition happening allover the place, but the issue with this is util_types.h is not guaranteed to be included before other includes.

We could introduce util_compiler.h and such checks/defines into there and include this header before checking for C++11.

Or for now we might just not bother and copy-paste longer ifdef :)

intern/cycles/util/util_map.h
24

This we could also get rid of, see https://developer.blender.org/diffusion/B/browse/depsgraph_refactor/source/blender/depsgraph/util/depsgraph_util_map.h

Could be done as a separate change as well.

intern/cycles/util/util_task.h
31

shouldn't it just become:

typedef function<void(void)> TaskRunFunction;

?

intern/cycles/util/util_thread.h
52
thread(function<void(void)> run_cb_)  ?
83
function<void(void)> run_cb; ?
Sergey Sharybin (sergey) requested changes to this revision.Mar 18 2015, 2:36 PM
Sergey Sharybin (sergey) edited edge metadata.
This revision now requires changes to proceed.Mar 18 2015, 2:36 PM
Martijn Berger (juicyfruit) updated this revision to Diff 3765.Mar 19 2015, 9:59 AM
Martijn Berger (juicyfruit) edited edge metadata.
Martijn Berger (juicyfruit) updated this object.
  • Fixes for Sergey's comments
Martijn Berger (juicyfruit) updated this revision to Diff 3766.Mar 19 2015, 10:22 AM
Martijn Berger (juicyfruit) edited edge metadata.
  • Fix cutting out typedef ..
Sergey Sharybin (sergey) added a comment.EditedMar 25 2015, 3:28 PM

Mainly seems fine to me. Would need to test performance of unordered collections in msvc2013. They used to be really slow in msvc2008, so need to be accurate about this.

EDIT: Sent reply to fast.. I'll check performance on my laptop in a bit.

Sergey Sharybin (sergey) accepted this revision.Mar 28 2015, 11:49 AM
Sergey Sharybin (sergey) edited edge metadata.

Tested std::unordered_map with msvc2013 and can't really tell the difference comparing with boost's ones. Even if there's a speed difference it's not as dramatic as it used to be.

So i think once you compile-check the patch with msvc it could just go in.

This revision is now accepted and ready to land.Mar 28 2015, 11:49 AM
Closed by commit rBf01456aaa4a8: Optionally use c++11 stuff instead of boost in cycles where possible. We do and… (authored by Martijn Berger (juicyfruit)). · Explain WhyMar 29 2015, 10:14 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
intern/
cycles/
21 lines
12 lines
device/
4 lines
10 lines
render/
6 lines
4 lines
util/
8 lines
25 lines
27 lines
8 lines
12 lines
2 lines
21 lines

Diff 3854

View Options

intern/cycles/CMakeLists.txt

Loading...
View Options

intern/cycles/SConscript

Loading...
View Options

intern/cycles/device/device.h

Loading...
View Options

intern/cycles/device/device_task.h

Loading...
View Options

intern/cycles/render/image.h

Loading...
View Options

intern/cycles/render/session.h

Loading...
View Options

intern/cycles/util/util_foreach.h

Loading...
View Options

intern/cycles/util/util_function.h

Loading...
View Options

intern/cycles/util/util_map.h

Loading...
View Options

intern/cycles/util/util_progress.h

Loading...
View Options

intern/cycles/util/util_set.h

Loading...
View Options

intern/cycles/util/util_task.h

Loading...
View Options

intern/cycles/util/util_thread.h

Loading...