Playground validation: fix undefined renderImage in the scriptToRun path - #1822
Merged
bkaradzic-microsoft merged 1 commit intoAug 7, 2026
Conversation
… path `loadPlayground` declares its third parameter as `referenceImage`, but the `test.scriptToRun` branch passes `renderImage` to `processCurrentScene`. `renderImage` is not bound in that scope, so as soon as the scene script is evaluated the call throws `ReferenceError: renderImage is not defined`, the surrounding catch reports it, and the test fails. Every other call site in `loadPlayground` already passes `referenceImage`, and `processCurrentScene` forwards the argument straight to `evaluateScreenshot(test, screenshot, renderImage, ...)` as the reference image to compare against -- so `referenceImage` is the intended value. This was latent until now: the `scriptToRun` tests could never reach this line, because they wait on `request.onreadystatechange`, which the JsRuntimeHost XMLHttpRequest polyfill never invokes (it only dispatches handlers registered through `addEventListener`). Those tests therefore hung until the harness timeout rather than running, which is why they are currently excluded on every graphics API. With that polyfill gap fixed in JsRuntimeHost, the tests get this far and immediately hit the ReferenceError. With both fixed, Fog, Lines, Lens and Self shadowing pass; Polygon, GUI and Procedural textures now fail fast with actionable, unrelated errors instead of hanging. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88569c10-a7ff-4373-9a58-afa9c68b8c09
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a runtime ReferenceError in the native playground validation harness when running tests via the scriptToRun path by passing the correct in-scope reference image argument into processCurrentScene.
Changes:
- Replace an out-of-scope
renderImageidentifier withreferenceImagein thetest.scriptToRunexecution branch. - Align the
scriptToRunbranch with the otherloadPlaygroundcall sites that already passreferenceImagethrough to screenshot comparison.
bkaradzic-microsoft
enabled auto-merge (squash)
August 7, 2026 04:36
bghgary
approved these changes
Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
loadPlaygrounddeclares its third parameter asreferenceImage:but the
test.scriptToRunbranch passesrenderImage:renderImageis not bound in that scope. It is only the parameter name ofprocessCurrentScene(test, renderImage, done, compareFunction). So the momenta
scriptToRunscene is evaluated, the call throwsReferenceError: renderImage is not defined, the enclosingcatchlogs it,and the test is marked failed.
Every other call site in
loadPlaygroundalready passesreferenceImage,and
processCurrentSceneforwards the argument straight through toevaluateScreenshot(test, screenshot, renderImage, ...)where it is used asthe reference image to compare against.
referenceImageis clearly theintended value.
Why this was never noticed
The
scriptToRuntests could not reach this line. They wait onrequest.onreadystatechange, and the JsRuntimeHostXMLHttpRequestpolyfill never invokes it --
RaiseEventonly dispatches handlers registeredvia
addEventListener, and there are noon<event>property accessors atall. So the request completed (
readyState4,status200) but thecallback never ran and the test hung until the harness timeout.
That is fixed separately in BabylonJS/JsRuntimeHost#TBD. With the polyfill fixed,
these tests finally get as far as evaluating the scene -- and immediately hit
this ReferenceError.
Effect
Tests affected: Fog, Polygon, Lines, Lens, Self shadowing, GUI, Procedural
textures. They are currently excluded on every graphics API (D3D11, D3D12,
OpenGL, Vulkan, Metal, WebGPU) because they hang, so this is not WebGPU/Dawn
specific -- it affects every rendering backend.
With this fix plus the JsRuntimeHost one, locally:
earcut is not defined)name is not defined)The three remaining failures are separate, unrelated issues that were previously
invisible; they now surface as fast, actionable errors instead of a hang.