Fix Canvas image reload and cancellation lifetimes - #1893
bkaradzic-microsoft merged 4 commits into
Conversation
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
Fixes Canvas Image.src lifecycle so each assignment has an independent cancellation scope, cancelled completions don’t surface as errors or override newer loads, and decoded-image cleanup is separated from load cancellation.
Changes:
- Split image release from cancellation (
ReleaseImage()vsDispose()), and release previous decoded images on replacement. - Reset cancellation lifetime per
srcassignment and ignore cancelled URL/data-URL completions. - Add JS unit tests + fixture asset to cover reloads, superseded requests, and recovery after errors.
| File | Description |
|---|---|
| Polyfills/Canvas/Source/Image.h | Adds ReleaseImage() API for separating decoded-image cleanup from cancellation/disposal. |
| Polyfills/Canvas/Source/Image.cpp | Refactors image cleanup and per-src cancellation; ignores cancelled completions and avoids poisoning future loads. |
| Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts | Adds regression tests for repeated loads, superseding, immediate src reflection, and recovery after missing-file errors. |
| Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js | Compiled test output reflecting the new regression suite. |
| Apps/UnitTests/CMakeLists.txt | Packages new PNG fixture used by the added tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Fixed the Windows SDK installation failure in 772c425.
Validation: the real installed-SDK consumer builds without A verified Balanced Copilot re-review is still pending: the public review-request API ignores the effort field, so I have not substituted another Lite request. |
772c425 to
87e05ae
Compare
Review-Group: E3 Source: PR BabylonJS#1893 Squashed final review changes, including regressions and review follow-ups. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae
87e05ae to
21509a0
Compare
Use a fresh cancellation source for each src assignment, ignore superseded completions, and release replaced image data without cancelling the load. Preserve image error reporting and PNG normalization; add local reload, cancellation, src reflection, and error-recovery regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae
Use an in-memory URL resolver and a final queued image completion as the barrier for cancellation assertions, instead of sleeping for 50 ms. Keep real app URL reload/error coverage and document JS-thread serialization of data decoding and src/disposal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae
Guard the test-only UrlLib resolver with the existing native image-test feature flag, including its headers, cleanup, registration, and linkage. The installed SDK consumer reuses this source without image tests and must not depend on private UrlLib headers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae
SetBuffer replaces the decoded pixels, but contexts cached a NanoVG texture by NativeCanvasImage pointer. Compare a content generation and keep the old handle until flush. Cover URL-to-URL and URL-to-data reloads, and stop the reload tests from referencing a webpack helper that is not in scope. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e6c3c123-c355-4790-a3d4-94337ed6e052
21509a0 to
cd10c56
Compare
Summary
Image.srcassignment its own cancellation lifetime and reflect the assigned source immediately.Dispose()and poison subsequent loads; data-URL replacements must release the previous image.Independence
Based directly on upstream
masterat2a9dc944, with published Babylon.js 9.21.2 and unchanged native dependency pins. Not stacked on the other new PRs.This extracts only the image-lifecycle portion of the shotgun work. It does not add SVG support, Canvas branding changes, or a silent-error policy. It retains the PNG normalization from #1887. It does not replace #1882's separate image-parser error handling; the changes should compose.
Validation
Image.cpp/Image.hproduce six failures, including the two URL reload timeouts; restoring this fix passes all eight new cases.