Skip to content

fix(playground): report GLSL compiler errors#72

Merged
devallibus merged 2 commits intodevelopmentfrom
fix/playground-glsl-error-reporting
Mar 10, 2026
Merged

fix(playground): report GLSL compiler errors#72
devallibus merged 2 commits intodevelopmentfrom
fix/playground-glsl-error-reporting

Conversation

@devallibus
Copy link
Owner

Summary

  • replace the brittle gl.CURRENT_PROGRAM check in the playground with Three's renderer.debug.onShaderError hook
  • collect and normalize GLSL compile/link diagnostics into a small reusable helper
  • add regression coverage for shader diagnostic extraction and wire it into test:web

Verification

  • bun run test
  • bun run build:web
  • manual local repro against the playground API + browser-connected session confirms invalid GLSL now returns non-empty compilationErrors

Closes #68.

Copy link
Owner Author

@devallibus devallibus left a comment

Choose a reason for hiding this comment

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

LGTM. The switch from gl.CURRENT_PROGRAM to renderer.debug.onShaderError is the right call — hooks into Three.js's own error reporting instead of relying on brittle WebGL state queries. The diagnostic extraction helper is clean, well-tested, and properly handles edge cases (dedup, fallback chain, both throw and non-throw paths).

Two minor nits left as inline comments (redundant | null type, optional missing test for link-only fallback). Neither blocks merge.

Comment on lines +14 to +15
vertexShader?: unknown | null
fragmentShader?: unknown | null
Copy link
Owner Author

Choose a reason for hiding this comment

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

unknown | null is redundant since unknown already includes null. Stylistically fine — communicates intent — but worth knowing it's a no-op at the type level.

Suggested change
vertexShader?: unknown | null
fragmentShader?: unknown | null
vertexShader?: unknown
fragmentShader?: unknown

return [{ kind: 'glsl-compile', message: 'GLSL shader compilation failed.' }]
}

return [{ kind: 'glsl-link', message: 'GLSL program linking failed.' }]
Copy link
Owner Author

Choose a reason for hiding this comment

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

Nice fallback chain. One path that isn't covered by tests: if onShaderError fires but neither shader reports a compile failure and getProgramInfoLog returns empty, this line is reached. Easy to add:

runTest('collectShaderDiagnostics falls back to a generic link message', () => {
  const diagnostics = collectShaderDiagnostics({
    gl: mockGl,
    program: { log: null },
    vertexShader: { ok: true, log: null },
    fragmentShader: { ok: true, log: null },
  })
  assert.deepEqual(diagnostics, [
    { kind: 'glsl-link', message: 'GLSL program linking failed.' },
  ])
})

nit / optional

Comment on lines +190 to 224
const shaderDiagnostics: ReturnType<typeof collectShaderDiagnostics> = []
const previousShaderError = renderer.debug.onShaderError

renderer.debug.onShaderError = (gl, program, glVertexShader, glFragmentShader) => {
shaderDiagnostics.splice(
0,
shaderDiagnostics.length,
...collectShaderDiagnostics({
gl,
program,
vertexShader: glVertexShader,
fragmentShader: glFragmentShader,
}),
)
}

if (errors.length > 0) {
props.onError(errors)
return
}
try {
renderer.render(scene, camera)
} catch (renderError) {
const fallbackMessage = renderError instanceof Error
? renderError.message
: 'Shader compilation failed'

props.onError(
shaderDiagnostics.length > 0 ? diagnosticsToMessages(shaderDiagnostics) : [fallbackMessage],
)
return
} finally {
renderer.debug.onShaderError = previousShaderError
}

if (shaderDiagnostics.length > 0) {
props.onError(diagnosticsToMessages(shaderDiagnostics))
return
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

Good use of renderer.debug.onShaderError — this is the official Three.js hook and much more reliable than querying gl.CURRENT_PROGRAM after the fact. The try/catch/finally ensures the hook is always restored, and the post-render shaderDiagnostics.length > 0 check correctly handles the case where Three.js calls onShaderError without throwing (it may continue with a fallback shader). Well done.

@devallibus devallibus merged commit c2a6042 into development Mar 10, 2026
4 checks passed
@devallibus devallibus deleted the fix/playground-glsl-error-reporting branch March 10, 2026 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant