Skip to content

UI: Don't pass the shapes to the lambda function when continuing tracking #9548

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

SpecLad
Copy link
Contributor

@SpecLad SpecLad commented Jun 19, 2025

Motivation and context

Currently, the UI always passes the current shape points when calling the tracker function. When initializing the tracking state, this makes sense.

However, when continuing tracking, the utility of this is dubious.

  • The point coordinates passed in this way are those for the previous frame; however, the function does not have access to that frame itself. Therefore, it cannot examine the pixel data that the shape covers.

  • In every case, these shapes are data that the function has already seen: it's either the shape that the function received in the initial request, or the shape that the function has output in the previous request. Therefore, if the function really needs this data, it can already ensure access to it by including it into the tracking state.

OTOH, including the shapes has some downsides:

  • It's unnecessary data bloating the request. For rectangles, this isn't a big deal, but for, say, masks the extra data could be significant.

  • It complicates the mental model for the tracker API.

Because of all this, this patch removes the passing of the shapes.

This is a breaking change. However, none of the example trackers in this repository use the previous-frame shapes, and neither does the private SAM2 tracker, so chances are that most 3rd-party trackers won't either. I have included compatibility code that passes a dummy shapes array to the Nuclio function, so that code that relies on its length will still work.

How has this been tested?

I have tried using TransT and SiamMask (and they still work).

Checklist

  • I submit my changes into the develop branch
  • I have created a changelog fragment
  • [ ] I have updated the documentation accordingly
  • [ ] I have added tests to cover my changes
  • [ ] I have linked related issues (see GitHub docs)

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.

…king

Currently, the UI always passes the current shape points when calling the
tracker function. When initializing the tracking state, this makes sense.

However, when continuing tracking, the utility of this is dubious.

* The point coordinates passed in this way are those for the _previous_
  frame; however, the function does not have access to that frame itself.
  Therefore, it cannot examine the pixel data that the shape covers.

* In every case, these shapes are data that the function has already
  seen: it's either the shape that the function received in the initial
  request, or the shape that the function has output in the previous
  request. Therefore, if the function really needs this data, it can already
  ensure access to it by including it into the tracking state.

OTOH, including the shapes has some downsides:

* It's unnecessary data bloating the request. For rectangles, this isn't a
  big deal, but for, say, masks the extra data could be significant.

* It complicates the mental model for the tracker API.

Because of all this, this patch removes the passing of the shapes.

This is a breaking change. However, none of the example trackers in this
repository use the previous-frame shapes, and neither does the private SAM2
tracker, so chances are that most 3rd-party trackers won't either. I have
included compatibility code that passes a dummy `shapes` array to the Nuclio
function, so that code that relies on its _length_ will still work.
@SpecLad SpecLad force-pushed the either-states-or-shapes branch from da8b57a to 139de6f Compare June 19, 2025 14:19
@SpecLad SpecLad marked this pull request as ready for review June 19, 2025 14:19
Copy link

@SpecLad
Copy link
Contributor Author

SpecLad commented Jun 19, 2025

@bsekachev Could you review this?

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 73.78%. Comparing base (7f5263c) to head (139de6f).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #9548      +/-   ##
===========================================
- Coverage    73.79%   73.78%   -0.01%     
===========================================
  Files          448      448              
  Lines        46695    46703       +8     
  Branches      3942     3942              
===========================================
+ Hits         34458    34460       +2     
- Misses       12237    12243       +6     
Components Coverage Δ
cvat-ui 77.75% <ø> (-0.03%) ⬇️
cvat-server 70.69% <75.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@bsekachev bsekachev left a comment

Choose a reason for hiding this comment

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

lgtm

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.

3 participants