Skip to content

fix(wgpu): v29 wire layouts, surface texture on error, pinned pointers in Proc.Call - #32

Open
YindSoft wants to merge 5 commits into
go-webgpu:mainfrom
YindSoft:fix/v29-abi-uintptrescapes
Open

YindSoft wants to merge 5 commits into
go-webgpu:mainfrom
YindSoft:fix/v29-abi-uintptrescapes

Conversation

@YindSoft

Copy link
Copy Markdown

Summary

Four fixes found while building a Go game client on go-webgpu (wgpu-native v29.0.0.0, Windows / DX12). One commit per topic.

  1. Wire layouts match the webgpu.h shipped with wgpu-native v29.0.0.0. WGPUBindGroupLayoutEntry gains bindingArraySize (uint32 + padding) between visibility and buffer; WGPUVertexAttribute and WGPURenderPassDepthStencilAttachment gain the leading nextInChain. Without them every later field was read from the wrong offset and wgpu-native panicked with invalid buffer binding type for buffer binding layout at binding 0 and invalid vertex format for vertex attribute: 0. TestABIWireStructAlignment now asserts the v29 layouts instead of the "migration gap" expectations.
  2. Adapter strings are copied out of wgpu-native memory. stringViewToString aliased it with unsafe.String; the adapter name read garbage once wgpu-native reused the buffer.
  3. Surface.GetCurrentTexture returns the SurfaceTexture on error statuses, and Surface.Present reports a failed wgpuSurfacePresent. Closes Missing Release() in the error path of Surface.GetCurrentTexture() #31. Dropping the texture on Lost/Timeout/Occluded/Error left the swapchain image acquired and the next call aborted the process with Surface image is already acquired. Releasing a texture after a failed present made wgpu-native discard the acquisition a second time (fatal in wgpuTextureRelease), so callers need to know.
  4. Proc.Call carries //go:uintptrescapes. Every binding method passes Go structs as uintptr(unsafe.Pointer(&local)). With Proc as an interface the conversion happened in a call that lacked the directive, so the local stayed on the goroutine stack and could move (stack growth, GC shrink) between the conversion and the syscall; wgpu-native then read stale input or wrote its output to the old stack. Reproduced as wgpuSurfaceGetCurrentTexture "returning" a zeroed WGPUSurfaceTexture (status 0) while the texture had really been acquired, followed by an abort in wgpuSurfaceConfigure (SurfaceOutput must be dropped before a new Surface is made), about once every few program starts. Proc is now a concrete struct and Library.NewProc returns *Proc; the directive is only honored on direct calls. Breaking only for code that implements its own Library/Proc.

Testing

  • WGPU_NATIVE_PATH=<wgpu-native v29.0.0.0 dll> go test ./... -count=1 passes on Windows 11 with an NVIDIA RTX 3080 Ti (DX12), including the render tests.
  • GOOS=linux, GOOS=darwin and GOOS=linux GOARCH=arm64 builds of ./wgpu.
  • The client that motivated the fixes (SDL3 window with a wgpu surface, ~600 draws per frame) no longer hits the sporadic startup abort after commit 3.

🤖 Generated with Claude Code

Three wire structs were missing fields that the webgpu.h shipped with
wgpu-native v29.0.0.0 has, so every field after them was read from the
wrong offset:

- WGPUBindGroupLayoutEntry: bindingArraySize (uint32 + padding) between
  visibility and buffer. wgpu-native panicked with "invalid buffer binding
  type for buffer binding layout at binding N".
- WGPUVertexAttribute: the leading nextInChain. wgpu-native read the format
  from the offset field and panicked with "invalid vertex format for vertex
  attribute: 0".
- WGPURenderPassDepthStencilAttachment: the leading nextInChain.

TestABIWireStructAlignment now asserts the v29 layouts instead of the old
"migration gap" expectations. The binding's own test suite passes against
the official v29.0.0.0 DLL with these changes.
…rt failed present

- stringViewToString aliased wgpu-native memory with unsafe.String; the
  adapter name turned into garbage once wgpu-native reused the buffer.
  Copy the bytes instead.
