Skip to content

Edge Drawing Improvements #2907

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

Merged
merged 1 commit into from
Apr 19, 2021

Conversation

sturkmen72
Copy link
Contributor

@sturkmen72 sturkmen72 commented Apr 2, 2021

#2314

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 other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • 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

documentation preview

force_builders=Linux x64,docs

@sturkmen72 sturkmen72 force-pushed the Edge_Drawing_Improvements branch 8 times, most recently from 0d8a72e to ace1239 Compare April 11, 2021 06:24
@sturkmen72 sturkmen72 force-pushed the Edge_Drawing_Improvements branch 5 times, most recently from 51b69b8 to 693db65 Compare April 13, 2021 08:01
@sturkmen72 sturkmen72 changed the title WIP Edge Drawing Improvements Edge Drawing Improvements Apr 13, 2021
@sturkmen72
Copy link
Contributor Author

@alalek please review

@sturkmen72 sturkmen72 force-pushed the Edge_Drawing_Improvements branch from 693db65 to 98a17f3 Compare April 13, 2021 08:38
@@ -251,6 +332,9 @@ void EdgeDrawingImpl::write(cv::FileStorage& fs) const
EdgeDrawingImpl::EdgeDrawingImpl()
{
params = EdgeDrawing::Params();
nfa = new NFALUT(1, 1/2, 1, 1);
dH = new double[MAX_GRAD_VALUE];
grads = new int[MAX_GRAD_VALUE];
Copy link
Member

Choose a reason for hiding this comment

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

new

Where are these objects destroyed?


dH, grads

Consider using std::vector for these fields.

@@ -337,6 +338,13 @@ EdgeDrawingImpl::EdgeDrawingImpl()
grads = new int[MAX_GRAD_VALUE];
}

EdgeDrawingImpl::~EdgeDrawingImpl()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

is it enough for now ? i am planning to work on optimizing all code later.

@sturkmen72 sturkmen72 force-pushed the Edge_Drawing_Improvements branch 4 times, most recently from 429d82c to f483d27 Compare April 14, 2021 12:21
@sturkmen72 sturkmen72 changed the title Edge Drawing Improvements WIP Edge Drawing Improvements Apr 14, 2021
@sturkmen72 sturkmen72 force-pushed the Edge_Drawing_Improvements branch 2 times, most recently from 0802e7e to f483d27 Compare April 14, 2021 23:16
@sturkmen72 sturkmen72 changed the title WIP Edge Drawing Improvements Edge Drawing Improvements Apr 14, 2021
@sturkmen72 sturkmen72 force-pushed the Edge_Drawing_Improvements branch from f483d27 to 8dad620 Compare April 14, 2021 23:50
@sturkmen72 sturkmen72 changed the title Edge Drawing Improvements WIP Edge Drawing Improvements Apr 15, 2021
Copy link
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Thank you for updates!

Could you please reduce amount of default performance test cases?


/* 4. Allocate and initialize arguments for tested function */
std::string filename = getDataPath("perf/1680x1050.png");
Mat src = imread(filename, 0);
Copy link
Member

Choose a reason for hiding this comment

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

0

Please avoid using of magic numbers. Use IMREAD_GRAYSCALE instead.

int gx = 0;
int gy = 0;

switch (op)
Copy link
Member

Choose a reason for hiding this comment

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

BTW, "constant" mode switch in inner body of two loops may cause some performance loss.

(in the current patch it is just code move, so we can keep it "as is" in this PR)


int sum;

if (SumFlag)
Copy link
Member

Choose a reason for hiding this comment

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

if

similar to "constant" mode switch.

@sturkmen72 sturkmen72 force-pushed the Edge_Drawing_Improvements branch from e9794cc to 42288b6 Compare April 16, 2021 00:08
@sturkmen72 sturkmen72 force-pushed the Edge_Drawing_Improvements branch from 42288b6 to aa850cd Compare April 19, 2021 10:41
@sturkmen72 sturkmen72 changed the title WIP Edge Drawing Improvements Edge Drawing Improvements Apr 19, 2021
Copy link
Member

@alalek alalek 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 👍

@opencv-pushbot opencv-pushbot merged commit f5aeecf into opencv:3.4 Apr 19, 2021
@sturkmen72 sturkmen72 deleted the Edge_Drawing_Improvements branch April 19, 2021 20:29
@alalek alalek mentioned this pull request May 1, 2021
@alalek alalek mentioned this pull request Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants