Skip to content

Conversation

@lyonsno
Copy link
Owner

@lyonsno lyonsno commented Nov 21, 2025

Summary

  • fail fast in SEC3DEMO when the required WEBGL_draw_buffers extension is unavailable
  • gate shader prefixing on WEBGL_draw_buffers support to avoid extension-related link failures

Testing

  • not run (not requested)

Codex Task

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +522 to +526
if (!gl.getExtension("WEBGL_draw_buffers")) {
console.warn("WEBGL_draw_buffers extension unavailable; rendering will fail.");
if (msg) {
msg.innerText = "WEBGL_draw_buffers is required for SEC3DEMO.";
}

Choose a reason for hiding this comment

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

P1 Badge Fail-fast check leaves demo in inconsistent state

When WEBGL_draw_buffers is unavailable, setupScene now returns early here (return at line 527), but main() still calls setKeyInputs, SEC3.renderer.init(), etc. (lines 203‑221) assuming scene, interactor, and lights were created. In browsers where the extension is missing, this path now triggers runtime exceptions (e.g., interactor.onKeyDown or renderer init using scene) instead of a graceful exit. The fail-fast should either halt the rest of main() or propagate a failure signal so the remaining initialization is skipped.

Useful? React with 👍 / 👎.

Copy link
Owner Author

Choose a reason for hiding this comment

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

@codex I think without drawbuffers the move is to fail gracefully. Let's check how common it would be that they wouldn't be available these days.

Choose a reason for hiding this comment

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

Summary

  • Made setupScene return explicit success status for WebGL context acquisition, extension availability, and FBO initialization so failures exit early with clear messaging. demos/SEC3DEMO.jsL506-L636
  • Updated main to skip key/input setup and renderer initialization when scene setup fails, preventing runtime errors when WEBGL_draw_buffers is missing. demos/SEC3DEMO.jsL203-L220

Testing

  • ⚠️ Not run (not requested).

View task →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants