Segment Anything: supported bounding box input #8270
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update enhances the Segment Anything functionality by introducing support for bounding box segmentation, allowing for more precise user control. It refines interaction handling and state management across various components, promoting asynchronous operations and improved UI responsiveness. The updates facilitate better annotation processes, making tools more flexible and user-friendly while streamlining workflows in the application. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
changelog.d/20240807_113114_boris_support_sam_rect.md (1)
1-4: Improve grammar and clarity in changelog entry.The current entry is slightly unclear. Consider rephrasing for better readability.
- Segment Anything now supports starts from bounding box setting + Segment Anything now supports starting from a bounding box settingcvat-ui/src/actions/annotation-actions.ts (1)
Function calls to
repeatDrawShapeAsyncdo not handle the new parameter properly.The function
repeatDrawShapeAsynchas been updated to incorporate checks foractiveInteractorParameters. However, the call sites in the following files do not pass any parameters to it:
cvat-ui/src/containers/annotation-page/standard3D-workspace/controls-side-bar/controls-side-bar.tsxcvat-ui/src/containers/annotation-page/standard-workspace/controls-side-bar/controls-side-bar.tsxPlease ensure that all function calls to
repeatDrawShapeAsyncproperly handle the new parameter.Analysis chain
Line range hint
1414-1439:
LGTM! But verify the function usage in the codebase.The changes to incorporate checks for
activeInteractorParametersare approved.However, ensure that all function calls to
repeatDrawShapeAsyncproperly handle the new parameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `repeatDrawShapeAsync` properly handle the new parameter. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type typescript -A 5 $'repeatDrawShapeAsync'Length of output: 83
Script:
#!/bin/bash # Description: Verify all function calls to `repeatDrawShapeAsync` properly handle the new parameter. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ts -A 5 $'repeatDrawShapeAsync'Length of output: 3837
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (14)
- changelog.d/20240807_113114_boris_support_sam_rect.md (1 hunks)
- cvat-canvas/src/typescript/interactionHandler.ts (2 hunks)
- cvat-core/src/core-types.ts (2 hunks)
- cvat-core/src/ml-model.ts (1 hunks)
- cvat-ui/src/actions/annotation-actions.ts (3 hunks)
- cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/opencv-control.tsx (6 hunks)
- cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/tools-control.tsx (13 hunks)
- cvat-ui/src/components/annotation-page/standard-workspace/styles.scss (1 hunks)
- cvat-ui/src/cvat-canvas-wrapper.ts (2 hunks)
- cvat-ui/src/reducers/annotation-reducer.ts (3 hunks)
- cvat-ui/src/reducers/index.ts (1 hunks)
- cvat/apps/lambda_manager/views.py (3 hunks)
- serverless/pytorch/facebookresearch/sam/nuclio/function-gpu.yaml (1 hunks)
- serverless/pytorch/facebookresearch/sam/nuclio/function.yaml (1 hunks)
Additional comments not posted (27)
cvat-ui/src/cvat-canvas-wrapper.ts (2)
27-27: LGTM! The filtering logic is correct.The filtering logic correctly checks both the
buttonandshapeTypeparameters.
Line range hint
17-27:
Ensure the new parametertypeis always provided.The function signature has been updated to include a new parameter
type. Ensure that this parameter is always provided when the function is called.Verification successful
Verification successful: The
typeparameter is always provided when calling theconvertShapesForInteractorfunction.
- Instances found in:
cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/tools-control.tsxcvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/opencv-control.tsxScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `convertShapesForInteractor` include the new `type` parameter. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type typescript -A 5 $'convertShapesForInteractor'Length of output: 89
Script:
#!/bin/bash # Description: Verify all function calls to `convertShapesForInteractor` include the new `type` parameter. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg -t ts -A 5 'convertShapesForInteractor'Length of output: 4348
cvat-core/src/core-types.ts (2)
18-18: LGTM! The new propertystartWithBoxOptionalis correctly added.The new optional property
startWithBoxOptionalhas been correctly added to theModelParamsinterface.
53-53: LGTM! The new propertystartswith_box_optionalis correctly added.The new optional property
startswith_box_optionalhas been correctly added to theSerializedModelinterface.serverless/pytorch/facebookresearch/sam/nuclio/function.yaml (1)
15-15: Enhancement: Addedstartswith_box_optionalconfigurationThe new configuration option
startswith_box_optionalset totrueenhances the flexibility of the interactor by allowing it to function without a starting box. This change improves usability in scenarios where a starting box is not required.serverless/pytorch/facebookresearch/sam/nuclio/function-gpu.yaml (1)
15-15: Enhancement: Addedstartswith_box_optionalconfigurationThe new configuration option
startswith_box_optionalset totrueenhances the flexibility of the interactor by allowing it to function without a starting box. This change improves usability in scenarios where a starting box is not required.cvat-core/src/ml-model.ts (1)
60-60: Enhancement: AddedstartWithBoxOptionalpropertyThe new property
startWithBoxOptionalenhances the flexibility of theMLModelclass by allowing it to handle an optional starting box. This change improves the model's configurability and aligns with the new configuration option introduced in the YAML files.cvat-ui/src/components/annotation-page/standard-workspace/styles.scss (1)
168-177: LGTM!The new CSS class
.cvat-tools-interactor-setupsand its nested styles improve the layout and spacing of components related to tools interactor setups.cvat-ui/src/reducers/index.ts (1)
740-740: LGTM!The addition of the new optional property
activeInteractorParametersto theAnnotationStateinterface enhances the functionality by allowing the state to store parameters related to the currently active interactor.cvat-canvas/src/typescript/interactionHandler.ts (1)
Line range hint
196-211:
LGTM!The changes to the event handling logic for the 'drawstop' event, including the event parameter and additional conditional logic, enhance the functionality by allowing interactions to be contextually dependent on the control key state.
cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/opencv-control.tsx (5)
22-22: Approved: Added import forObjectState.The addition of
ObjectStateis appropriate for managing object states within this component.
61-67: UpdatedDispatchToPropsinterface.The changes to the
DispatchToPropsinterface reflect a shift towards asynchronous handling of actions and a more streamlined approach to state management. The removal of methods likeupdateAnnotationsandchangeFramesuggests a simplification of the interface, while the introduction of asynchronous methods likecreateAnnotationsandfetchAnnotationsimproves the component's responsiveness.
Line range hint
388-390: Approved: UpdatedonTrackingmethod.The updates streamline the logic for handling tracking interactions and improve error handling.
241-241: Approved: UpdatedconvertShapesForInteractorfunction call.The modification to accept a new parameter format likely reflects updates in how shapes are processed during interactions.
However, ensure that all usages of
convertShapesForInteractorare updated to match the new parameter format.Verification successful
Verified: All usages of
convertShapesForInteractormatch the new parameter format.The function calls in
opencv-control.tsxandtools-control.tsxhave been updated consistently to reflect the new parameter format.
cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/opencv-control.tsxcvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/tools-control.tsxScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of `convertShapesForInteractor` match the new parameter format. # Test: Search for the function usage. Expect: Only occurrences of the new parameter format. rg --type tsx -A 5 $'convertShapesForInteractor'Length of output: 75
Script:
#!/bin/bash # Description: Verify all usages of `convertShapesForInteractor` match the new parameter format. # Test: Search for the function usage in all TypeScript files. Expect: Only occurrences of the new parameter format. rg 'convertShapesForInteractor' -t ts -A 5Length of output: 4348
587-591: Approved: UpdatedonInteractionStartmethod.The update to
onInteractionStartto utilizeinteractWithCanvaswith additional parameters enhances the interaction model by allowing more detailed parameters to be passed during canvas interactions.However, ensure that all usages of
interactWithCanvasare updated to match the new parameter requirements.cvat-ui/src/reducers/annotation-reducer.ts (2)
499-499: Approved: Updated state initialization for drawing.Using
defaultState.drawinginstead of spreadingstate.drawingensures that the drawing state resets to its default values whenever this part of the reducer is invoked, which is beneficial for scenarios requiring a fresh start.
Line range hint
933-943: Approved: UpdatedINTERACT_WITH_CANVAScase.The inclusion of
activeInteractorParametersin the payload structure allows the reducer to manage more detailed interaction data with the canvas, enhancing the functionality related to how interactions with the canvas are processed.cvat/apps/lambda_manager/views.py (3)
197-197: Approved: Addedstartswith_box_optionalattribute.The addition of
startswith_box_optionalallows for the configuration of an optional behavior related to thestartswith_boxattribute, enhancing the flexibility of attribute parsing.
218-218: Approved: Updatedto_dictmethod to includestartswith_box_optional.Including the
startswith_box_optionalattribute in the dictionary output ensures that this attribute is correctly represented in the method's output.
388-390: Approved: Simplified logic forpos_pointsand modified handling ofobj_bbox.The simplification of the logic for determining the value of
pos_pointsand the modification of the handling ofobj_bboxstreamline the code by reducing conditional checks and enhancing clarity.cvat-ui/src/actions/annotation-actions.ts (1)
1388-1398: LGTM! But verify the function usage in the codebase.The changes to the function signature are approved.
However, ensure that all function calls to
interactWithCanvasmatch the new signature.cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/tools-control.tsx (6)
147-147: LGTM!The addition of the new state variable
startInteractingWithBoxis approved.
239-239: LGTM!The initialization of the new state variable
startInteractingWithBoxin the constructor is approved.
481-482: LGTM!The inclusion of the new parameter
pos_boxesin the interaction data is approved.
1111-1131: LGTM!The addition of the switch for
startInteractingWithBoxin therenderInteractorBlockmethod is approved.
1148-1159: LGTM!The inclusion of the new parameter
interactorParameterswithstartWithBoxsettings in theonClickhandler is approved.
74-79: LGTM! But verify the function usage in the codebase.The changes to the
DispatchToPropsinterface to return promises are approved.However, ensure that all function calls to these methods properly handle the asynchronous nature.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8270 +/- ##
===========================================
- Coverage 83.39% 83.37% -0.02%
===========================================
Files 390 390
Lines 41551 41560 +9
Branches 3861 3863 +2
===========================================
+ Hits 34650 34651 +1
- Misses 6901 6909 +8
|
|
Motivation and context
Resolved #6281
How has this been tested?
Checklist
developbranch(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.