Skip to content

fix: cancel requestAnimationFrame loops in Tuner and Oscilloscope#5160

Open
manvirsingh01 wants to merge 5 commits intosugarlabs:masterfrom
manvirsingh01:fix/cancel-animation-frame-loops-5145
Open

fix: cancel requestAnimationFrame loops in Tuner and Oscilloscope#5160
manvirsingh01 wants to merge 5 commits intosugarlabs:masterfrom
manvirsingh01:fix/cancel-animation-frame-loops-5145

Conversation

@manvirsingh01
Copy link

@manvirsingh01 manvirsingh01 commented Jan 14, 2026

Fixes Issue #5145 - Tuner, Oscilloscope & Sampler continue running requestAnimationFrame loops after being stopped, causing CPU and battery drain.

Changes:

  • synthutils.js: Added tunerAnimationId and tunerRunning flag to properly track and cancel the animation frame loop in stopTuner()
  • oscilloscope.js: Added isRunning flag to stop animation loop when widget is closed, preventing CPU usage from orphaned animation frames

The sampler.js pitch detection already had proper cleanup implemented via stopPitchDetection() which cancels the animation frame correctly.

@Chaitu7032 @walterbender please my request
Closes #5145

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@Chaitu7032
Copy link
Contributor

@manvirsingh01 thanks for the PR
Appreciate your work on this .

And project maintainers either walterbender sir , om santhosh sir will review your PR by completing checks soon . And I just want to mention you that their is also need of working with sampler .. which also be helpful in optimization of background process.

@walterbender
Copy link
Member

Something is wrong:

Screenshot From 2026-01-17 12-08-40

The sampler breaks.

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@manvirsingh01
Copy link
Author

manvirsingh01 commented Jan 19, 2026

@walterbender
Fix: TunerUtils is not defined error
Root Cause what i get : TunerUtils in tuner.js was only exported for Node.js (CommonJS), not exposed to the browser's global window object. Additionally, widgets/tuner was missing from the RequireJS module list in activity.js.

Changes:

  1. js/widgets/tuner.js
  2. Added window.TunerUtils and window.TunerDisplay for browser access js/activity.js
  3. Added "widgets/tuner" to MUSICBLOCKS_EXTRAS before "widgets/sampler"

Verification: All 88 test suites pass (2275 tests)
Sampler widget loads without console errors

@walterbender
Copy link
Member

Working much better now. But please rebase to address the conflict in activity.js

@manvirsingh01 manvirsingh01 force-pushed the fix/cancel-animation-frame-loops-5145 branch from 4d53ef7 to ee8b87e Compare January 19, 2026 16:07
@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

1 similar comment
@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@manvirsingh01
Copy link
Author

@walterbender Thanks for the feedback! I've rebased the branch and resolved the merge conflict in activity.js.
The conflict was between my __makeNewNote function addition and the latest master - I kept my changes since master had no conflicting code at that location.

Ready for another look when you have time! @walterbender

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@manvirsingh01 manvirsingh01 force-pushed the fix/cancel-animation-frame-loops-5145 branch from 65c172b to 2fbb256 Compare January 20, 2026 03:53
@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@manvirsingh01
Copy link
Author

manvirsingh01 commented Jan 21, 2026

@walterbender @Chaitu7032 can you approve my work

@Chaitu7032
Copy link
Contributor

@manvirsingh01 I truly appreciate the effort you’ve put in . Your work is on track for approval soon. In the meantime, I encourage you to explore further opportunities where you can contribute meaningfully and with confidence.

Fixes Issue sugarlabs#5145 - Tuner, Oscilloscope & Sampler continue running
requestAnimationFrame loops after being stopped, causing CPU and battery drain.

Changes:
- synthutils.js: Added tunerAnimationId and tunerRunning flag to properly
  track and cancel the animation frame loop in stopTuner()
- oscilloscope.js: Added isRunning flag to stop animation loop when widget
  is closed, preventing CPU usage from orphaned animation frames

The sampler.js pitch detection already had proper cleanup implemented via
stopPitchDetection() which cancels the animation frame correctly.

Closes sugarlabs#5145
- Added window.TunerUtils and window.TunerDisplay in tuner.js for browser access
- Added widgets/tuner to MUSICBLOCKS_EXTRAS in activity.js before widgets/sampler
- Fixes 'TunerUtils is not defined' error in sampler widget
@manvirsingh01 manvirsingh01 force-pushed the fix/cancel-animation-frame-loops-5145 branch from 2fbb256 to fb08fb2 Compare January 23, 2026 06:36
@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@walterbender
Copy link
Member

I don't understand the changes you made to activity.js

@manvirsingh01
Copy link
Author

manvirsingh01 commented Jan 25, 2026

I don't understand the changes you made to activity.js

Sir, I have not seen any test error in my local system.so i make the empty commit to get fresh log . Can you help me with that?

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@manvirsingh01
Copy link
Author

@walterbender I’ve updated the Prettier version to 2.8.

@walterbender
Copy link
Member

I don't understand. What does makeNewNote have to do with the issue you are addressing? It seems unrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tuner, Oscilloscope & Sampler continue running requestAnimationFrame loops after being stopped, causing CPU and battery drain

3 participants