Skip to content

fix: wait for images to load and decode before rendering - #592

Open
eoftedal wants to merge 1 commit into
bubkoo:masterfrom
eoftedal:fix/wait-for-image-decode
Open

fix: wait for images to load and decode before rendering#592
eoftedal wants to merge 1 commit into
bubkoo:masterfrom
eoftedal:fix/wait-for-image-decode

Conversation

@eoftedal

@eoftedal eoftedal commented Aug 30, 2026

Copy link
Copy Markdown

Description

Images with a data url src were serialised into the foreignObject without ever being loaded, so the browser could rasterise the svg before it had them and silently leave them out of the capture.

  • add decodeImage(), which waits until an image can be painted and never rejects
  • await it for data urls instead of returning early, and after inlining a fetched resource, since load only says the data arrived
  • drop image.decode = resolve, which overwrote the clone's decode() method instead of awaiting it (Fix Safari images #325)
  • await it in createImage() too: decode() may reject for an image the browser can still paint, and the unhandled rejection left the promise from toPng() pending forever

Tests are in test/spec/image.spec.ts. Three of the four fail against unpatched src: the data url wait (Expected true to be false), decode() still being usable on an inlined image (Expected toBeResolved to be called on a promise), and createImage (unhandled rejection, then a 20s jasmine timeout that takes the browser down with it). The fourth covers the fallback when decode() rejects. With the fix the suite is 55 of 59, the 4 skips being the pre-existing xits.

Motivation and Context

I had issues where images would not show up in the final image. I tried redoing it and it would sometimes work and sometimes not.

Two parts of this overlap with PRs that are already open: #589 fixes the same createImage hang when decode() rejects, and #401 fixes the same image.decode = resolve misuse. This consolidates both with the data url wait, which is the part that explains images going missing. Happy to rebase on either if you would rather take them separately.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Enhancement (changes that improvement of current feature or performance)
  • Refactoring (changes that neither fixes a bug nor adds a feature)
  • Test Case (changes that add missing tests or correct existing tests)
  • Code style optimization (changes that do not affect the meaning of the code)
  • Docs (changes that only update documentation)
  • Chore (changes that don't modify src or test files)

Self Check before Merge

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@biiibooo

biiibooo Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

👋 @eoftedal

💖 Thanks for opening this pull request! 💖

Please follow the contributing guidelines. And we use semantic commit messages to streamline the release process.

Examples of commit messages with semantic prefixes:

  • fix: don't overwrite prevent_default if default wasn't prevented
  • feat: add graph.scale() method
  • docs: graph.getShortestPath is now available

Things that will help get your PR across the finish line:

  • Follow the TypeScript coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes you've made.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.50%. Comparing base (d9b2fcf) to head (9ccde2c).
⚠️ Report is 26 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #592   +/-   ##
=======================================
  Coverage   66.50%   66.50%           
=======================================
  Files          10       10           
  Lines         612      612           
  Branches      150      150           
=======================================
  Hits          407      407           
  Misses        144      144           
  Partials       61       61           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Images with a data url src were serialised into the foreignObject without
ever being loaded, so the browser could rasterise the svg before it had
them and silently leave them out of the capture.

- add decodeImage(), which waits until an image can be painted and never
  rejects
- await it for data urls instead of returning early, and after inlining a
  fetched resource, since load only says the data arrived
- drop `image.decode = resolve`, which overwrote the clone's decode()
  method instead of awaiting it (bubkoo#325)
- await it in createImage() too: decode() may reject for an image the
  browser can still paint, and the unhandled rejection left the promise
  from toPng() pending forever
@eoftedal
eoftedal force-pushed the fix/wait-for-image-decode branch from bd1db7c to 9ccde2c Compare August 30, 2026 20:06
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.

1 participant