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

[Enhancement] Provide OpenCV backend for visualizer #1142

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Om-Doiphode
Copy link

@Om-Doiphode Om-Doiphode commented May 10, 2023

@HAOCHENYE
Copy link
Collaborator

Hi! Is this PR ready for review? It remains a draft.

@Om-Doiphode
Copy link
Author

Hi @HAOCHENYE , two functions are left to be implemented, I will issue a PR soon.

@Om-Doiphode Om-Doiphode marked this pull request as ready for review May 17, 2023 15:49
@Om-Doiphode
Copy link
Author

Hi @HAOCHENYE, I have issued the PR, please review. Thanks!

@HAOCHENYE HAOCHENYE changed the title Implemented draw_circles and draw_points functions [Enhancement] Provide OpenCV backend for visualizer May 19, 2023
@HAOCHENYE
Copy link
Collaborator

HAOCHENYE commented May 19, 2023

Hi, we should provide the OpenCV backend for Visualizer to draw bboxes, points, .etc, just like the show method does:

Although there will be a lot of if-else logic, it is the only way to make downstream repos can use the OpenCV backend without changing their code (They have already implemented their Visualizer by inheriting the current Visualizer).

@Om-Doiphode
Copy link
Author

Hi @HAOCHENYE, I have made the necessary changes, please review. Thanks!

mmengine/visualization/visualizer.py Show resolved Hide resolved
mmengine/visualization/visualizer.py Show resolved Hide resolved
mmengine/visualization/visualizer.py Show resolved Hide resolved
mmengine/visualization/visualizer.py Outdated Show resolved Hide resolved
mmengine/visualization/visualizer.py Show resolved Hide resolved
mmengine/visualization/visualizer.py Show resolved Hide resolved
mmengine/visualization/visualizer.py Show resolved Hide resolved
mmengine/visualization/visualizer.py Show resolved Hide resolved
mmengine/visualization/visualizer.py Outdated Show resolved Hide resolved
mmengine/visualization/visualizer.py Outdated Show resolved Hide resolved
@Om-Doiphode
Copy link
Author

Hi @HAOCHENYE, I have made the suggested changes, please review. Thanks!

@HAOCHENYE
Copy link
Collaborator

HAOCHENYE commented Jun 1, 2023

Hi, you can update the unittest in test_visualizer.py like this:

from parameterized import parameterized

...

    @parameterized.expand([['cv2'], ['matplotlib']])
    def test_draw_bboxes(self, backend):
        visualizer = Visualizer(image=self.image)

        # only support 4 or nx4 tensor and numpy
        visualizer.draw_bboxes(torch.tensor([1, 1, 2, 2]), backend=backend)
        # valid bbox
        visualizer.draw_bboxes(torch.tensor([1, 1, 1, 2]), backend=backend)
        bboxes = torch.tensor([[1, 1, 2, 2], [1, 2, 2, 2.5]])
        visualizer.draw_bboxes(
            bboxes, alpha=0.5, edge_colors=(255, 0, 0), line_styles='-', backend=backend)
        bboxes = bboxes.numpy()
        visualizer.draw_bboxes(bboxes, backend=backend)

        # test invalid bbox
        with pytest.raises(AssertionError):
            # x1 > x2
            visualizer.draw_bboxes(torch.tensor([5, 1, 2, 2]))

        # test out of bounds
        with pytest.warns(
                UserWarning,
                match='Warning: The bbox is out of bounds,'
                ' the drawn bbox may not be in the image'):
            visualizer.draw_bboxes(torch.tensor([1, 1, 20, 2]))

        # test incorrect bbox format
        with pytest.raises(TypeError):
            visualizer.draw_bboxes([1, 1, 2, 2])

I get some unexpected error when updating the unit test.

@Om-Doiphode
Copy link
Author

Hi @HAOCHENYE, I have updated the tests, please review. Thanks!

@Om-Doiphode
Copy link
Author

Hi @HAOCHENYE, is there anything else which needs to be done for this PR? Can you please elaborate?

@CastleDream
Copy link

Hi @HAOCHENYE, is there anything else which needs to be done for this PR? Can you please elaborate?

Your PR is not run correctly,
image

you need to see the detail and correct
image

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.

None yet

3 participants