Skip to content

perf(rendering): combine caption active and fade lookup - #2537

Open
KrishCodesw wants to merge 5 commits into
CapSoftware:mainfrom
KrishCodesw:perf/caption-lookup-single-pass
Open

KrishCodesw wants to merge 5 commits into
CapSoftware:mainfrom
KrishCodesw:perf/caption-lookup-single-pass

Conversation

@KrishCodesw

@KrishCodesw KrishCodesw commented Oct 14, 2026 •

Copy link
Copy Markdown

Summary

  • Combine the active-caption and exit-animation lookup loops into one pass.
  • Preserve active-caption priority and the first matching exit-animation segment in input order.
  • Add regression tests for overlapping and unsorted segments, exact boundaries, and a differential comparison with the previous two-pass implementation.
  • Add an ignored release-mode microbenchmark for active, fading, gap, and after-track lookups.

Closes #2523.

Benchmark

The benchmark is included but has not been run in this environment: Rust/Cargo is not installed, and the container cannot reach GitHub to clone the repository. I have not recorded or claimed a performance gain.

Run from the repository root:

cargo test -p cap-rendering benchmark_caption_lookup_before_and_after --release -- --ignored --nocapture

The output compares median lookup time for the original two-pass implementation and the new implementation across 2,400 caption segments. Please keep this PR as a draft until the results confirm a useful gain and no unacceptable regression for active-caption lookups.

Validation

  • Added a differential test over overlapping, unsorted segments and multiple fade overrides.
  • Added explicit tests for active-over-fading priority, input order, and active/fade boundaries.
  • Rust tests and benchmark execution remain unverified in this environment.

RetriggerConfidence Score: 5/5

No blocking code issues remain; keep the PR draft until the benchmark meets the author's stated release condition.

Summary

Combines active-caption and fade lookup into one pass while preserving selection order.

  • Caption rendering checks active and fading segments in one scan.

Reviews (2) · Last reviewed commit: "style(rendering): apply rustfmt to capti..." · Reviewed by Greptile

@vercel

vercel Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

@KrishCodesw is attempting to deploy a commit to the Cap Software Inc Team on Vercel.

A member of the Team first needs to authorize it.

@KrishCodesw
KrishCodesw marked this pull request as ready for review October 14, 2026 01:38
Comment thread crates/rendering/src/layers/captions.rs Outdated
Comment thread crates/rendering/src/layers/captions.rs Outdated
KrishCodesw and others added 2 commits October 14, 2026 07:12
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@richiemcilroy

Copy link
Copy Markdown
Member

Thanks for the PR. Greptile currently has this at 4/5, and we only merge PRs once Greptile scores the current head 5/5. Please address the remaining Greptile feedback, then comment @greptileai to request a re-review, and repeat until it reaches 5/5.

Copy link
Copy Markdown
Author

@greptileai Please re-review the current PR head and update the score. The redundant test comment is absent from the current source, and the Rust formatting changes are committed in 65f56c3.

Copy link
Copy Markdown
Author

@richiemcilroy Thanks! Greptile now scores the latest PR head 5/5. I've addressed the review feedback. Let me know if anything else is needed.

@KrishCodesw

Copy link
Copy Markdown
Author

Hey @richiemcilroy , just wanted to follow up on this PR and check if there are any updates.
Thanks!

This branch has not been deployed

No deployments
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.

perf: investigate redundant caption lookup passes during rendering

2 participants