Skip to content

Aruco javascript fix and added functionality#18985

Merged
alalek merged 19 commits intoopencv:masterfrom
ZEISS:feature/aruco_js_fix
Dec 18, 2020
Merged

Aruco javascript fix and added functionality#18985
alalek merged 19 commits intoopencv:masterfrom
ZEISS:feature/aruco_js_fix

Conversation

@urbste
Copy link
Copy Markdown
Contributor

@urbste urbste commented Dec 1, 2020

Pull Request Readiness Checklist

resolves opencv/opencv_contrib#2526
resolves #15514

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 other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • [ x] 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_only=docs,Custom
build_image:Docs=docs-js
build_image:Custom=javascript
buildworker:Custom=linux-4

@urbste urbste changed the title whitespace Aruco Javascript Fix and added functionality Dec 1, 2020
@urbste urbste changed the title Aruco Javascript Fix and added functionality Aruco javascript fix and added functionality Dec 1, 2020
@urbste
Copy link
Copy Markdown
Contributor Author

urbste commented Dec 1, 2020

Issue: #15514

@urbste
Copy link
Copy Markdown
Contributor Author

urbste commented Dec 1, 2020

Corresponding PR in opencv/opencv_contrib#2769

@dkurt
Copy link
Copy Markdown
Member

dkurt commented Dec 7, 2020

Hi, @urbste, can you please resolve merge conflicts and consider comments? Thansk!

@dkurt
Copy link
Copy Markdown
Member

dkurt commented Dec 15, 2020

Please resolve merge conflicts - rebase with the latest master branch

@urbste
Copy link
Copy Markdown
Contributor Author

urbste commented Dec 15, 2020

How do I resolve the merge conflicts. Sorry I am not that fit with git rebase from a remote? I did

git fetch origin 
git rebase origin/master

However nothing really happened. And locally I do not get the merge conflicts. What am I doing wrong?

@dkurt
Copy link
Copy Markdown
Member

dkurt commented Dec 15, 2020

Try the following:

  1. Make sure that origin is opencv/opencv:
$ git remote -v
  1. Update
git checkout master
git pull origin HEAD
  1. Rebase
git rebase master feature/aruco_js_fix
  1. Resolve conflict in file
  2. git add filename + git rebase --continue
  3. git push fork feature/aruco_js_fix -f

@urbste
Copy link
Copy Markdown
Contributor Author

urbste commented Dec 15, 2020

Thanks for the tips. Still got problems. I did:

git remote -v
origin	https://github.com/urbste/opencv (fetch)
origin	https://github.com/urbste/opencv (push)
upstream	https://github.com/opencv/opencv (fetch)
upstream	https://github.com/opencv/opencv (push)

origin was not opencv/opencv. So I did

git remote set-url origin https://github.com/opencv/opencv
git checkout master
git pull origin HEAD
git rebase master feature/aruco_js_fix
git mergetool
git rebase --continue

However, I only had one conflict in the documentation file, not the emdingen.py as indicated here on github. So something is still wrong I guess... :(

This is not working:

git push fork feature/aruco_js_fix -f
fatal: 'fork' does not appear to be a git repository

Sorry for the inconveniece. :/

@urbste
Copy link
Copy Markdown
Contributor Author

urbste commented Dec 15, 2020

Ok I think I found a solution. I did not rebase but merge the master into my branch.

Copy link
Copy Markdown
Member

@dkurt dkurt left a comment

Choose a reason for hiding this comment

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

👍 Well done!

@dkurt dkurt self-assigned this Dec 15, 2020
@dkurt
Copy link
Copy Markdown
Member

dkurt commented Dec 15, 2020

@urbste, instead of "fork" you had to write your fork name. Anyway, now it's OK.

@urbste
Copy link
Copy Markdown
Contributor Author

urbste commented Dec 15, 2020

@dkurt I do have some more significant changes in the Aruco module, that we would also like to share with the community. I still have forked the repositories in my github account. However I would like to transfer the repositories to my companies github team. Do you know if this PR will be influenced by that?

@dkurt
Copy link
Copy Markdown
Member

dkurt commented Dec 15, 2020

@urbste, aruco module is located in a separate repository. OPENCV_EXTRA_MODULES_PATH can with with any custom path to modules.

@urbste
Copy link
Copy Markdown
Contributor Author

urbste commented Dec 15, 2020

I think this issue is also related to this PR: opencv/opencv_contrib#2526

@alalek alalek merged commit b82700a into opencv:master Dec 18, 2020
@alalek alalek mentioned this pull request Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error Building OpenCV js with Contrib modules build aruco.DetectorParameters in Javascript

3 participants