Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 27, 2025

Fixes the missing implementation of bufferData(target, size, usage) in the WebGL JavaScript bindings. This method was already supported in the underlying C++ implementation (client_graphics::WebGLContext::bufferData) but the JavaScript binding layer was throwing an error instead of calling the appropriate C++ method.

Problem

The WebGL binding in src/bindings/webgl/rendering_context-inl.hpp contained a TODO comment and explicitly threw an error when bufferData() was called with a number as the second parameter:

// This would fail with "bufferData(target, size, usage) is not supported yet."
gl.bufferData(gl.ARRAY_BUFFER, 1024, gl.STATIC_DRAW);

Solution

Removed the artificial limitation and implemented proper handling for the size-based bufferData call:

  • Extract target and usage parameters upfront
  • Check if the second parameter is a number (size)
  • If so, call the existing C++ method: glContext_->bufferData(target, size, usage)
  • Preserve all existing functionality for data-based calls (TypedArray, DataView, ArrayBuffer)

Testing

Added comprehensive test fixture fixtures/html/webgl-conformance/bufferdata-size-test.html that validates:

  • Basic buffer allocation with size
  • Different buffer targets (ARRAY_BUFFER, ELEMENT_ARRAY_BUFFER)
  • Different usage patterns (STATIC_DRAW, DYNAMIC_DRAW, STREAM_DRAW)
  • Large buffer allocation
  • Backward compatibility with existing TypedArray-based calls

Changes

  • Modified: src/bindings/webgl/rendering_context-inl.hpp (net -3 lines)
  • Added: Test fixture for validation
  • No breaking changes: All existing bufferData usage continues to work

This change enables WebGL applications to efficiently allocate GPU buffers with specific sizes without providing initial data, which is a common pattern for dynamic buffer usage.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • registry.npmmirror.com
    • Triggering command: npm ci (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
Copilot AI changed the title [WIP] 在 src/bindings/webgl/rendering_context-inl.hpp 中,BufferData() 支持第二个参数是 size,即 bufferData(target, size, usage),这个方法已经在 client_graphics::WebGLContext::bufferData 中支持了 Implement bufferData(target, size, usage) support in WebGL bindings Aug 27, 2025
Copilot AI requested a review from yorkie August 27, 2025 09:47
@yorkie yorkie marked this pull request as ready for review August 27, 2025 10:30
@yorkie yorkie merged commit a48f9f0 into main Aug 27, 2025
2 checks passed
@yorkie yorkie deleted the copilot/fix-660ec106-5e86-4d26-a123-280bd3d97f72 branch August 27, 2025 10:30
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.

2 participants