-
Notifications
You must be signed in to change notification settings - Fork 47
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
Precision must be double for JSON #528
Conversation
brayns/common/Transformation.h
Outdated
@@ -37,29 +37,29 @@ class Transformation : public BaseObject | |||
public: | |||
Transformation() = default; | |||
|
|||
Transformation(const Vector3f& translation, const Vector3f& scale, | |||
const Quaternionf& rotation, const Vector3f& rotationCenter) | |||
Transformation(const Vector3d& translation, const Vector3f& scale, |
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.
Shouldn't the scale
and rotationCenter
also use Vector3d
?
brayns/common/Transformation.h
Outdated
Quaternionf _rotation; | ||
Vector3f _rotationCenter{0.f, 0.f, 0.f}; | ||
Vector3d _translation{0., 0., 0.}; | ||
Vector3d _scale{1.f, 1.f, 1.f}; |
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.
Remove .f
?
brayns/common/Transformation.h
Outdated
Vector3d _translation{0., 0., 0.}; | ||
Vector3d _scale{1.f, 1.f, 1.f}; | ||
Quaterniond _rotation; | ||
Vector3d _rotationCenter{0.f, 0.f, 0.f}; |
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.
Remove .f
?
brayns/common/Transformation.h
Outdated
@@ -93,7 +93,7 @@ inline Transformation operator*(const Transformation& a, | |||
a.getRotationCenter()}; | |||
} | |||
|
|||
inline Boxf transformBox(const Boxf& box, const Transformation& trafo) | |||
inline Boxd transformBox(const Boxd& box, const Transformation& trafo) |
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.
What's trafo
? 😄
tests/brayns.cpp
Outdated
|
||
const auto& animParams = pm.getAnimationParameters(); | ||
BOOST_CHECK_EQUAL(animParams.getFrame(), 0); | ||
|
||
const auto& volumeParams = pm.getVolumeParameters(); | ||
BOOST_CHECK_EQUAL(volumeParams.getDimensions(), brayns::Vector3ui(0, 0, 0)); | ||
BOOST_CHECK_EQUAL(volumeParams.getElementSpacing(), | ||
brayns::Vector3f(1.f, 1.f, 1.f)); | ||
brayns::Vector3d(1.f, 1.f, 1.f)); |
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.
Remove .f
?
tests/brayns.cpp
Outdated
BOOST_CHECK_EQUAL(volumeParams.getOffset(), | ||
brayns::Vector3f(0.f, 0.f, 0.f)); | ||
brayns::Vector3d(0.f, 0.f, 0.f)); |
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.
Remove .f
?
b2ca871
to
278ebf4
Compare
JSON comes with infinite precision and if incoming numbers are truncated to float, we can not guarantee to re-broadcast the same numbers again. Float is still supported nonetheless in general, but no longer for PropertyMap.
@@ -33,6 +33,15 @@ namespace brayns | |||
ospSet##OSP(ospObject, prop->name.c_str(), \ | |||
prop->get<std::array<TYPE, NUM>>().data()); \ | |||
break; | |||
#define SET_ARRAY_FLOAT(OSP, NUM) \ |
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.
I think we could make these defines templated functions. For another PR though.
No description provided.