Skip to content

Add BackgroundColorAnnotator #1385

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 13 commits into from
Aug 8, 2024
Merged

Conversation

capjamesg
Copy link
Collaborator

Description

This PR adds a new sv.BackgroundColorAnnotator that highlights all regions outside of a list of bounding boxes or masks.

Type of change

  • New feature (non-breaking change which adds functionality)

How has this change been tested, please provide a testcase or example of how you tested the change?

You can test this feature using the following Colab:

https://colab.research.google.com/drive/18GLw4TR-au9DUmAeWU30M7BFJ_00BlxO?usp=sharing

Any specific deployment considerations

N/A

Docs

N/A

@capjamesg capjamesg requested review from yeldarby and SkalskiP July 19, 2024 23:00
@ensure_cv2_image_for_annotation
def annotate(self, scene: ImageType, detections: Detections) -> ImageType:
"""
Annotates the given scene with masks based on the provided detections.
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's explain in more detail what happens. For folks relying on VSCode docs, this it the #1 most important place for documentation.

I'd like to know:

  • what this does. (draws outside boxes and masks)
  • how it treats mask and xyxy (boxes).

@LinasKo
Copy link
Contributor

LinasKo commented Jul 29, 2024

I really like this one - it'd be a key component in a project idea I have.

@capjamesg, I've made some comments. Could you also add a default image to the Colab?
Something like !wget https://media.roboflow.com/notebooks/examples/dog.jpeg should do the trick.

I'll do a deeper code review later on, but it generally looks okay.

)

if detections.mask is None or self.force_box:
for detection_idx in range(len(detections)):
Copy link
Collaborator

Choose a reason for hiding this comment

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

for x1, y1, x2, y2 in detections.xyxy.astype(int) should work as well and be shorter.

@SkalskiP
Copy link
Collaborator

SkalskiP commented Aug 6, 2024

Hi, @capjamesg 👋🏻 would you have time to respond to the comments above?

@capjamesg
Copy link
Collaborator Author

@LinasKo @SkalskiP I have responded to your feedback and added documentation to the detections Annotators documentation page.

@LinasKo
Copy link
Contributor

LinasKo commented Aug 7, 2024

Hey @capjamesg,

Could we get an expanded Colab, please?

Here's the tests I'd like to do:

  1. Test on image with many detections (you have this already)
  2. Test with no detections (you can call sv.Detections.empty()
  3. In-place call of the annotator:
# Instead of:
annotated_image = bounding_box_annotator.annotate(
    scene=image, detections=detections)
    
# Call:
bounding_box_annotator.annotate(
    scene=image, detections=detections)
    
# This should still annotate the image!

Also, since I've seen different behaviours recently, it would be nice to check if both in-place and not in-place work on PIL images. You can use sv.cv2_to_pillow(image) before passing to the model. (or just load with PIL)

@capjamesg
Copy link
Collaborator Author

@LinasKo See my revisions to the Colab and code.

@LinasKo
Copy link
Contributor

LinasKo commented Aug 7, 2024

Close, but to match the behaviour of annotators, it should work in-place for numpy images. I mean, it should modify the image you pass in: https://colab.research.google.com/drive/1SP4QyD_MgvWz9UI6qdkjAIaF74eRoUTy?usp=sharing

Can you modify and return the same image the annotator gets as an argument, instead of returning a copy with Image.fromarray(colored_mask[:, :, ::-1])? np.copyto may or may not be useful - I'm not sure.

Let me know if it gets too much - I'm a bit busy, but could help out 😉

@SkalskiP
Copy link
Collaborator

SkalskiP commented Aug 7, 2024

@LinasKo / @capjamesg, what do you think about renaming it to BackgroundFillAnnotator or BackgroundOverlayAnnotator?

@LinasKo
Copy link
Contributor

LinasKo commented Aug 7, 2024

Yesss, I like the new name much more

@SkalskiP
Copy link
Collaborator

SkalskiP commented Aug 7, 2024

@LinasKo which one? :)

@LinasKo
Copy link
Contributor

LinasKo commented Aug 7, 2024

I misread; I thought you mentioned the previous one too.

BackgroundOverlayAnnotatormakes the most sense to me.

@SkalskiP
Copy link
Collaborator

SkalskiP commented Aug 7, 2024

@LinasKo, is there any chance you could propagate that name change in this PR? We wouldn't need to ask @capjamesg to work on it once again.

@LinasKo
Copy link
Contributor

LinasKo commented Aug 7, 2024

On it.

@LinasKo
Copy link
Contributor

LinasKo commented Aug 7, 2024

Okay, I can take this one over.

The return np.copyto is incorrect, but we're a millimeter away from the solution 😆

@capjamesg, thanks a lot! It's an awesome annotator to have in our toolbox.

@LinasKo
Copy link
Contributor

LinasKo commented Aug 7, 2024

Changed the name, return type.

Tested here: https://colab.research.google.com/drive/1SP4QyD_MgvWz9UI6qdkjAIaF74eRoUTy?usp=sharing

@SkalskiP SkalskiP merged commit 43041c2 into develop Aug 8, 2024
9 checks passed
@onuralpszr onuralpszr deleted the add-backgroundcolor-annotator branch September 23, 2024 15:10
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.

3 participants