Skip to content

refactor: replace createjs.Ticker framerate with requestAnimationFrame idle management#7451

Open
Mohd-Ali-Creator wants to merge 1 commit into
sugarlabs:masterfrom
Mohd-Ali-Creator:feat/high-contrast-theme
Open

refactor: replace createjs.Ticker framerate with requestAnimationFrame idle management#7451
Mohd-Ali-Creator wants to merge 1 commit into
sugarlabs:masterfrom
Mohd-Ali-Creator:feat/high-contrast-theme

Conversation

@Mohd-Ali-Creator

@Mohd-Ali-Creator Mohd-Ali-Creator commented May 31, 2026

Copy link
Copy Markdown
Contributor

Category

refactor

Summary

Removes all createjs.Ticker.framerate calls and
replaces them with the existing requestAnimationFrame
render loop (_startRenderLoop).

Changes

  • Removed createjs.Ticker.framerate = 60 (initial setup)
  • Removed createjs.Ticker.framerate = ACTIVE_FPS
    in _resetIdleTimer
  • Removed createjs.Ticker.framerate = IDLE_FPS
    in idle watcher
  • Removed unused ACTIVE_FPS and IDLE_FPS constants
  • _startRenderLoop() now handles idle/active rendering

Why This Is Safe

_startRenderLoop() at line 537 already uses
requestAnimationFrame and calls stage.update()
directly. It naturally pauses when stageDirty=false,
making explicit Ticker framerate control redundant.

Testing

  • App loads and renders correctly ✅
  • Idle mode works via rAF loop natural pause ✅
  • User interaction restarts rendering ✅

Part of CreateJS migration effort for DMP 2026.

PR Category

  • Bug Fix
  • Feature
  • Performance
  • Tests
  • Documentation

cc @walterbender @sum2it @omsuneri

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Jest Test Results

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

Coverage: Statements: 48.09% | Branches: 39.61% | Functions: 52.88% | Lines: 48.5%
Master Coverage: Statements: 48.09% | Branches: 39.61% | Functions: 52.88% | Lines: 48.49%

@github-actions github-actions Bot added performance Improves performance (load time, memory, rendering) size/S Small: 10-49 lines changed area/javascript Changes to JS source files tests Adds or updates test coverage and removed tests Adds or updates test coverage labels May 31, 2026
@lavjeetrai

Copy link
Copy Markdown
Contributor

AFAIU,
binding to high frequency events like mousemove spawns multiple parallel loops, causing an rAF stampede that pushes the framerate to 600+ and freezes the browser.

Wrap _startRenderLoop() in a boolean lock (e.g., this._isRendering) so it can only queue one frame at a time:

@Mohd-Ali-Creator

Copy link
Copy Markdown
Contributor Author

Thanks for the review @lavjeetrai!

_startRenderLoop() already has a boolean
guard at line 537:

this._startRenderLoop = () => {
    if (this._renderLoopRunning) return;
    this._renderLoopRunning = true;

This ensures only one rAF loop runs at a
time, regardless of how many times
_startRenderLoop() is called from
mousemove events.

The existing lock prevents any rAF stampede.
Does this address your concern? 🙂

@lavjeetrai lavjeetrai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm,thanks for clarification

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

Labels

area/javascript Changes to JS source files performance Improves performance (load time, memory, rendering) size/S Small: 10-49 lines changed

Projects

Development

Successfully merging this pull request may close these issues.

2 participants