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

Added support of ellipses #4062

Merged
merged 23 commits into from
Jan 12, 2022
Merged

Added support of ellipses #4062

merged 23 commits into from
Jan 12, 2022

Conversation

bsekachev
Copy link
Member

@bsekachev bsekachev commented Dec 22, 2021

Motivation and context

Resolved #579

ellipses

TODOs:

  • Export import to some formats (<ellipse .. /> in CVAT formats, and masks if the format supports them)
  • Add several API tests to cvat-core
  • Support new shape type in filters

How has this been tested?

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2021 Intel Corporation
#
# SPDX-License-Identifier: MIT

@bsekachev bsekachev added the enhancement New feature or request label Dec 22, 2021
@bsekachev bsekachev changed the title Added support of ellipses [WIP] Added support of ellipses Dec 22, 2021
@nmanovic
Copy link
Contributor

@bsekachev , should we keep the shape size/position unchanged if some part of the shape is outside of a frame? Otherwise it is impossible to annotate some objects which are particularly outside of a frame.

@bsekachev
Copy link
Member Author

@nmanovic

For now for all shapes where it is possible we cut shape if it is out of image. Should we cut ellipses or not is it a question. Probably we should (to keep consistensy), but we also need to add an ability to configure this behaviour for all the shapes.

@nmanovic
Copy link
Contributor

@nmanovic

For now for all shapes where it is possible we cut shape if it is out of image. Should we cut ellipses or not is it a question. Probably we should (to keep consistensy), but we also need to add an ability to configure this behavior for all the shapes.

What is the major difference with rotated bounding boxes? From my point of view an ellipse shape and a rotated bounding box are very similar. I agree that it is better to configure the behavior but for some shapes like a rotated bounding box or an ellipse it is impossible to cut them without conversion to a polygon. Also could you please give me an example when the implemented behavior makes sense?

@bsekachev bsekachev marked this pull request as ready for review January 10, 2022 09:18
@bsekachev bsekachev changed the title [WIP] Added support of ellipses Added support of ellipses Jan 10, 2022
@ActiveChooN
Copy link
Contributor

ActiveChooN commented Jan 11, 2022

Hi,
Ellipse is not clipped for the size of the frame as you show in the gif (not only ellipses btw), but in the develop it works fine
изображение
Will ellipses support rotation as bounding boxes? Looks like it can be helpful in some cases.

@bsekachev
Copy link
Member Author

bsekachev commented Jan 11, 2022

@ActiveChooN

Ellipse is not clipped for the size of the frame as you show in the gif (not only ellipses btw), but in the develop it works fine

For ellipses it was changed according to @nmanovic ideas. GIF just is not updated.
For other shapes I will look at the issue

ActiveChooN
ActiveChooN previously approved these changes Jan 11, 2022
Copy link
Contributor

@ActiveChooN ActiveChooN left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@nmanovic nmanovic left a comment

Choose a reason for hiding this comment

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

@bsekachev , I will recommend to draw an ellipse as it is done by Photoshop and gimp by default. Looks like it is a better way for our scenarios as well. Draw an ellipse and try to annotate it using the current approach when an ellipse is drawing from a center. It will be difficult. Better to fit an ellipse into a virtual bounding box. Thus the interface can be similar to a bounding box but instead of a box, users will get an ellipse fitted into the box.

Need to say that graphical editors support both methods. The current behaviors is supported using ALT+CTRL. But I'm not sure that we need the second method. In real life it is very difficult to find a center of an object.

@nmanovic
Copy link
Contributor

nmanovic commented Jan 11, 2022

@bsekachev , another idea which is similar for rotated bounding boxes. Rotated shapes better to annotate using 4 points.Thus you can draw a rotated bounding box or fit an ellipse into it.

BTW, how to rotate an ellipse?

@klakhov
Copy link
Contributor

klakhov commented Jan 12, 2022

@bsekachev great job! What do you think about quick way to draw a circle? Usually we can hold shift using ellipse tool to draw circles
circle

@bsekachev
Copy link
Member Author

@nmanovic

I will recommend to draw an ellipse as it is done by Photoshop

There was default way to draw ellipses provided by the svg library. Now I redesigned it according to photoshop default way of drawing.

BTW, how to rotate an ellipse?

Actually it was disabled because nobody asked us about rotation of ellipses. Now, If you wish, I enabled it.

another idea which is similar for rotated bounding boxes.

Agree, but it is a different feature where additional design stage is necessary for both bounding boxes and ellipses.

@bsekachev
Copy link
Member Author

@klakhov

That's interesting, but as far as I remember there were not such requests for bounding boxes for years. Maybe nobody needs it? And if we decide to implement the feature, it is probably better to do it for both bounding boxes and ellipses.

@nmanovic
Copy link
Contributor

@klakhov

That's interesting, but as far as I remember there were not such requests for bounding boxes for years. Maybe nobody needs it? And if we decide to implement the feature, it is probably better to do it for both bounding boxes and ellipses.

@bsekachev , @klakhov ,

I agree that it is extremely rare case when you need circle or square. We are annotating real objects, not draw something on the canvas.

P.S. 'bounding box', 'ellipse' should have mostly the same implementation. The only difference is representation (how we are drawing it and export to a file).

Copy link
Contributor

@nmanovic nmanovic left a comment

Choose a reason for hiding this comment

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

LGTM

@nmanovic nmanovic merged commit b85a4ad into develop Jan 12, 2022
@nmanovic nmanovic deleted the bs/ellipses branch January 12, 2022 14:22
@bsekachev
Copy link
Member Author

@TOsmanov @aschernov

Guys, could you add a piece of documentation about the new supported type of shape?

@bsekachev
Copy link
Member Author

bsekachev commented Jan 13, 2022

Hi, @dvkruchinin

Could you please add some tests to ellipses?
Drawing shapes, tracks / rotation / interpolation (I have already added a command to draw ellipses actually and updated test with statistics).

@dvkruchinin
Copy link
Contributor

Hi, @dvkruchinin

Could you please add some tests to ellipses? Drawing shapes, tracks / rotation / interpolation (I have already added a command to draw ellipses actually and updated test with statistics).

Hi @bsekachev,
Sure. A test will be prepared.

@TOsmanov
Copy link
Contributor

@TOsmanov @aschernov

Guys, could you add a piece of documentation about the new supported type of shape?

Hi @bsekachev,
Yes, the documentation will be updated

@mshz88
Copy link

mshz88 commented Jan 30, 2022

please help me, why I can't find the ellipse shape in the web version? how to add the ellipse shape?

@bsekachev
Copy link
Member Author

@mshz88

If you speak about cvat.org, the server was not updated yet. We are working on this, but it takes long time because of significant changes in CVAT. I hope the process will be finished the next week.

@mshz88
Copy link

mshz88 commented Jan 30, 2022

Thank you, @bsekachev CVAT is the best tool among others with many features. We are looking forward to seeing the ellipse shape in the updated version.

@nmanovic nmanovic mentioned this pull request Mar 4, 2022
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Circle/Ellipse-shape annotation
8 participants