-
-
Notifications
You must be signed in to change notification settings - Fork 55.7k
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
Imgproc: templmatch: Add support for mask for all methods #15214
Conversation
So I see that some tests fail, suspiciously only on MacOSX, namely exactly all I suspect, that maybe I did not have a deeper look into Other than that I have no idea where these test errors on MacOSX come from. |
@jumostedu Do you have a chance to finish the patch? CI bot reports issues, please take a look. |
I have not found time to try to fix the errors on MacOSX so far, but I will try to have a look again the next week(s). |
187aded
to
5d64332
Compare
5d64332
to
df07c97
Compare
I can not say, what the problem is, that causes the tests to fail on MacOSX, my code does not contain anything platform specific. My guess would be that it's something inside the I also don't have a Mac, so I can really only debug this by looking at the CI results/output here. This is not a great way to do it, so if someone has a Mac and wants to dive into this, they're welcome. I've written short tests to narrow down if the problem is really
Pushing to my branch or does not seem to do it. Neither does adding
some days now. @alalek , how can I retrigger the CI tests? |
@jumostedu I re-started CI for you. CI starts automatically every time when you push changes, so manual triggering usually is not required. |
As an interested party, has this PR seen any movement through CI recently? |
a63ecfe
to
fa9da50
Compare
@alalek can you proceed with review? I resolved the problem with the clang build (I reproduced the original problem on Ubuntu 18.04 clang 9.0.0-2 Release) and removed |
@VadimLevin @alalek are there any open items in the PR? Could we merge it? |
@VadimLevin Thanks for fixing the problem (and prettying up the code :)), I did not realize it was a compiler dependent problem. If you want, I can remove the test for CrossCorr (df07c97) (as it is removed by @VadimLevin 's commit anyway). Or we could leave it in, if someone ever wants to make a test they could use it as reference (if they find it in the git history). |
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.
Thank you for contribution!
Need to take a look on "mask" design more carefully.
/cc @vpisarev
// It does not matter what to use Mat/MatExpr, it should be evaluated to perform assign subtraction | ||
Mat temp_res = img_mask_corr.mul(sum(templx_mask).div(mask_sum)); |
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 believe there is some bug in MatExpr
, need to prepare simple reproducer for tests with similar code.
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.
Should probably be a separate issue. I'm did obviously not resolve this, so you may resolve this conversation yourself :)
@jumostedu Are you planning to finalize this PR and pass the review or should I do this on my own? |
@VadimLevin Yeah, I'll do it within the next few (two or three) days. Thanks for your help! |
fa9da50
to
dba9437
Compare
Add support for masked template matching. Fix/scrub old implementation for masked matching, as it did partly not even really do a meaningful masking, and only supported limited template matching methods. Add documentation including formulas for masked matching.
dba9437
to
342fed9
Compare
Test accuracy by comparing to naive implementation for one point. Test compatibility/correctness by comparing results without mask and with all ones mask. All tests are done for all methods, all supported depths, and for 1 and 3 channels.
Add a test for the crossCorr function in templmatch.cpp. crossCorr() had to be added to exported functions to be testable. This test can maybe help to identify the problem with template matching on MacOSX.
If it should be exported, it should be done as separate PR.
I just realized I can then extend the test to test to using 8U masks with 32F image and template, as well as single channel mask with multichannel image and template, so I did that. |
342fed9
to
43b2b2b
Compare
@jumostedu Do you have a chance to finalize the patch? |
From my side I think I've resolved all open points. If you want me to rebase something, I can do that. Other than that there's a review pending by @vpisarev |
@vpisarev could you look at it again? |
@jumostedu, @alalek, @asmorkalov, sorry for delay. |
I am using 4.4.0 and seeing unexpected behavior. In python:
This yields minMaxLoc[0] to be 0 if mask is None, but yields ~0.00000045 when I generate a mask. |
@anwar6953 OpenCV uses (in most cases) single precision in computations involving floating point numbers and |
Fair enough that it is approximately zero. Thanks for the explanation. |
* imgproc: templmatch: Add support for mask for all methods Add support for masked template matching. Fix/scrub old implementation for masked matching, as it did partly not even really do a meaningful masking, and only supported limited template matching methods. Add documentation including formulas for masked matching. * imgproc: test: Add tests for masked template matching Test accuracy by comparing to naive implementation for one point. Test compatibility/correctness by comparing results without mask and with all ones mask. All tests are done for all methods, all supported depths, and for 1 and 3 channels. * imgproc: test: templmatch: Add test for crossCorr Add a test for the crossCorr function in templmatch.cpp. crossCorr() had to be added to exported functions to be testable. This test can maybe help to identify the problem with template matching on MacOSX. * fix: Fixed wrong evaluations of the MatExpr on Clang * fix: removed crossCorr from public interface. If it should be exported, it should be done as separate PR. Co-authored-by: Vadim Levin <vadim.levin@xperience.ai>
…error Implemented in OpenCV 4.4.0: opencv/opencv#15214
* imgproc: templmatch: Add support for mask for all methods Add support for masked template matching. Fix/scrub old implementation for masked matching, as it did partly not even really do a meaningful masking, and only supported limited template matching methods. Add documentation including formulas for masked matching. * imgproc: test: Add tests for masked template matching Test accuracy by comparing to naive implementation for one point. Test compatibility/correctness by comparing results without mask and with all ones mask. All tests are done for all methods, all supported depths, and for 1 and 3 channels. * imgproc: test: templmatch: Add test for crossCorr Add a test for the crossCorr function in templmatch.cpp. crossCorr() had to be added to exported functions to be testable. This test can maybe help to identify the problem with template matching on MacOSX. * fix: Fixed wrong evaluations of the MatExpr on Clang * fix: removed crossCorr from public interface. If it should be exported, it should be done as separate PR. Co-authored-by: Vadim Levin <vadim.levin@xperience.ai>
resolves #6919
resolves #14076
Current situation
There are multiple requests for supporting a mask in the
matchTemplate()
function, see #14076. Currently only the matching methodsCV_TM_SQDIFF
andCV_TM_SQDIFF
support a mask, and there is justified doubt (#6919) about the correctness.This pullrequest changes
This pull request adds support for masked template matching for all methods, adds the appropriate documentation, as well as tests (which pass). The mask works in an expectable way (the old implementation is scrubbed): The mask is multiplied to the template and to each image patch that is matched.