Skip to content

fix(gpu): check for a device before queueing a path draw - #534

Open
timzifer wants to merge 1 commit into
gogpu:mainfrom
timzifer:fix/path-ops-check-device-before-queueing
Open

timzifer wants to merge 1 commit into
gogpu:mainfrom
timzifer:fix/path-ops-check-device-before-queueing

Conversation

@timzifer

@timzifer timzifer commented Sep 7, 2026

Copy link
Copy Markdown

Summary

With the GPU accelerator registered and no usable adapter present, every filled and stroked path is silently dropped. Text still renders, so the output is a valid image with the labels in it and none of the geometry — and no error is returned anywhere.

FillPath, StrokePath, FillShape and StrokeShape queue a draw command without establishing that a device can be had. Context.doFill and doStroke read the nil return as "the GPU has this" and skip the software rasterizer. The failure only surfaces at Flush, where ensureGPU() runs, fails with request adapter: no adapters available, logs a warning and returns ErrFallbackToCPU — by which point the pending draws have nowhere to go.

DrawText and DrawGlyphMaskText already make this check before they queue (gpu_render_context.go:546, :583), which is why text is unaffected and why the symptom looks like a rasterizer artefact rather than a missing device.

Minimal reproduction, no external dependencies:

import (
	"github.com/gogpu/gg"
	_ "github.com/gogpu/gg/gpu"
)

c := gg.NewContext(200, 150)
c.MoveTo(20, 130)
c.LineTo(80, 30)
c.LineTo(180, 20)
c.SetStroke(gg.Stroke{Width: 3})
c.SetColor(gg.RGBA{B: 1, A: 1})
_ = c.Stroke()   // returns nil
_ = c.FlushGPU() // returns ErrFallbackToCPU, too late
_ = c.SavePNG("out.png") // blank

Without the gpu import, or after gg.CloseAccelerator(), the same code draws the line.

Changes

  • internal/gpu: FillPath, StrokePath, FillShape and StrokeShape check for a device before queueing, returning ErrFallbackToCPU when there is none, so the caller's CPU path runs.
  • internal/gpu: new deviceUsable() helper — the check DrawText makes inline, hoisted.
    • It asks for deviceReady, not gpuReady. Under strategyRasterAtlas the shape pipelines are deliberately absent while a device exists and Flush dispatches the queue on the CPU, so a queued draw does reach the buffer there. Guarding on gpuReady breaks TestFlushCPU_TempPixmapDimensions and TestFlushCPUToView_WithNoop.
    • It sits after the compute-mode delegation, which routes to VelloAccelerator and answers for its own readiness through CanCompute(). Guarding before it breaks TestStrokeRouting_ComputeModeUsesVello.
  • gpu/fallback_test.go: regression test.

No public API change. No behaviour change on a machine with a working adapter: deviceReady is already true there, so the check is a field read.

Testing

The test asserts the property the gpu package documents — "If GPU initialization fails, the registration is silently skipped and rendering falls back to CPU" — on a stroked path: with the accelerator registered, the stroke has to be in the buffer. It holds on hardware too, where the draw goes to the GPU and comes back.

Against the current code on a machine with no adapter:

--- FAIL: TestAPathIsDrawnWithOrWithoutADevice
    fallback_test.go:57: the stroked path is not in the buffer: every pixel is the same colour

With the fix, and across the repository:

go test -race ./...   # all packages pass

Found while rendering charts through a plotting library that offers this package as an opt-in tier; every chart came out with its axis labels and no axes.

Environment: Windows 11 x64, Go 1.27.0, branched off main at bd563f4.

Checklist

  • Tests pass (go test -race ./...)
  • Linter passes (golangci-lint run) — could not run locally: golangci-lint v2.12.2 is built with Go 1.25 and panics on this tree with file requires newer Go version go1.27. Left to CI.
  • Code formatted (go fmt ./... — changed files only; a repository-wide go fmt rewrites every file here because of CRLF checkout, so that was reverted)
  • Documentation updated (if applicable) — no public API change; the behaviour now matches what gpu/gpu.go's package comment already promises

FillPath, StrokePath, FillShape and StrokeShape queued a draw command
without establishing that a device could be had. Context.doFill and
doStroke read the nil return as "the GPU has this" and skip the software
rasterizer, so the failure only surfaced at Flush, where ensureGPU runs —
by which time the pending draws had nowhere to go and were dropped.

On a machine with no adapter the result was silent geometry loss: every
filled and stroked path vanished while text still rendered, because
DrawText and DrawGlyphMaskText make this check before they queue. What
came out was a valid image with nothing in it but the labels, and no
error anywhere: Stroke had already returned nil.

The four queueing entry points now make the same check. It asks for
deviceReady rather than gpuReady, because under strategyRasterAtlas the
shape pipelines are deliberately absent and Flush dispatches the queue on
the CPU — a queued draw still reaches the buffer there. It sits after the
compute-mode delegation, which routes to VelloAccelerator and answers for
its own readiness.

The test is the property the gpu package documents — with the accelerator
registered, a chart still renders — asserted on a stroked path. It fails
against the old code on any machine without an adapter, and passes on
hardware, where the draw goes to the GPU and comes back.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qry8YtUUPEPY5fiaAK5QWu
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