Animate a firing only where it can be seen - #9655
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
a8af603 to
c29ba66
Compare
PR SummaryLow Risk Overview Visibility is decided when a firing lands: React Flow’s pan/zoom is read once via Docs add a “What a firing looks like” section describing the culling behavior; unit tests cover pan, zoom floor, and arc bounding boxes. Reviewed by Cursor Bugbot for commit 6f59a6d. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
🟡 Changes recommended
Default custom arcs can remain visible while their endpoint-only bounds incorrectly suppress firing animations.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Optimizes Petrinaut simulation rendering by suppressing firing animations outside the visible viewport or below a zoom threshold.
Changes:
- Adds viewport-aware visibility checks for transition and arc animations.
- Shares transition animation logic across both node styles.
- Adds tests, documentation, and a patch changeset.
File summaries
| File | Description |
|---|---|
.changeset/canvas-firing-animation-budget.md |
Records the performance improvement. |
docs/simulation.md |
Documents animation visibility behavior. |
arc.tsx |
Gates arc firing animations by visibility. |
classic-transition-node.tsx |
Uses the shared animation hook. |
firing-animation-visibility.test.ts |
Tests viewport visibility rules. |
firing-animation-visibility.ts |
Implements visibility calculations. |
transition-node.tsx |
Uses the shared animation hook. |
use-transition-firing-animation.ts |
Centralizes transition firing animation logic. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
c29ba66 to
7d69c07
Compare
Every transition that fires animates its box, its bolt and each of its arcs, and the style engine resolves and paints each of those animations for as long as it runs. On a large net that is hundreds in flight at once, costing more per frame than everything else the canvas does. Most of that work lands where nobody is looking: on nodes off the side of the pane, or on a net drawn small enough that the flash covers a few pixels. Both are now skipped. On a thousand-node net a scrub goes from 13 to 44 frames per second, and playback from 16 to 39.
…d the zoom gate into the visibility check
7d69c07 to
6f59a6d
Compare
Summary
Before this PR, playing or scrubbing a large net cost far more than the canvas draws. Every transition that fires animates its box, its bolt and each of its arcs, and the style engine resolves and paints each of those for as long as it runs. On a thousand-node net that is hundreds of animations in flight per frame, on nodes a few pixels across that nobody is looking at.
A firing now animates only where it can be seen: nodes and arcs off the side of the pane, and any net drawn small enough that a node covers a few pixels, skip the flash. The transitions in view still animate, and those animations are what the remaining frame time goes to when the canvas is zoomed in.
Measured against the built website in headless Chromium, driving a ring net through a run, with frame intervals sampled from
requestAnimationFrame; the last two rows against production builds of the base and this head withsmoke-ring.mjs. Zoomed in, the transitions in view still animate, and those animations are what the remaining frame time goes to.Before
9655-before.mp4
After
9655-after.mp4
Links
Changes
Review fixes
Test coverage
firing-animation-visibility.test.ts:@hashintel/petrinautunit suite.How to test