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

Label color #2014

Merged
merged 25 commits into from
Aug 21, 2020
Merged

Label color #2014

merged 25 commits into from
Aug 21, 2020

Conversation

ActiveChooN
Copy link
Contributor

@ActiveChooN ActiveChooN commented Aug 11, 2020

Motivation and context

This PR helps user to change predefined label color. When creating a label, it is assigned a color from a color map. Also ability to change this color from UI is available.

изображение

Resolve #1480, resolve #1572, closes #842

How has this been tested?

Manually

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) 2020 Intel Corporation
#
# SPDX-License-Identifier: MIT

@ActiveChooN ActiveChooN added the enhancement New feature or request label Aug 11, 2020
cvat/apps/engine/migrations/0026_labelcolor.py Outdated Show resolved Hide resolved
colors[label] = tuple(map(int, color.split(',')))
return colors

def normalize_label(label):
Copy link
Contributor

Choose a reason for hiding this comment

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

The function is not used anymore, but it should be to avoid problems with symbol case and special characters.

The logic is:

  • sort labels to provide stable results (across different tasks with same labels)
  • find matching labels using normalization
  • if have conflicts, the first match uses the table
  • generate random colors for the rest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now colors are set at the time of label creation. So i guess sorting by its name can not provide stable result now. I can not create labels with the same name, so are there any conflicts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Conflicts appear from labels like BACKGROUND, _BA_CK_gro_und_, back_GROUND etc after their normalization. I feel that sorting might be unnecessary anymore, or should be done in label creation code, when it is done in batch mode.

@coveralls
Copy link

coveralls commented Aug 11, 2020

Pull Request Test Coverage Report for Build 7036

  • 55 of 66 (83.33%) changed or added relevant lines in 4 files are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.007%) to 69.729%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cvat/apps/engine/serializers.py 11 12 91.67%
cvat/apps/dataset_manager/formats/utils.py 42 52 80.77%
Files with Coverage Reduction New Missed Lines %
src/labels.js 4 82.86%
Totals Coverage Status
Change from base Build 6998: 0.007%
Covered Lines: 11750
Relevant Lines: 16392

💛 - Coveralls

Copy link
Contributor

@zhiltsov-max zhiltsov-max left a comment

Choose a reason for hiding this comment

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

I feel like some kind of a button to cancel current label color changes could be useful somewhere around the palette

image

Looks good.

zhiltsov-max
zhiltsov-max previously approved these changes Aug 12, 2020
@bsekachev
Copy link
Member

@ActiveChooN

When we change label color on the annotation view (Enter shortcut), is it expected to be changed on a server?
Annotation view color picker still has default preset of colors, are you planning to change it?

bsekachev
bsekachev previously approved these changes Aug 13, 2020
@ActiveChooN ActiveChooN changed the title Label color [WIP] Label color Aug 13, 2020
@nmanovic
Copy link
Contributor

nmanovic commented Aug 17, 2020

@ActiveChooN ,

What is the status of the PR?

Several comments from my side:

  • Need to have a button "OK" when you select a color, "Cancel", and "Reset to default"
  • "Label color" should be replaced by a color pictogram. Color feedback is important.
  • Check with Maxim why road is turquoise by default? It looks unusual.

@ActiveChooN
Copy link
Contributor Author

@nmanovic, finishing work on annotation view. I'm totally agree about "reset to default". But changing color proceed immediately after picking. So I don't think button "Ok" is necessary. What do you think?

@nmanovic
Copy link
Contributor

finishing work on annotation view. I'm totally agree about "reset to default". But changing color proceed immediately after picking. So I don't think button "Ok" is necessary. What do you think?

I think that need to replace "Label color" by a color pictogram for sure. I think that colors for default labels should be reviewed one more time. Also press X isn't intuitive way to choose a color.

@nmanovic
Copy link
Contributor

@ActiveChooN , looks like colors of labels are not matched between the task view and annotation view. Is it something you are working right now?

@nmanovic
Copy link
Contributor

nmanovic commented Aug 17, 2020

Let's add a similar menu:

image

@nmanovic
Copy link
Contributor

Create new task dialog:

  • Use color picker icon instead of “Label color”
  • The color picker should show the current color

Color picker dialog:

  • It should have “OK”, “Cancel” and “Reset” buttons (see Gimp screenshot)

Job view:

  • Use “color by” == “label” by default
  • Use color picker dialog implemented for create new task dialog
  • Use color picker icon to choose a color
  • You can choose color only if “color by” equals to “instance” or “group”. Thus “Enter” and “color picker” is enabled only when “color by” != “label”.
  • Don't save selected colors on the server

@ActiveChooN ActiveChooN changed the title [WIP] Label color Label color Aug 19, 2020

for task in Task.objects.all():
labels = Label.objects.filter(task_id=task.id)
normalized_names = [normalize_label(l.name) for l in labels]
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the code in 3 places. Need to remove code cloning because the logic isn't trivial.

Will Road and road have the same color? Is it OK to have the same color for different classes?

Copy link
Contributor Author

@ActiveChooN ActiveChooN Aug 20, 2020

Choose a reason for hiding this comment

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

@nmanovic, sorry for a long wait. Realized that default python hash function has a random seed every run and colors weren't consistent.

I see the code in 3 places. Need to remove code cloning because the logic isn't trivial.

Done.

Will Road and road have the same color? Is it OK to have the same color for different classes?

If that labels will appear in the task only once they will have the same color. But if label will appear in the task more than one time, for the first label color will be taken from the color map and color will be calculated with hash for others.

@bsekachev
Copy link
Member

In this patch I can change colors (when Color by set to instance) only using shortcut. UI controls don't work.

@bsekachev bsekachev merged commit bee4c37 into develop Aug 21, 2020
@ActiveChooN ActiveChooN deleted the dk/label-color branch August 24, 2020 10:13
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
5 participants