- Surface.GetCurrentTexture returned nil on Lost/Timeout/Occluded/Error
  even when wgpu had handed out a texture. The caller could not present or
  release it, the swapchain image stayed acquired and the next call aborted
  the process with "Surface image is already acquired". Return the
  SurfaceTexture (status, and the texture only when the handle is non-zero)
  together with the error.
- Surface.Present ignored the WGPUStatus of wgpuSurfacePresent. When the
  present fails wgpu-native has already dropped the acquisition, and
  releasing the texture afterwards discards it a second time (fatal
  "already acquired" in wgpuTextureRelease). Return an error so callers can
  skip the release.
… Proc.Call

Every binding method passes Go structs to wgpu-native as
uintptr(unsafe.Pointer(&local)). Proc was an interface, so the conversion
happened in a call that lacked //go:uintptrescapes: the local stayed on the
goroutine stack and could move (stack growth, GC shrink) between the
conversion and the syscall. wgpu-native then read stale input or wrote its
output to the old stack. In practice wgpuSurfaceGetCurrentTexture
"returned" a zeroed WGPUSurfaceTexture (status 0, texture 0) while the
texture had really been acquired, and the process aborted on the next
present or configure ("Surface image is already acquired" /
"SurfaceOutput must be dropped before a new Surface is made"), about once
every few starts.

Proc is now a concrete struct wrapping the platform implementation and
Proc.Call carries //go:uintptrescapes, which the compiler only honors on
direct calls. Library.NewProc returns *Proc; the platform loaders wrap
their implementation with newProc. This is a breaking change only for
code implementing its own Library/Proc.
//go:uintptrescapes on Proc.Call only protects pointers converted to uintptr
directly in the call's argument list. Many descriptors reach wgpu-native
nested: their address is stored as uintptr inside another struct (render
pass color/depth attachments, SetBindGroup dynamic offsets, color target
Blend, surface sources, command-buffer arrays) or converted before the call.
When those values live on the goroutine stack, a stack growth or shrink
between the conversion and the native call leaves wgpu-native reading stale
memory.

pin[T](p *T) *T returns p unchanged but forces the object onto the heap
(an escape through a package-level sink the compiler cannot rule out); the Go
heap does not move objects. Every uintptr(unsafe.Pointer(&x)) in the package
now goes through pin, plus the two already-taken pointers stored in other
descriptors. Cost: those structs are heap-allocated.

Tested: go vet on windows/linux/darwin; go test ./wgpu passes with the
wgpu-native v29.0.0.0 DLL on Windows (D3D12/Vulkan adapter).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@YindSoft

Copy link
Copy Markdown
Author

Pushed one more commit to this PR: fix(wgpu): pin nested descriptors on the heap (61ba8a6).

It is the same class of bug as the //go:uintptrescapes fix already here, but for pointers the directive doesn't cover. The directive only protects pointers converted to uintptr directly in the argument list of Proc.Call. Many descriptors reach wgpu-native nested instead:

  • render pass color/depth attachments
  • SetBindGroup dynamic offsets
  • the color target Blend of render pipelines
  • surface sources
  • command-buffer arrays

Their address is stored as uintptr inside another struct, or converted before the call. When those values live on the goroutine stack, a stack move between the conversion and the native call leaves wgpu-native reading stale memory.

pin[T](p *T) *T returns p unchanged but forces the object onto the heap, which does not move. Every uintptr(unsafe.Pointer(&x)) in the package (98 sites) now goes through it. The cost is that those structs become heap allocations.

Tested:

  • go vet passes on windows, linux and darwin.
  • go test ./wgpu passes with the wgpu-native v29.0.0.0 DLL on Windows.

@Tnze

Tnze commented Sep 24, 2026

Copy link
Copy Markdown

Oh, this PR fixs #34

@Tnze

Tnze commented Sep 24, 2026

Copy link
Copy Markdown

pin[T](p *T) *T returns p unchanged but forces the object onto the heap, which does not move. Every uintptr(unsafe.Pointer(&x)) in the package (98 sites) now goes through it. The cost is that those structs become heap allocations.

Should use runtime.Pinner instead ?

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.

Missing Release() in the error path of Surface.GetCurrentTexture()

2 participants