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

[Feature] Support CopyPaste for RotatedBoxes #657

Closed
wants to merge 5 commits into from

Conversation

nijkah
Copy link
Contributor

@nijkah nijkah commented Dec 13, 2022

Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily get feedback. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.

Motivation

Closes #82

Modification

  1. Add RBitMasks
  2. Add RCopyPaste

Welcome any suggestions on the current implementation!

Example

dota_1

dota_2

dota_3

@CLAassistant
Copy link

CLAassistant commented Dec 13, 2022

CLA assistant check
All committers have signed the CLA.

Comment on lines +28 to +32
contours = cv2.findContours(self.masks[idx], cv2.RETR_TREE,
cv2.CHAIN_APPROX_SIMPLE)[0][0]
(cx, cy), (w, h), a = cv2.minAreaRect(contours)
rboxes[idx, :] = np.array(
[cx, cy, w, h, np.radians(a)], dtype=np.float32)
Copy link
Contributor Author

@nijkah nijkah Dec 14, 2022

Choose a reason for hiding this comment

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

Here, we need to support mmrotate.core.bbox.poly2obb_np for various angle versions.
However, poly2obb supposes that polygons have 4 vertices.
How can we support this?

Copy link
Collaborator

@zytx121 zytx121 Dec 14, 2022

Choose a reason for hiding this comment

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

In mmrotate1.x, we suppose all rotated boxes use oc angle version during data transforms.
We only need to distinguish different angle versions within the box_head.
So I think it's OK to write like this.

@zytx121
Copy link
Collaborator

zytx121 commented Dec 14, 2022

Excellent job! It's very useful.

@zytx121
Copy link
Collaborator

zytx121 commented Feb 15, 2023

@nijkah Sorry to reply so late. Anyway, I'm back.

The project structure of MMDet and mmrotate has been updated. Please update your code according to https://github.com/open-mmlab/mmdetection/blob/3.x/mmdet/structures/mask/structures.py

I will continue to pay attention to this important PR and hope your enthusiasm will not fade.

@hongxrsysu
Copy link

I use this code, and met a bug:

File "tools/misc/browse_dataset.py", line 110, in
main()
File "tools/misc/browse_dataset.py", line 86, in main
for item in dataset:
File "/root/miniconda3/lib/python3.8/site-packages/mmdet/datasets/dataset_wrappers.py", line 431, in getitem
updated_results = transform(copy.deepcopy(results))
File "/root/miniconda3/lib/python3.8/site-packages/mmdet/datasets/pipelines/transforms.py", line 2830, in call
selected_results = self._select_object(results['mix_results'][0])
File "/root/miniconda3/lib/python3.8/site-packages/mmdet/datasets/pipelines/transforms.py", line 2839, in _select_object
masks = results['gt_masks']
KeyError: 'gt_masks'

I find that "gt_masks" is in the "mmrotate/datasets/pipelines/transforms.py".
image

So how to fix it?

@nijkah
Copy link
Contributor Author

nijkah commented Feb 28, 2023

@hongxrsysu I think it works normally after upgrading mmdet >= 2.26.0.
It supposes that CopyPaste supports generating a pseudo gt_masks when gt_masks is not available.

Please refer to these links.
https://github.com/open-mmlab/mmdetection/releases/tag/v2.26.0
open-mmlab/mmdetection#8905

@nijkah
Copy link
Contributor Author

nijkah commented Feb 28, 2023

@zytx121 The CopyPaste implementation seems quite different between mmdet==2.x and mmdet==3.x.
For example, in mmdet==3.x, it does not support generating pseudo masks from bounding boxes.

Is there any good way to support both mmrotate==1.x and mmrotate==0.x?
Or would it be okay to implement each in a different branch?

@hongxrsysu
Copy link

@hongxrsysu I think it works normally after upgrading mmdet >= 2.26.0. It supposes that CopyPaste supports generating a pseudo gt_masks when gt_masks is not available.

Please refer to these links. https://github.com/open-mmlab/mmdetection/releases/tag/v2.26.0 open-mmlab/mmdetection#8905

Thanks! It works!

@zytx121
Copy link
Collaborator

zytx121 commented Mar 24, 2023

@zytx121 The CopyPaste implementation seems quite different between mmdet==2.x and mmdet==3.x. For example, in mmdet==3.x, it does not support generating pseudo masks from bounding boxes.

Is there any good way to support both mmrotate==1.x and mmrotate==0.x? Or would it be okay to implement each in a different branch?

Hi @nijkah!
As far as I know, there is currently no way to support two branches at the same time. It is okay to implement each in a different branch. Due to a shortage of personnel, mmrotate0. x has actually stopped maintenance since 2023. So the priority of 1. x will increase.

In 1. x, projects\ provides a quick iterative code solution that does not require strict unit testing and documentation.

@nijkah
Copy link
Contributor Author

nijkah commented Jun 14, 2023

Hi, @zytx121. Apologize for the slow progress.
Recently. I create a PR #877 for MMRotate=1.x. 😄

I'll close this PR.

@nijkah nijkah closed this Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants