V2: go:wasmimport + custom runtime JS helpers based bindings #8
Conversation
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
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthroughThis PR refactors WebAssembly interoperability by removing the wasm-ffi-go dependency and introducing custom FFI helpers in Go (wasm.go) and JavaScript (Runtime class). It restructures the web frontend initialization (removes loading overlay UI), updates Raylib Go structs with HostLayout fields and cptr pointer abstraction, and removes old WASM-specific public APIs from raylib_wasm.go and rcamera.go. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant IndexJS as index.js
participant RaylibModule as Raylib Module
participant GoRuntime as Go Runtime
participant RaylibWASM as Raylib WASM
participant Runtime as Runtime.js
Browser->>IndexJS: Page loads
IndexJS->>RaylibModule: Import Module from raylib.js
RaylibModule-->>IndexJS: Module object (with canvas, wasmBinary)
IndexJS->>GoRuntime: Create Go instance
IndexJS->>Runtime: Import Runtime helpers
Runtime-->>IndexJS: Runtime class
IndexJS->>GoRuntime: Inject raylib into go.importObject.raylib
IndexJS->>GoRuntime: Bind Runtime methods to go.importObject
IndexJS->>GoRuntime: go.Run(wasmInstance)
GoRuntime->>RaylibWASM: Call FFI helpers (malloc, copyToC, cStringFromGoString, etc.)
RaylibWASM-->>GoRuntime: Memory operations/pointers
GoRuntime->>Runtime: Call Runtime bridge methods for memory access
Runtime->>RaylibWASM: Read/write Go and C memory regions
RaylibWASM-->>Runtime: Memory views
Runtime-->>GoRuntime: Slice/struct/string data
GoRuntime-->>Browser: Raylib instance ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@index/runtime.js`:
- Around line 35-38: The function loadSlice is calling a global getInt64 which
is undefined; update the calls to use the instance method (e.g., this.getInt64)
so loadSlice uses the object's getInt64 implementation and then calls
this.getmem(array, len); specifically change getInt64(addr + 0) and
getInt64(addr + 8) to this.getInt64(addr + 0) and this.getInt64(addr + 8) inside
loadSlice to avoid a ReferenceError and ensure proper method resolution.
- Around line 117-124: In CopyToGo, the third argument (srcCptr) is being read
from the wrong stack slot (currently from sp + 8 * 2 + 4); update it to read
from the correct third 8-byte slot (same pattern as CopyToC) i.e. sp + 8 * 3 so
srcCptr is pulled from the proper stack location; adjust the getInt32 call in
the CopyToGo function accordingly to match the Go wasm ABI.
- Around line 23-27: The setInt64 implementation zeroes the high 32 bits which
truncates 64-bit values and breaks pointer/size handling; in setInt64 (and
related places that pair with getInt64/CopyToGo/CopyToC) restore the high-word
calculation instead of writing 0: compute the high 32-bit word from v (e.g.
Math.floor(v / 4294967296) or equivalent unsigned-shift/divide) and set that via
this.mem.setUint32(addr + 4, high, true) so full 64-bit values are preserved.
In `@raylib/raylib.go`:
- Around line 571-590: The NewMatrix and NewMat2 constructors assign many Matrix
fields to the wrong parameters (e.g., M13, M10, M14, M11, M15 are incorrectly
set to m1), producing invalid matrices; update both NewMatrix and NewMat2 so
each Matrix field M0..M15 is assigned the corresponding parameter m0..m15
respectively (ensure M0=m0, M1=m1, M2=m2 ... M15=m15) and keep the HostLayout
initialization unchanged—locate the assignments in the NewMatrix and NewMat2
functions and correct the mapping between the mN parameters and the Matrix.MN
fields.
- Around line 32-42: NewWave currently drops the provided audio buffer by
setting Wave.Data to 0; instead allocate the Go byte slice into C memory and
store its pointer in Wave.Data. In the NewWave function use allocSliceInC(data)
(or the appropriate allocator helper) to obtain a C pointer for the provided
data slice and assign that to the Data field of the returned Wave struct,
keeping FrameCount, SampleRate, SampleSize, Channels and __
(structs.HostLayout{}) as-is; ensure you reference NewWave and allocSliceInC so
the allocation replaces the current Data: 0 assignment.
In `@raylib/wasm.go`:
- Around line 76-83: allocSliceInC and copyArrayToGo currently index s[0] which
panics for empty slices; add a length guard at the start of each function that
returns a nil/zero cptr (or otherwise no-op) when len(s) == 0, and only compute
element size and call unsafe.SliceData/copyToC when len > 0; update
allocSliceInC to return cptr(0) for empty slices instead of indexing s[0], and
update copyArrayToGo to skip reads/writes if len == 0 so neither function ever
accesses s[0] on empty slices.
🧹 Nitpick comments (1)
index/index.js (1)
36-42: Add a MIME-type fallback forinstantiateStreaming.Some hosts serve
main.wasmwith a non‑wasm MIME type; a fallback avoids hard failures.🔧 Proposed fix
-WebAssembly.instantiateStreaming(fetch("main.wasm"), go.importObject).then( - (result) => { - const instance = result.instance; - globalThis.goInstance = instance; - go.run(instance); - }, -); +const goResp = await fetch("main.wasm"); +const goRespClone = goResp.clone(); +const { instance } = await WebAssembly.instantiateStreaming(goResp, go.importObject).catch( + async () => WebAssembly.instantiate(await goRespClone.arrayBuffer(), go.importObject), +); +globalThis.goInstance = instance; +go.run(instance);
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.
These are a rewrite of the bindings using
go:wasmimportand custom javascript helpers that are injected into the Go runtime.This would give us the flexibility to implement all the raylib functions properly and efficiently. The current ones do a lot of unnecessary copying. (Go -> JS -> Raylib).
In the V2 bindings we control the Go runtime, so we can create helpers that directly copy Go memory into raylib and vice-versa (Go <-> Raylib)
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.