Skip to content

Move (u)int64 typedefs from the global to the cv namespace in C++#25248

Closed
vrabaud wants to merge 7 commits intoopencv:4.xfrom
vrabaud:opencv_js
Closed

Move (u)int64 typedefs from the global to the cv namespace in C++#25248
vrabaud wants to merge 7 commits intoopencv:4.xfrom
vrabaud:opencv_js

Conversation

@vrabaud
Copy link
Copy Markdown
Contributor

@vrabaud vrabaud commented Mar 21, 2024 •
edited
Loading

This solves #7573
The only public API change are:

  • typedefs in modules/core/include/opencv2/core/hal/interface.h are now in the cv namespace in C++
  • in modules/flann/include/opencv2/flann/timer.h

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 another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the 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

@vrabaud vrabaud marked this pull request as draft March 21, 2024 12:48
vrabaud added a commit to vrabaud/opencv_contrib that referenced this pull request Mar 21, 2024
@vrabaud
Use proper C++ types.
38f2f80 
This is necessary to get opencv/opencv#25248
working.
@vrabaud vrabaud mentioned this pull request Mar 21, 2024
Merged
6 tasks
@vrabaud
Copy link
Copy Markdown
Contributor Author

vrabaud commented Mar 21, 2024

opencv/opencv_contrib#3703 needs to be in first.

@vrabaud vrabaud changed the title Remove (u)int64 typedefs from the global namespace in C++ Move (u)int64 typedefs from the global to the cv namespace in C++ Mar 22, 2024
asmorkalov pushed a commit to opencv/opencv_contrib that referenced this pull request Mar 26, 2024
@vrabaud
Merge pull request #3703 from vrabaud:cpp
5300337 
Use proper C++ types. #3703

This is necessary to get opencv/opencv#25248 working.

### Pull Request Readiness Checklist

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

- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
- [x] The PR is proposed to the proper branch
- [x] There is a reference to the 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
vrabaud added a commit to vrabaud/opencv_contrib that referenced this pull request Mar 26, 2024
@vrabaud
Use proper C++ types again.
6b4a01f 
This is necessary to get opencv/opencv#25248 working.
@vrabaud vrabaud mentioned this pull request Mar 26, 2024
Merged
vrabaud added a commit to vrabaud/opencv_contrib that referenced this pull request Mar 26, 2024
@vrabaud
Use proper C++ types again.
9edb0ce 
This is necessary to get opencv/opencv#25248 working.
This was missed in 5300337
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Mar 26, 2024

@vrabaud I merged related patch to contrib. Please go ahead.

@vrabaud
Copy link
Copy Markdown
Contributor Author

vrabaud commented Mar 26, 2024

Thx @asmorkalov , I apparently had missed one on Windows: opencv/opencv_contrib#3705

@vrabaud vrabaud force-pushed the opencv_js branch 2 times, most recently from 05533fe to 9294d7f Compare March 27, 2024 09:56
@vrabaud
Copy link
Copy Markdown
Contributor Author

vrabaud commented Mar 27, 2024

@mshabunin , @asmorkalov , who could help me fix the issue in the objc generator?
I believe I need to add the type into core/misc/objc/gen_dict.json. I could also tweak objc/generator/gen_objc.py. Not sure of what the best solution is. Thx!

rurban pushed a commit to SpexAI/opencv_contrib that referenced this pull request Mar 28, 2024
@vrabaud @rurban
Use proper C++ types again.
7a01c47 
This is necessary to get opencv/opencv#25248 working.
This was missed in 5300337
@mshabunin
Copy link
Copy Markdown
Contributor

mshabunin commented Mar 28, 2024

I don't have enough experience with ObjC bindings. As I can see there is one error currently:

2024-03-27T10:04:24.7188640Z /Users/opencv-cn/GHA-OCV-3/_work/opencv/opencv/ios/build/build-armv7-iphoneos/modules/objc_bindings_generator/ios/gen/objc/dnn/Net.mm:55:103: error: use of undeclared identifier 'uchar'
2024-03-27T10:04:24.7192950Z     cv::Ptr<cv::dnn::Net> retVal = new cv::dnn::Net(cv::dnn::Net::readFromModelOptimizer((std::vector<uchar>&)bufferModelConfig.nativeRef, (std::vector<uchar>&)bufferWeights.nativeRef));
2024-03-27T10:04:24.7195380Z

vector<uchar> is described in the core/misc/objc/gen_dict.json (

opencv/modules/core/misc/objc/gen_dict.json

Lines 387 to 395 in f87e1ef

"vector_uchar": {
"objc_type": "ByteVector*",
"to_cpp": "(std::vector<uchar>&)%(n)s.nativeRef",
"from_cpp": "[ByteVector fromNative:(std::vector<char>&)%(n)s]",
"cast_to": "std::vector<uchar>",
"primitive_vector": true,
"swift_type": "[UInt8]",
"unsigned": true
},
), perhaps it is not used when parsing dnn module. Maybe we can create the same record in the dnn/misc/objc/gen_dict.json?

@vrabaud
Copy link
Copy Markdown
Contributor Author

vrabaud commented Apr 2, 2024

There would be too much of an ABI change because C headers would now refer to cv::uchar. Not worth the risk. This is part of the 5.0 refactoring anyway.

@vrabaud vrabaud closed this Apr 2, 2024
@vrabaud vrabaud deleted the opencv_js branch April 2, 2024 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vrabaud @asmorkalov @mshabunin