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

Add pixel group and contour expand ops #993

Merged
merged 12 commits into from
May 12, 2021

Conversation

jeffreykuang
Copy link
Contributor

@jeffreykuang jeffreykuang commented Apr 26, 2021

Motivation

The pixel group and contour expand operation are usually used in text detection.
They are now in mmocr, which result in installation difficulty.
Put them in mmcv so that mmocr is compilation-free and can be installed more smoothly.

Modification

Add pixel group op and its unit test.
Add contour expand op and its unit test.

@CLAassistant
Copy link

CLAassistant commented Apr 26, 2021

CLA assistant check
All committers have signed the CLA.

@ZwwWayne ZwwWayne mentioned this pull request Apr 27, 2021
13 tasks
@codecov
Copy link

codecov bot commented Apr 27, 2021

Codecov Report

Merging #993 (ce5f416) into master (1a5bf76) will increase coverage by 0.14%.
The diff coverage is 100.00%.

❗ Current head ce5f416 differs from pull request most recent head aa2b4c5. Consider uploading reports for the commit aa2b4c5 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #993      +/-   ##
==========================================
+ Coverage   64.61%   64.75%   +0.14%     
==========================================
  Files         152      154       +2     
  Lines        9817     9858      +41     
  Branches     1784     1791       +7     
==========================================
+ Hits         6343     6384      +41     
  Misses       3144     3144              
  Partials      330      330              
Flag Coverage Δ
unittests 64.75% <100.00%> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
mmcv/ops/__init__.py 100.00% <100.00%> (ø)
mmcv/ops/contour_expand.py 100.00% <100.00%> (ø)
mmcv/ops/pixel_group.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1a5bf76...aa2b4c5. Read the comment docs.

@ZwwWayne ZwwWayne requested review from ZwwWayne and innerlee April 28, 2021 05:16
@jeffreykuang jeffreykuang changed the title add pixel group ops wip: add pixel group and contour expand ops Apr 29, 2021
@jeffreykuang jeffreykuang changed the title wip: add pixel group and contour expand ops Add pixel group and contour expand ops Apr 29, 2021
@hellock hellock requested a review from grimoire May 10, 2021 08:49
Copy link
Member

@grimoire grimoire left a comment

Choose a reason for hiding this comment

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

Hi
Variable names like q_n kernel_cv p_res are a little hard to understand, better use more meaningful ones.
And loops with a single assign statement could be replaced with std::transform, faster and more readable.

Point2d point(tmp_x, tmp_y);
queue.push(point);
text_line[tmp_x][tmp_y] = label;
is_edge = false;
Copy link
Member

Choose a reason for hiding this comment

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

About this is_edge flag.

The point does not need to be revisited if all direction has been labeled or out of bound. Is that right? Would it be better to avoid feeding these points to next_queue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

given a pixel, if its all neighbors are labelled or out of bound, it indicates it is on edge. Therefore, it is the start point to expanding.

Copy link
Member

Choose a reason for hiding this comment

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

I still don't understand, for example:

label_map=[
[0,1,0],
[1,1,1],
[0,1,0],
]

label_map[1,1] is not on edge but it will be passed to next_queue in every loop cause text_line[tmp_x][tmp_y] > 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(1,1) can not be in the queue. because the queue is initialized with edge pixels and re-initialized with queue_next.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(1,1) can not be in the queue. because the queue is initialized with pixels on edge and re-initialized with queue_next, which are again pixels on edge.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

label_map can not be=[
[0,1,0],
[1,1,1],
[0,1,0],
]
It can be
label_map=[
[0,1,0],
[1,0,1],
[0,1,0],
]

Copy link
Member

Choose a reason for hiding this comment

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

I see, thanks for the explanation.

@jeffreykuang
Copy link
Contributor Author

Hi
Variable names like q_n kernel_cv p_res are a little hard to understand, better use more meaningful ones.
done
And loops with a single assign statement could be replaced with std::transform, faster and more readable.
done

@innerlee
Copy link
Contributor

Kindly ping @hellock

@ZwwWayne ZwwWayne merged commit 2623fbf into open-mmlab:master May 12, 2021
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

Successfully merging this pull request may close these issues.

5 participants