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

[IconAnnotator] - Add IconAnnotator to Mark Objects with Custom Icons/Images #930

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

dfreilich
Copy link

Description

This PR adds an IconAnnotator class, which can annotate images. For best results, the icon should be a transparent icon without a background, so that it will come up clearly against any images.

zidane-icon-annotated

Some open questions for discussion:

  1. The demo image for the annotation didn't follow the standard placement for the icon (it seems to be above the image, rather than at the center top border of the detection). I added logic to center the icon horizontally, but I wasn't sure about adding that vertically as well.
  2. To what degree is it important to have color adaption for the icon annotation? It can look very nice, but it also introduces more error cases, for icons that aren't transparent, and will just show up as a colored box as a result.

I'd love to hear your thoughts on it.

A Collab notebook which demonstrates the change is here

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

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

This change includes a unit test (the first created for annotations) to verify that the annotation works, and the result closely matches a so-called golden file

Docs

  • Docs updated? What were the changes:

Closes #460

* Add test for icon annotator, with sample test files to verify
@CLAassistant
Copy link

CLAassistant commented Feb 20, 2024

CLA assistant check
All committers have signed the CLA.

@SkalskiP
Copy link
Collaborator

SkalskiP commented Feb 21, 2024

Hi @dfreilich 👋🏻 ! Thank you very much for your interest in Supervision.

  • Given that we don't yet have a concrete idea for end-to-end testing of annotators, I wouldn't want to introduce it as part of this PR. I kindly ask you to remove the following files from this PR:
    • test/data/zidane.jpg
    • test/data/zidane-icon-annotated.jpg
    • test/data/icons8-diamond-50.png
    • test/annotators/test_core.py
  • Your new annotator has not been included in the documentation. To do this, make the appropriate changes to annotators.md.

supervision/annotators/core.py Outdated Show resolved Hide resolved
supervision/annotators/core.py Outdated Show resolved Hide resolved
supervision/annotators/core.py Outdated Show resolved Hide resolved

# The current positions of the anchors don't account for annotations that
# have mass. This visually centers the anchor, given the size of the icon
x_offset = 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about handling other cases? I assume that in appropriate cases, we should align to the left, to the center, and to the right. Do we also plan to support y_offset?

Copy link
Author

Choose a reason for hiding this comment

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

It was most glaring in the center case, which is why I handled it. I can also handle it in left/right cases as well, to make sure that it's outside of the bounds or on the boundary itself. y_offset we definitely can add (albeit we'll need to be careful to make sure it doesn't run into conflict with boundaries of the image), but I was waiting to get feedback from the project on whether x_offset was desired first before adding it

supervision/annotators/core.py Outdated Show resolved Hide resolved
* Simplify code to remove unnecessary comments
* Make code style and sample code fit with other patterns in file
* Raise exception if invalid icon
@dfreilich
Copy link
Author

@SkalskiP Thanks for the review! I addressed the comments, and removed the unit tests I had created. With regards to the changes in acceptance.md, I assumed that it didn't make sense to add the IconAnnotator to the carousel at the top (like we do here:

=== "BoundingBox"
```python
import supervision as sv
image = ...
detections = sv.Detections(...)
bounding_box_annotator = sv.BoundingBoxAnnotator()
annotated_frame = bounding_box_annotator.annotate(
scene=image.copy(),
detections=detections
)
```
<div class="result" markdown>
![bounding-box-annotator-example](https://media.roboflow.com/supervision-annotator-examples/bounding-box-annotator-example-purple.png){ align=center width="800" }
</div>
) until we have an agreed upon example that's hosted on media.roboflow.com, like the rest of the examples, but I can add one if you'd like.

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.

[IconAnnotator] - mark objects with custom icons / images
3 participants