Skip to content
This repository was archived by the owner on Apr 18, 2024. It is now read-only.

Conversation

@BradNeuberg
Copy link
Contributor

@BradNeuberg BradNeuberg commented Dec 16, 2022

This pull request implements a Magic Wand, making it possible to click in a region of an image a user is doing segmentation labeling on, drag the mouse to dynamically change flood filling tolerance, then release the mouse button to get a new labeled area. It is particularly effective at labeling broad, diffuse, complex edged objects, such as clouds, cloud shadows, snow, etc. in earth observation applications or organic shapes in biomedical applications.

A short video of the tool in action can be seen here: https://www.youtube.com/watch?v=XxjqBx_0-wk

Detailed notes on its implementation can be seen at the top of src/tools/MagicWand.js.

Brad Neuberg added 11 commits December 16, 2022 10:01
This PR implements a Magic Wand, making it possible to click in a region of an image a user is doing segmentation labeling on, drag the mouse to dynamically change flood filling tolerance, then release the mouse button to get a new labeled area. It is particularly effective at labeling broad, diffuse, complex edged objects, such as clouds, cloud shadows, snow, etc. in earth observation applications or organic shapes in biomedical applications.

In-depth technical overview on how it works at the top of src/tools/MagicWand.js.
@BradNeuberg
Copy link
Contributor Author

Opened another PR to track this one more time, making a cleaner git history off of master using git cherry-pick to just pull over my changes, now that I've managed conflicts going from 1.6.0 to 1.7.0 release.

@BradNeuberg
Copy link
Contributor Author

Note that examples/image_magic_wand/annotations/1.json points to a Google Cloud bucket for serving up some example images; before landing this we will want to copy these to your public bucket and change the path. Note that some CORS headers will also have to be set on that bucket in order for the Magic Wand to work from demo images served from it; those changes are documented in the START.md file.

@makseq
Copy link
Member

makseq commented Dec 22, 2022

Related feature request: #1118

@makseq makseq added the enhancement New feature or request label Dec 22, 2022
The old bucket was configured under the wrong project, and using https instead of http causes mixed content issues for CORS. Local development is done with http so using that.
bradneuberg added 2 commits January 9, 2023 11:35
This means undo/redo will now work correctly, but will end up creating a new class layer if more magic wanding is done on the current instance.
@BradNeuberg
Copy link
Contributor Author

There was a bug related to drawing several magic wand regions, hitting undo, then attempting to magic wand again -- the offscreen performance cache was incorrectly keeping the old undone region around. I've now fixed this bug with the latest commit to this PR.

@BradNeuberg
Copy link
Contributor Author

I've pushed changes to put the Magic Wand behind a feature flag, though I'm still trying to figure out how to configure the Label Studio server to pick up this new flag. Conversation in Slack here: https://label-studio.slack.com/archives/CQ8LYPPJS/p1673472983241769?thread_ts=1671057042.347029&cid=CQ8LYPPJS

@BradNeuberg
Copy link
Contributor Author

I've taken the QA teams input and made the Magic Wand threshold more robust with better UX. Magic Basically, we now calculate the Magic Wand thresholding from the entire screen, not just relative to the image. We also correctly allow you to drag in any direction to get positive or negative thresholding.

bradneuberg added 2 commits February 7, 2023 11:01
…me tools.

The Magic Wand, the Brush, and the Eraser should all retain selection when moving between them, as well as when moving between those tools and the Zoom or Pan tools.

This fix allows removing some hacky specific code I added for the Magic Wand for this behavior.

Fixes HumanSignal/label-studio#3510.
@BradNeuberg
Copy link
Contributor Author

I've changed the paths of example/test images introduced in this PR to point to the Heartex bucket instead of my own.

@hlomzik hlomzik requested a review from bmartel February 10, 2023 12:43
@hlomzik hlomzik dismissed bmartel’s stale review February 10, 2023 12:44

Brandon already explained that all issue either addressed or existed before you PR

nikitabelonogov and others added 2 commits February 10, 2023 17:52
Erase previously had false, so it should not unselect region in any case.
Tools like Selection, Zoom and others should not unselect it as well.
So set default to false and added true to DrawingTool mixin.
@BradNeuberg
Copy link
Contributor Author

BradNeuberg commented Feb 11, 2023

@hlomzik @bmartel Thanks for the review and approval. I'm still working on:

  • Some integration tests for the brush & magic wand unselection bug I fixed in this PR (where if you are using the brush or magic wand tools, then use the eraser, then go back to the brush tool, you will end up with a new region, rather than having your new brush region get added to the existing one, due to the previous region incorrectly getting unselected). I especially want e2e integration tests for this to prevent it from regressing again (it worked 5 months ago but broke with a change that happened then).
  • Pulling the latest label-studio server code and testing the Magic Wand against that with both the feature flag on and off a final time to ensure things are fine.
  • Re-running all the integration tests to double check everything.

Once that is done, early next week, I'll report back here on this PR and it sounds like we might be able to land it on main?

Currently tests are failing when image in gallery is switched.
These CORS errors can be fixed in separate PR, so had to disable it.
@hlomzik
Copy link
Collaborator

hlomzik commented Feb 13, 2023

Hi, Brad!
The last thing left is e2e failing because of CORS error (you can see this in screenshots attached as artifact to test runs). The problem here is that browser sometimes doesn't send CORS headers (or almost any headers) at all. Easily reproduced case is that one in test — when you select any other image from the gallery (thumbnails under the main image) it'll break with error. Should be checked with our urls, because unlike your GCS with static serving our AWS (and part of our clients') will serve only CORS requests, even if they are wide open/anonymous it still requires CORS headers. And browser doesn't send them on image switch.
My guess is there are some issues with updating image src or drawing context during image substitution, but I haven't been able to find it so far. If you have any ideas, it would be great!
Currently we decided to disable this part in test and merge it in current state. It was tested, there is a huge block of new code, you have no issues with your cloud, it's covered by feature flag and tests (except for this issue) — so it's safe to merge current state and deal with the rest in new PRs.

@hlomzik hlomzik merged commit 18fc2e5 into HumanSignal:master Feb 13, 2023
@BradNeuberg
Copy link
Contributor Author

@hlomzik How are you running the integration tests? I think it might be sensitive to how its run. For my setup, I first start the Label Studio frontend debug setup with npm run start, then in another window I run the integration tests as npm run test:local. This means everything is coming from the same domain, localhost on some port, so I don't have CORS issues and the Magic Wand works fine.

In a true deployment scenario of the integration tests, how are they run? That can give me a pointer to ensure things work for the Magic Wand e2e tests.

@BradNeuberg
Copy link
Contributor Author

FYI I've fixed the S3/CORS integration issue in this PR: #1191

@BradNeuberg
Copy link
Contributor Author

FYI e2e tests for the selection fixes here: #1192

hlomzik added a commit to HumanSignal/label-studio that referenced this pull request Mar 1, 2023
hlomzik added a commit to HumanSignal/label-studio that referenced this pull request Mar 20, 2023
* docs: Magic Wand documentation

After LSF PR HumanSignal/label-studio-frontend#1119
by @BradNeuberg

* Add missing image

* "publicly" spelling
shayantabatabaee pushed a commit to shayantabatabaee/label-studio that referenced this pull request Sep 19, 2023
* docs: Magic Wand documentation

After LSF PR HumanSignal/label-studio-frontend#1119
by @BradNeuberg

* Add missing image

* "publicly" spelling
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request feat

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants