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.

Sign up for GitHub

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

Jump to bottom

Radon Transform #3739

Open
1 of 4 tasks
GerhardZangerl opened this issue May 17, 2024 · 1 comment · May be fixed by #3813
Open
1 of 4 tasks

Radon Transform #3739

GerhardZangerl opened this issue May 17, 2024 · 1 comment · May be fixed by #3813
Labels
bug

Comments

@GerhardZangerl
Copy link

System Information

openCV version 4.9.0-dev
Ubuntu 22.04.4 LTS

Detailed description

The Radon Transform provided by openCV changes its src contrary to its documentation. I fixed that behavior locally by changing:

Mat _srcMat = src.getMat() ---> Mat _srcMat = src.getMat().clone();

in the "radon_transform.cc" file.

Steps to reproduce

After the call

double thetastep = 1.0
double  startAngle = -20;
double endAngle = 20;
cv::ximgproc::RadonTransform(matRef, radMatRef, thetaStep, startAngle, endAngle, true);

the cv::Mat matRef seess to be chagen. More detailed: I was using the Radon Transform within a loop when i recognized the behavior.

Issue submission checklist

  • I report the issue, it's not a question
  • I checked the problem with documentation, FAQ, open issues, forum.opencv.org, Stack Overflow, etc and have not found any solution
  • I updated to the latest OpenCV version and the issue is still there
  • There is reproducer code and related data files (videos, images, onnx, etc)
@mshabunin mshabunin transferred this issue from opencv/opencv May 19, 2024
@sturkmen72 sturkmen72 linked a pull request Oct 21, 2024 that will close this issue
Open
6 tasks
@Dugnom
Copy link

Dugnom commented Dec 4, 2024

Where in the documentation does it state, that it is not changed? I do not find that in the current version.

But I do agree, changing the input is not something I expected either.

Instead of transposing the input I'm proposing to change the math a bit to work with the non-transposed version. Instead of projecting the rows, we could project the columns. But I am unsure, if that would hurt performance, since we would use cv::reduce for columns instead of rows, we would have to test it.

Doing my proposed chang would mean that we do neither need to clone the input beforehand, as you propose, nor do we need to store the transposed version in a new Mat as is proposed in #3813

This does work as I show in the PR below. But to keep the sinogram horizontal instead of vertical we have to transpose the output. This will keep the output the same, just to not introduce major (breaking) changes. I don't know if it is ok to have the radon transformatin transposed.

Alternatively we could just improve documentation, clearly stating that the srcMat get's transposed, which is easily reversible in case anyone wants to reuse the srcMat.

I am happy for your input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Parallel implementation for Radon transform sturkmen72/opencv_contrib
2 participants
@GerhardZangerl @Dugnom