Skip to content
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

Ordinary quaternion #18335

Merged
merged 13 commits into from Nov 19, 2020
Merged

Ordinary quaternion #18335

merged 13 commits into from Nov 19, 2020

Conversation

chargerKong
Copy link
Contributor

@chargerKong chargerKong commented Sep 14, 2020

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

force_builders=linux,Docs,Win64,Mac,Win32,android,ios

@asmorkalov asmorkalov added category: core feature pr: needs test New functionality requires minimal tests set labels Sep 14, 2020
@asmorkalov
Copy link
Contributor

@chargerKong Thanks for the contribution. Could you add tests for your implementation?

modules/core/include/opencv2/core.hpp Outdated Show resolved Hide resolved
@chargerKong chargerKong changed the title Ordinary quaternion [WIT]Ordinary quaternion Sep 15, 2020
@chargerKong chargerKong changed the title [WIT]Ordinary quaternion [WIP]Ordinary quaternion Sep 15, 2020
@chargerKong
Copy link
Contributor Author

Thanks for your review. We will add the corresponding test file in next few days.

@chargerKong
Copy link
Contributor Author

Thanks for your code review. We're carefully fixing these issues and adding test files, which will be done in the next few days. @savuor

@tompollok
Copy link
Contributor

can we have a function for conversion to rodruigez vector and back?

@catree
Copy link
Contributor

catree commented Sep 18, 2020

Typedef for Quat<double> and Quat<float> would be useful, similar to Eigen::Quaterniond and Eigen::Quaterniondf.

@chargerKong
Copy link
Contributor Author

can we have a function for conversion to rodruigez vector and back?

Thanks for your code review, we will add it in next commit

@chargerKong
Copy link
Contributor Author

Typedef for Quat<double> and Quat<float> would be useful, similar to Eigen::Quaterniond and Eigen::Quaterniondf.

OK, Thanks for your suggestions. This will be added in next commit.

@catree
Copy link
Contributor

catree commented Sep 20, 2020

Typedef for Quat<double> and Quat<float> would be useful, similar to Eigen::Quaterniond and Eigen::Quaterniondf.

OK, Thanks for your suggestions. This will be added in next commit.

Maybe only float and double types should be allowed? To avoid user doing something like Quat<int>?
Don't know how this kind of things are handled in other libraries like Eigen.

Related OE 33. 3D Module for Quaternion / dual Quaternion support?

@chargerKong
Copy link
Contributor Author

Typedef for Quat<double> and Quat<float> would be useful, similar to Eigen::Quaterniond and Eigen::Quaterniondf.

OK, Thanks for your suggestions. This will be added in next commit.

Maybe only float and double types should be allowed? To avoid user doing something like Quat<int>?
Don't know how this kind of things are handled in other libraries like Eigen.

Related OE 33. 3D Module for Quaternion / dual Quaternion support?

Yes,Quat<int> is indeed meaningless. This could be stricted by

template <typename T>
    class Quat
    {   
    using value_type = typename std::enable_if<
                    std::is_same<float, T>::value ||
                    std::is_same<double, T>::value,
                    T
                >::type;

This work does relat to OE 33. 3D Module, and we plan to implement dual quaternions after ordinary ones.

modules/core/include/opencv2/core/quaternion.hpp Outdated Show resolved Hide resolved
modules/core/include/opencv2/core/quaternion.hpp Outdated Show resolved Hide resolved
modules/core/include/opencv2/core/quaternion.hpp Outdated Show resolved Hide resolved
modules/core/include/opencv2/core/quaternion.hpp Outdated Show resolved Hide resolved
modules/core/include/opencv2/core/quaternion.hpp Outdated Show resolved Hide resolved
modules/core/include/opencv2/core/quaternion.hpp Outdated Show resolved Hide resolved
modules/core/include/opencv2/core/quaternion.hpp Outdated Show resolved Hide resolved
modules/core/include/opencv2/core/quaternion.hpp Outdated Show resolved Hide resolved
modules/core/include/opencv2/core/quaternion.hpp Outdated Show resolved Hide resolved
modules/core/include/opencv2/core/quaternion.hpp Outdated Show resolved Hide resolved
@chargerKong
Copy link
Contributor Author

chargerKong commented Oct 25, 2020

I'm sorry for not updating code and replying in time resulting from business trip over the last two weeks.
Thanks all for providing valuable comments! I will modify the code in next week. @catree @savuor @VadimLevin @alalek

change some constructor to createFrom* function;
change Rodrigues vector to rotation vector;
change the matexpr to mat of 3x3 return type;
improve comments;
improve docs;
add using std::cos funcs
@chargerKong
Copy link
Contributor Author

@savuor @alalek Can this PR consider merging?

@alalek
Copy link
Member

alalek commented Nov 12, 2020

@chargerKong Well done! Looks good to me.

@savuor
Copy link
Contributor

savuor commented Nov 12, 2020

@chargerKong have just reviewed it; found no issues. If tests are passed, this should be merged.

@chargerKong
Copy link
Contributor Author

OK. Thanks very much! @savuor @alalek

add std::* in affine.hpp,warpers_inl.hpp;
@chargerKong
Copy link
Contributor Author

There is no scheduled status and the rest are all not_queued.

screenshot

But status diaplays here is waiting for builds(This has already waited for a week:sweat_smile:)
1

I think maybe the reason is Linux OpenCL, Win64 OpenCL, Linux x64 Debug and Linux32 are not builded?

How could I start to build it?
@savuor @alalek

@chargerKong
Copy link
Contributor Author

chargerKong commented Nov 16, 2020

The tests are all passed. Can you please proceed with merge? Thanks @alalek @savuor

@savuor
Copy link
Contributor

savuor commented Nov 19, 2020

👍

#include <iostream>
namespace cv
{
//! @addtogroup core
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vpisarev Which documentation group should be used for this?

- avoid non-proper names in `cv::` namespace
- it makes sence to put that into `Quat` class directly, but there are some isses because Quat is template (code becomes too verbose)
Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thank you 👍

@alalek alalek merged commit 11cfa64 into opencv:master Nov 19, 2020
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
Ordinary quaternion

* version 1.0

* add assumeUnit;
add UnitTest;
check boundary value;
fix the func using method: func(obj);
fix 4x4;
add rodrigues vector transformation;
fix mat to quat;

* fix blank and tab

* fix blank and tab
modify test;cpp to hpp

* mainly improve comment;
add rvec2Quat;fix toRodrigues;
fix throw to CV_Error

* fix bug of quatd * int;
combine hpp and cpp;
fix << overload error in win system;
modify include in test file;

* move implementation to quaternion.ini.hpp;
change some constructor to createFrom* function;
change Rodrigues vector to rotation vector;
change the matexpr to mat of 3x3 return type;
improve comments;

* try fix log function error in win

* add enums for assumeUnit;
improve docs;
add using std::cos funcs

* remove using std::* from header;
add std::* in affine.hpp,warpers_inl.hpp;

* quat: coding style

* quat: AssumeType => QuatAssumeType
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants