Skip to content

fix(desktop): stop idle renderer animation loops - #66160

Closed
andyylin wants to merge 6 commits into
NousResearch:mainfrom
andyylin:fix/desktop-idle-rendering
Closed

andyylin wants to merge 6 commits into
NousResearch:mainfrom
andyylin:fix/desktop-idle-rendering

Conversation

@andyylin

Copy link
Copy Markdown
Contributor

Summary

Hermes deliberately disables Chromium's background throttling so an answer keeps painting while its window is blurred or covered. That workaround is load-bearing, but it also exposed three decorative/layout loops that ran at the display refresh rate forever:

  • PetSprite requested RAF continuously although the sprite changes only about 5–6 times/sec.
  • usePetRoam requested RAF during idle dwell periods, not only while the pet was moving.
  • PersistentTerminal polled an unchanged slot rectangle every frame.

On a 120 Hz Mac, the terminal alone scheduled about 1,200 callbacks per 10 seconds; enabling the pet doubled that to about 2,400. Fully occluding the window did not stop them because background rendering is intentionally unthrottled.

This PR keeps that streaming workaround and makes decorative/layout work bounded instead:

  • schedule sprite wakes at the actual frame cadence and draw only when the visible cell changes;
  • use timers while roaming is dwelling and RAF only for real movement physics;
  • replace permanent terminal rect polling with resize, scroll, and layout-mutation invalidation plus a settling frame;
  • pause main-window decorative work while hidden, minimized, or unfocused;
  • keep the deliberately non-activating pop-out pet animated while it is visible;
  • flush streamed message deltas on the existing bounded cadence with a timer rather than background-sensitive RAF.

This builds on and preserves the authorship of @HOYALIM's earlier work in #61084.

Why the anti-throttling remains

backgroundThrottling: false and the renderer backgrounding switches were added because Chromium otherwise pauses/clamps renderer scheduling when a user switches to another app or covers Hermes. A chat response would appear to stall and then catch up on refocus.

This PR does not remove or relax those flags. Transcript flushing is decoupled from RAF and remains active on a bounded timer. Decorative loops independently subscribe to visibility/focus/Electron window-state signals and sleep when they have nothing useful to paint.

macOS measurements

Measured on Apple Silicon, macOS 26.5.2. Process CPU uses fixed 10-second SystemInfo.getProcessInfo.cpuTime deltas; RAF sites were instrumented through CDP. Before and after used the same Electron version, machine, harness, window states, and duration.

The installed production app reproduced the reported symptom directly:

Installed app, pet enabled Renderer GPU helper
Visible and focused 45.8% 31.5%
Visible but unfocused 42.6% 29.8%
Fully occluded 44.1% 29.6%

A controlled renderer harness isolated the changed paths:

10-second idle condition Before renderer / GPU After renderer / GPU Before RAF callbacks After RAF callbacks
Pet off, visible 8.15% / 3.53% below 0.01% / below 0.01% 1,200 0
Pet off, unfocused 8.18% / 3.56% below 0.01% / 0.01% 1,200 0
Pet off, fully occluded 5.23% / 1.90% below 0.01% / below 0.01% 1,199 0
Pet on, visible 8.05% / 3.37% 2.52% / 1.54% 2,402 60
Pet on, unfocused 8.70% / 3.71% below 0.01% / below 0.01% 2,400 0
Pet on, fully occluded 6.73% / 2.82% below 0.01% / below 0.01% 2,398 0

The remaining 60 visible pet callbacks per 10 seconds match the sprite's real animation cadence instead of the 120 Hz display.

Background streaming verification

A synthetic stream emitted 250 deltas at 25 deltas/sec, which exercises the old RAF path because each delta arrives after the 33 ms flush floor.

Condition Before transcript RAF After transcript RAF Final expected length
Visible 241 0 1,500 / 1,500
Unfocused 249 0 1,500 / 1,500
Fully occluded 0 1,500 / 1,500

The patched build reached the full transcript length while visible, unfocused, and fully covered. Background streaming therefore remains uninterrupted while decorative work stays asleep.

Tests

Added coverage for:

  • background delta flushing without RAF;
  • timer cancellation and a final single flush on unmount;
  • sprite cadence, pause/resume, and RAF/timer cleanup;
  • the intentionally unfocused pop-out overlay exemption;
  • roam dwell timers, pause/resume, and cleanup;
  • terminal settling, nested layout invalidation, focus/window-state suspension, and unmount cleanup;
  • no initial terminal RAF when mounted unfocused;
  • backgroundThrottling: false on all chat windows.

Verification performed:

  • npm run typecheck
  • npm run lint — 0 errors (repository warnings remain)
  • focused UI tests — 14 passed
  • Electron project — 432 passed, 1 skipped
  • npm run build — renderer, Electron main/preload, native staging, and dist assertion passed
  • git diff --check
  • full npm test — 1,785 passed, 1 skipped; three unrelated SkillsView tests flaked under full-suite load and passed 3/3 when rerun alone

An independent review found no blocking issues. Its two findings—nested terminal layout observation and the initial-unfocused mount race—were fixed and covered before submission.

Tradeoffs / scope

  • A visible, focused pet still animates at its configured cadence.
  • Roaming still uses RAF while the mascot is physically moving; idle dwell uses a bounded timer.
  • The terminal observer may schedule a coalesced measurement frame during real subtree/layout changes, but no permanent idle loop remains.
  • The pop-out pet overlay is intentionally exempt from focus pausing because its BrowserWindow is designed to be non-activating.
  • Boot/onboarding effects shown during active setup have their own intentional animations and are outside this configured-idle power fix.

@alt-glitch alt-glitch added type/perf Performance improvement or optimization P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) labels Jul 17, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related: this salvages and broadens closed-unmerged #61084, while targeting the Desktop idle renderer power/CPU cluster reported in #51927 and #53902.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Fixes idle renderer animation loops by tracking window visibility state (isMinimized, isVisible) and handling minimize/restore/hide/show events. Well-scoped fix with Electron event listeners added appropriately. No security concerns.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused performance work. I verified the reported continuous loops still exist on current main: terminal rect polling in apps/desktop/src/app/right-sidebar/terminal/persistent.tsx:104, sprite RAF chaining in apps/desktop/src/components/pet/pet-sprite.tsx:258, roam RAF chaining in apps/desktop/src/components/pet/use-pet-roam.ts:303, and due stream-flush RAF scheduling in apps/desktop/src/app/session/hooks/use-message-stream/index.ts:204.

No substantive correctness issue was identified in the reviewed diff. The change keeps backgroundThrottling: false while changing only the renderer-side scheduling strategy, fitting the existing background-streaming contract in apps/desktop/electron/main.ts:208-219. The added focused tests cover the affected timer/RAF cleanup and pause paths.

The PR base is 121 commits behind current main, but only apps/desktop/electron/main.ts overlaps a post-base desktop change; the current overlap is unrelated to these scheduling paths. Salvage should be mostly mechanical.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 18, 2026
andyylin added 2 commits July 22, 2026 09:43
# Conflicts:
#	apps/desktop/electron/main.ts
# Conflicts:
#	apps/desktop/src/app/session/hooks/use-message-stream/index.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants