-
Notifications
You must be signed in to change notification settings - Fork 421
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
replace much boost usage with C++11 equivalents #104
Changes from 1 commit
9703a24
5368a47
65b948e
5b82c3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,6 @@ | ||
# force C++11 mode | ||
add_definitions(-std=c++11) | ||
|
||
if(CMAKE_COMPILER_IS_GNUCXX) | ||
add_definitions(-W -Wall -g -Wextra -Wno-missing-field-initializers -Wno-unused-parameter) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you could just move the -std=c++11 here also. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
endif(CMAKE_COMPILER_IS_GNUCXX) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,10 +38,10 @@ | |
#ifndef FCL_RSS_H | ||
#define FCL_RSS_H | ||
|
||
|
||
#include <math.h> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: I would prefer to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 for |
||
#include "fcl/math/vec_3f.h" | ||
#include "fcl/math/matrix_3f.h" | ||
#include <boost/math/constants/constants.hpp> | ||
|
||
|
||
namespace fcl | ||
{ | ||
|
@@ -111,7 +111,7 @@ class RSS | |
/// @brief Volume of the RSS | ||
inline FCL_REAL volume() const | ||
{ | ||
return (l[0] * l[1] * 2 * r + 4 * boost::math::constants::pi<FCL_REAL>() * r * r * r); | ||
return (l[0] * l[1] * 2 * r + 4 * M_PI * r * r * r); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two problems with
Since "pi" is the only boost math constant FCL used, I think the simplest thing would be to define an fcl::kPi or whatever that could be defined in a common header with something like
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. M_PI caused compilation failures in Visual Studio 2015. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thanks -- I know about that but it's a non-standard hack so I think Mark's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good |
||
} | ||
|
||
/// @brief Size of the RSS (used in BV_Splitter to order two RSSs) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,7 @@ | |
#include "fcl/BVH/BV_splitter.h" | ||
#include "fcl/BVH/BV_fitter.h" | ||
#include <vector> | ||
#include <boost/shared_ptr.hpp> | ||
#include <memory> | ||
#include <boost/noncopyable.hpp> | ||
|
||
namespace fcl | ||
|
@@ -268,10 +268,10 @@ class BVHModel : public CollisionGeometry, | |
BVHBuildState build_state; | ||
|
||
/// @brief Split rule to split one BV node into two children | ||
boost::shared_ptr<BVSplitterBase<BV> > bv_splitter; | ||
std::shared_ptr<BVSplitterBase<BV> > bv_splitter; | ||
|
||
/// @brief Fitting rule to fit a BV node to a set of geometry primitives | ||
boost::shared_ptr<BVFitterBase<BV> > bv_fitter; | ||
std::shared_ptr<BVFitterBase<BV> > bv_fitter; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a change to the public API, should we bump the major version number? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I know, we might don't need to bump the major version number because FCL is still in 0.y.z. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point, we should at least bump the minor version then |
||
|
||
|
||
private: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This causes zillions of warnings on Windows -- that should only be added for gcc & clang.