-
Notifications
You must be signed in to change notification settings - Fork 24
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
Allow click prompt for SAM 2 #7993
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for implementing the frontend part so fast @philippotto.
Please find my potential improvements below
frontend/javascripts/oxalis/controller/combinations/tool_controls.ts
Outdated
Show resolved
Hide resolved
frontend/javascripts/oxalis/model/sagas/quick_select_ml_saga.ts
Outdated
Show resolved
Hide resolved
…ossos.DEV.flags.sam.useLocalMask = false)
selectionBottomRightXOpt: Option[Int], | ||
selectionBottomRightYOpt: Option[Int], | ||
pointXOpt: Option[Int], | ||
pointYOpt: Option[Int], | ||
dataShape: Vec3Int, // two of the axes will be 1024, the other is the "depth". Axis order varies depending on viewport |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: remove / adapt comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backend LGTM, excepting the remaining todo
This reverts commit 8362003.
@MichaelBuessemeyer could you have a look at my latest commits? I think, afterwards the PR might be good to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Frontend changes look good and testing worked well.
But I myself cannot approve this pr :)
* backend: add support for point interaction * format backend * fix buffer size calculation of buffer sent to torchserve * support point interaction with sam-based quick select * tmp enable SAM and disable most of ci * improve comments * update nux text * tweak width of nux * incorporate feedback * undo tmp changes * fix busy-mutex violated * make backend accept sam bboxes with width / height less than 1024 * refactor a bit (e.g.,DEV flags) * use a local bounding box for SAM by default (can be toggled via webknossos.DEV.flags.sam.useLocalMask = false) * tune busy blocking msg * refactor WkDevFlags further * remove unused import * tmp: disable some ci steps * update comment in backend about torchserver request params * fix that the padding could make the bbox too large * use viewport extent in correct mag as minimum for mask size * Revert "tmp: disable some ci steps" This reverts commit 8362003. * also mention clicking in quick select tooltip (and docs) * update changelog * add tooltip with hints for sam * improve styling of hint * improve tooltip text --------- Co-authored-by: Philipp Otto <[email protected]> Co-authored-by: Philipp Otto <[email protected]>
URL of deployed dev instance (used for testing):
Steps to test:
Issues:
(Please delete unneeded items, merge only when none are left open)