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

Intelligent scissors disabling feature #3510

Merged
merged 21 commits into from
Aug 18, 2021
Merged

Conversation

klakhov
Copy link
Contributor

@klakhov klakhov commented Aug 4, 2021

Motivation and context

Related #2800
Related #3383

  • OpenCV intelligent scissors temporarely disabling to create a boundary manually
    image
    blocking

  • Update UI for interactors' blocking mode
    interactors

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

@klakhov klakhov requested a review from bsekachev as a code owner August 4, 2021 09:21
@bsekachev bsekachev self-assigned this Aug 4, 2021
@klakhov klakhov changed the title [WIP] Intelligent scissors disabling feature Intelligent scissors disabling feature Aug 5, 2021
@bsekachev
Copy link
Member

bsekachev commented Aug 5, 2021

Hi, the feature is really nice, but I have some thoughts about implementation.

  1. Looks like I cannot add a point out of the threshold rectangle (when Block is active). We should allow users to put points anywhere this time, and probably we should hide the threshold this time.
    Screenshot from 2021-08-05 15-32-37

  2. Why B? We discussed using Ctrl in both intelligent scissors and interactors. Probably it is better to decrease the variety of hotkeys if possible (like in this case when Block plays the same role). BTW, B is a neighbor of N, which stops drawing. It is also inconvenient.

  3. Could you please add Block button also for interactors?

  4. Let's use <StopOutlined /> icon from antd.

  5. IMHO this tooltip is not informative. Maybe something like: "Postpone running the algorithm [B]" is better. You also can suggest your ideas.
    Screenshot from 2021-08-05 15-40-17

  6. When the cursor is near to the latest point, it disappears and the border shifts. Is it expected?
    Peek 2021-08-05 15-46

@bsekachev bsekachev changed the title Intelligent scissors disabling feature [WIP] Intelligent scissors disabling feature Aug 9, 2021
@bsekachev
Copy link
Member

@klakhov
Could you please remove WIP from the PR title if it is ready?

@klakhov klakhov changed the title [WIP] Intelligent scissors disabling feature Intelligent scissors disabling feature Aug 10, 2021
@klakhov
Copy link
Contributor Author

klakhov commented Aug 10, 2021

@klakhov
Could you please remove WIP from the PR title if it is ready?

Sure

cvat-canvas/src/typescript/interactionHandler.ts Outdated Show resolved Hide resolved
cvat-canvas/src/typescript/interactionHandler.ts Outdated Show resolved Hide resolved
cvat-canvas/src/typescript/interactionHandler.ts Outdated Show resolved Hide resolved
cvat-canvas/src/typescript/interactionHandler.ts Outdated Show resolved Hide resolved
cvat-canvas/src/typescript/interactionHandler.ts Outdated Show resolved Hide resolved
cvat-canvas/src/typescript/interactionHandler.ts Outdated Show resolved Hide resolved
@klakhov klakhov changed the title Intelligent scissors disabling feature [WIP] Intelligent scissors disabling feature Aug 11, 2021
cvat-canvas/src/typescript/interactionHandler.ts Outdated Show resolved Hide resolved
cvat-canvas/src/typescript/interactionHandler.ts Outdated Show resolved Hide resolved
cvat-canvas/src/typescript/interactionHandler.ts Outdated Show resolved Hide resolved
cvat-canvas/src/typescript/interactionHandler.ts Outdated Show resolved Hide resolved
cvat-ui/src/containers/annotation-page/top-bar/top-bar.tsx Outdated Show resolved Hide resolved
cvat-ui/src/utils/opencv-wrapper/intelligent-scissors.ts Outdated Show resolved Hide resolved
cvat-ui/src/utils/opencv-wrapper/intelligent-scissors.ts Outdated Show resolved Hide resolved
@klakhov klakhov changed the title [WIP] Intelligent scissors disabling feature Intelligent scissors disabling feature Aug 13, 2021
@bsekachev
Copy link
Member

Found two issues with this path:

  1. Block using Ctrl does not work (with both scissors and interactors) when start drawing using N hotkey.
  2. When use interactors, we can press Block button. It becomes "blue", but requests are still send to the server.

@bsekachev
Copy link
Member

When try to finish scissors when the algorithm blocked:

IndexSizeError: Failed to execute 'getImageData' on 'CanvasRenderingContext2D': The source width is 0.

@klakhov
Copy link
Contributor Author

klakhov commented Aug 17, 2021

When try to finish scissors when the algorithm blocked:

IndexSizeError: Failed to execute 'getImageData' on 'CanvasRenderingContext2D': The source width is 0

I've returned blocked mode check to final calculation and this error is gone now.

@bsekachev
Copy link
Member

Please add a piece of information to CHANGELOG and update package versions for canvas and ui.

@klakhov klakhov requested a review from nmanovic as a code owner August 17, 2021 13:47
@bsekachev bsekachev merged commit 1da3c96 into develop Aug 18, 2021
@klakhov
Copy link
Contributor Author

klakhov commented Aug 18, 2021

@aschernov @TOsmanov
Could you please update the user guide about using intelligent scissors according to new changes (pressing Ctrl enables blocking mode)?

@dvkruchinin
Could you please prepare a test for intelligent scissors blocking feature?

@TOsmanov
Copy link
Contributor

@aschernov @TOsmanov
Could you please update the user guide about using intelligent scissors according to new changes (pressing Ctrl enables blocking mode)?

@klakhov , yes, we will update the documentation

@dvkruchinin
Copy link
Contributor

@klakhov
Sure. A test will be updated to cover this feature.

@TOsmanov
Copy link
Contributor

I found that when the restrictive threshold (red square) intelligent scissors (OpenCV) is resized using ctrl+mousewheel triggers the block too, is this expected behavior? I think this is inconvenient as it turns out that the user will not be able to change the size of the threshold without enabling blocking

@bsekachev
Copy link
Member

bsekachev commented Aug 19, 2021

@TOsmanov

I found that when the restrictive threshold (red square) intelligent scissors (OpenCV) is resized using ctrl+mousewheel triggers the block too, is this expected behavior?

It is expected.

it turns out that the user will not be able to change the size of the threshold without enabling blocking

It is possible. Why do you think it does not work? (If you use mousewheel during Ctrl is held, block will not be triggered)

@klakhov
Copy link
Contributor Author

klakhov commented Aug 19, 2021

@TOsmanov
As you can see on this gif, when we are resizing threshold using ctrl+mousewheel block remains disabled. But when the ctrl is pressed without threshold resizing block is triggered.
2021-08-19 17-01-22 (online-video-cutter com)

@TOsmanov
Copy link
Contributor

@bsekachev @klakhov , If you release ctrl a little later than scrolling the mouse wheel, the lock is triggered.

@bsekachev
Copy link
Member

@TOsmanov
Could you please provide a .gif where threshold changing happens at the same time with locking?
I cannot reproduce

@TOsmanov
Copy link
Contributor

@bsekachev

block_bug

I'm using the develop branch with the latest commit 0272384

@bsekachev
Copy link
Member

@TOsmanov

Do you have any error message in the browser console this time?

@TOsmanov
Copy link
Contributor

@TOsmanov

Do you have any error message in the browser console this time?

In this moment no.

But I sometimes get an error message in cvat:

OpenCV.js processing error occured
8390056 - Exception catching is disabled, this exception cannot be caught. Compile with -s DISABLE_EXCEPTION_CATCHING=0 > or DISABLE_EXCEPTION_CATCHING=2 to catch.

@klakhov
Copy link
Contributor Author

klakhov commented Aug 23, 2021

@TOsmanov

As we can't easily reproduce this bug, could you please update the documentation and create an issue with more detailed information about your browser/operating system with steps to reproduce this bug?

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.

4 participants