Add 1000 and 2500 as trail length viewing options #2051
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 using PR comments)
Other keywords and placeholders
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
🧹 Nitpick comments (1)
sleap/gui/overlays/tracks.py (1)
51-52: Consider adding performance optimization for longer trails.For the new longer trail options (1000, 2500), consider implementing trail point decimation or adaptive sampling to maintain performance while preserving visual quality. This could be especially important when multiple tracks are visible simultaneously.
Example optimization approach:
- For trail_length > 500, sample points at increasing intervals
- Use linear interpolation between sampled points
- Maintain higher sampling rate for recent frames and lower for older frames
Would you like me to propose a specific implementation for this optimization?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sleap/gui/overlays/tracks.py(1 hunks)
🔇 Additional comments (1)
sleap/gui/overlays/tracks.py (1)
51-52: Implementation looks good, but let's verify performance impact.
The addition of 1000 and 2500 as trail length options is implemented correctly. However, these significantly larger values could impact memory usage and rendering performance.
Let's verify the trail rendering performance optimization code:
✅ Verification successful
Let me gather more information about the trail rendering implementation to better understand the performance implications.
Trail length changes are safe with existing performance optimizations
The implementation already includes performance optimizations for longer trails:
- Trails are automatically segmented into 2-3 parts based on length (2 segments for ≤50 frames, 3 segments for longer trails)
- Each segment is rendered with different widths, optimizing both performance and visualization
- The segmentation logic scales proportionally with trail length, handling the new larger values (1000, 2500) efficiently
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any performance optimizations for longer trails
rg -A 5 "trail_length|performance|optimization" sleap/gui/overlays/tracks.py
# Look for any memory management or cleanup code
ast-grep --pattern 'def add_to_scene($params) {
$$$
self.items = []
$$$
}'
Length of output: 1429
Script:
#!/bin/bash
# Check the add_to_scene implementation for performance considerations
ast-grep --pattern 'def add_to_scene($params) {
$$$
}'
# Look for any trail-related performance optimizations
rg -A 10 "seg_count|seg_len" sleap/gui/overlays/tracks.py
Length of output: 1198
| @@ -48,8 +48,8 @@ def __attrs_post_init__(self): | |||
| @classmethod | |||
| def get_length_options(cls): | |||
| if prefs["trail length"] != 0: | |||
There was a problem hiding this comment.
is this conditional the correct thing to check for? won't it always be nonzero if we are using trails?
| @@ -48,8 +48,8 @@ def __attrs_post_init__(self): | |||
| @classmethod | |||
| def get_length_options(cls): | |||
| if prefs["trail length"] != 0: | |||
| return (0, 10, 50, 100, 250, 500, prefs["trail length"]) | |||
| return (0, 10, 50, 100, 250, 500) | |||
| return (0, 10, 50, 100, 250, 500, 1000, 2500, prefs["trail length"]) | |||
There was a problem hiding this comment.
check the lag for these larger numbers
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2051 +/- ##
===========================================
+ Coverage 73.30% 75.51% +2.20%
===========================================
Files 134 133 -1
Lines 24087 24647 +560
===========================================
+ Hits 17658 18612 +954
+ Misses 6429 6035 -394 ☔ View full report in Codecov by Sentry. |
Description
Expanded trail length viewing options as per #2043
Types of changes
Does this address any currently open issues?
Trail Length Viewing Options #2043
Notably, prefs and the 500 viewing option has been added since the issue was created. If those previous changes are deemed sufficient, then this PR is not needed, and we can respond to the issue #2043 thread with that information.
Outside contributors checklist
Thank you for contributing to SLEAP!
❤️
Summary by CodeRabbit
New Features
1000and2500.Bug Fixes