Skip to content

Conversation

@Tianj0o
Copy link
Contributor

@Tianj0o Tianj0o commented Aug 17, 2025

Description of change

This PR fixes multiple memory leaks in the Text object by ensuring that all internal event listeners and caches are properly cleared upon destruction.

Specifically, it addresses two main issues:

  1. The Text object's event listener for renderer.runners.resolutionChange was not being removed in the destroy() method.
  2. The CanvasTextMetrics cache was not being cleared, causing cached font style strings to remain in memory.

Process of Discovery and Test Results

I discovered these issues while working on an application that frequently creates and destroys a large number of Text objects. My initial observation was that memory usage continued to climb even after correctly calling destroy() on all objects.

Using DevTools Performance Monitor and Heap Snapshots, I was able to identify Text objects that were not being garbage collected. By analyzing their object retainers, I traced the root cause to two primary culprits: the un-removed resolutionChange listener and the persistent CanvasTextMetrics cache entries.

To verify the fix, I created a test case that rapidly creates and destroys Text objects. The results are clear:

  • Before the fix: The memory profile showed a steady increase in memory usage, indicating a clear leak.
  • After the fix: The memory profile stabilized, with memory usage returning to the baseline after each cycle of creation and destruction.
Pre-Merge Checklist
  • Lint process passed (npm run lint)
  • Tests passed (npm run test)

@Tianj0o
Copy link
Contributor Author

Tianj0o commented Aug 17, 2025

Test code

(async () => {
  // Create a new application
  const app = new Application();

  // Initialize the application
  await app.init({ background: '#1099bb', width: 800, height: 600 });

  // Append the application canvas to the document body

  function drawGraphicsAndText() {
    const removed = app.stage.removeChildren();
    removed.forEach(child => {
      child.destroy(true)
    })


    const words = ['Hello', 'World', 'PIXI', 'Graphics', 'Text', 'Random', 'Vue', 'TypeScript', 'Canvas', 'Animation'];

    for (let i = 0; i < 500; i++) {
      const randomWord = words[Math.floor(Math.random() * words.length)];

      const color = Math.random() * 0xFFFFFF;

      const fontSize = 12 + Math.random() * 24;

      const style = new TextStyle({
        fontFamily: 'Arial',
        fontSize: fontSize,
        fill: color,
        fontWeight: 'bold'
      });

      const text = new Text({
        text: randomWord,
        style
      });

      text.x = Math.random() * (app.screen.width - text.width);
      text.y = Math.random() * (app.screen.height - text.height);

      text.rotation = Math.random() * Math.PI * 2;
      app.stage.addChild(text);
    }
  }

  const btn = document.createElement('button')
  btn.innerHTML = 'draw'
  btn.addEventListener('click', async () => {
    for (let i = 0; i < 100; i++) {
      drawGraphicsAndText()
      await new Promise(rsolve => setTimeout(rsolve, 20))
    }
  })
  document.body.appendChild(btn)
  document.body.appendChild(app.canvas);
})();

@pkg-pr-new
Copy link

pkg-pr-new bot commented Aug 17, 2025

pixi.js-basepixi.js-bunny-mark

npm i https://pkg.pr.new/pixijs/pixijs/pixi.js@11619

commit: bc852ee

@Tianj0o
Copy link
Contributor Author

Tianj0o commented Aug 17, 2025

before fix
Snipaste_2025-08-17_11-34-03

after fix
Snipaste_2025-08-17_12-19-21

@Tianj0o Tianj0o changed the title fix(text): Fix multiple memory leaks in Text fix: fix multiple memory leaks in Text Aug 17, 2025
@mayakwd
Copy link
Collaborator

mayakwd commented Aug 18, 2025

Other than that, I'd like to admit - numbers are impressive!

@mayakwd
Copy link
Collaborator

mayakwd commented Aug 25, 2025

Well done! I appreciate your patience!

@Tianj0o
Copy link
Contributor Author

Tianj0o commented Aug 25, 2025

Thanks for your guidance 🎉

@Zyie Zyie added the ✅ Ready To Merge Helpful when issues are in the queue waiting to get merged. This means the PR is completed and has t label Aug 26, 2025
@Zyie Zyie added this pull request to the merge queue Aug 26, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 26, 2025
@Zyie Zyie added this pull request to the merge queue Aug 26, 2025
Merged via the queue into pixijs:dev with commit b4c050a Aug 26, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✅ Ready To Merge Helpful when issues are in the queue waiting to get merged. This means the PR is completed and has t

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants