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

Incorrect calls to cv::Sobel() in PuRe? #1

Open
steven807 opened this issue Sep 18, 2018 · 1 comment
Open

Incorrect calls to cv::Sobel() in PuRe? #1

steven807 opened this issue Sep 18, 2018 · 1 comment

Comments

@steven807
Copy link

steven807 commented Sep 18, 2018

It is hard to be sure with C++ being as unsafe as it is, but it looks like the two calls to cv::Sobel() take one too few arguments. They look like:

Sobel(blurred, dx, dx.type(), 1, 0, 7, 1, BORDER_REPLICATE);
Sobel(blurred, dy, dy.type(), 0, 1, 7, 1, BORDER_REPLICATE);

but the interface to cv::Sobel() is:

Sobel( InputArray src, OutputArray dst, int ddepth,
                         int dx, int dy, int ksize = 3,
                         double scale = 1, double delta = 0,
                         int borderType = BORDER_DEFAULT );

Note that the borderType parameter is in the 8th position, but the calls in PuRe.cpp put it at the 7th position. I'm guessing that there is a missing delta parameter. In OpenCV (3.4.2 at least), BORDER_DEFAULT is 1 (which means the value of delta being used here is 1), and the borderType being used is BORDER_DEFAULT, not BORDER_REPLICATE.

@tcsantini
Copy link
Owner

You are most definitely right: The delta parameter is missing, and as a result, the value 1 (the BORDER_REPLICATE macro) is being used for it and BORDER_DEFAULT for borderType.
Great spotting, thanks!

The consequence is a constant value being added to all the pixels, which doesn't really have any meaningful effect for our case. The fix is just adding delta as zero, i.e.:
Sobel(blurred, dx, dx.type(), 1, 0, 7, 1, 0, BORDER_REPLICATE);
Sobel(blurred, dy, dy.type(), 0, 1, 7, 1, 0, BORDER_REPLICATE);

C++ is not to blame though; blame me for my inattentiveness and OpenCV for not using a proper scoped enumeration for the parameter :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants