feat: expose query sets and render bundle encoders

[gusevgrishaem1]

Summary

This PR adds public query set types and render bundle encoders.

Related to #330

[kolkov]

Thanks for the PR! Good structural foundation — build tags, lifecycle pattern (atomic.Bool + idempotent Release), and HAL delegation are all correct.

A few issues to address before merge:

1. QueryType duplication

QueryType is already defined in hal/descriptor.go:547-555 with identical values. The PR adds a second independent definition in types.go, then casts between them in toHAL() via hal.QueryType(d.Type).

In Rust wgpu, QueryType lives in wgpu-types (the shared types crate) — single source of truth (wgpu-types/src/lib.rs:450). Our equivalent is gputypes, but for now a type alias works:

type QueryType = hal.QueryType

const (
    QueryTypeOcclusion = hal.QueryTypeOcclusion
    QueryTypeTimestamp  = hal.QueryTypeTimestamp
)

This eliminates the duplication and the fragile cast.

2. Incomplete API — QuerySet is unusable without companion methods

Creating a QuerySet is only half the story. The W3C WebGPU spec (index.bs:11239-11295) requires companion methods to actually use queries:

Method	Spec	Our HAL	Public API
CommandEncoder.resolveQuerySet()	Required	✅ hal/command.go:62-68	❌ Missing
RenderPassDescriptor.timestampWrites	Required	✅ hal/descriptor.go:481-482	❌ Missing
RenderPassDescriptor.occlusionQuerySet	Required	❌ Not in HAL	❌ Missing
RenderPass.beginOcclusionQuery()	Required	❌ Not in HAL	❌ Missing
RenderPass.endOcclusionQuery()	Required	❌ Not in HAL	❌ Missing

Without at least resolveQuerySet, a user creates a QuerySet but can never read results from it.

Suggestion: either add CommandEncoder.ResolveQuerySet() in this PR (the HAL implementation is ready on all backends), or add a doc comment stating "Phase 1 — companion methods in follow-up PR" with a link to #330.

3. Incomplete API — RenderBundle is unusable without ExecuteBundles

Same pattern: RenderBundleEncoder.Finish() produces a RenderBundle, but there's no way to execute it. The spec requires RenderPass.executeBundles() (index.bs:13935-13948), and our HAL already implements it (hal/command.go:149-151, all 6 backends).

Suggestion: add RenderPassEncoder.ExecuteBundles(bundles ...*RenderBundle) to the public API in this PR.

4. Finish() should accept a descriptor

Both the W3C spec and Rust wgpu pass a descriptor to finish:

• Spec: GPURenderBundle finish(optional GPURenderBundleDescriptor descriptor = {}) (index.bs:14075)
• Rust: pub fn finish(self, desc: &RenderBundleDescriptor) -> RenderBundle (render_bundle_encoder.rs:76)

The descriptor contains at minimum a label for the resulting bundle. Suggestion:

func (e *RenderBundleEncoder) Finish(desc *RenderBundleDescriptor) (*RenderBundle, error)

Returning (*RenderBundle, error) is more Go-idiomatic than returning nil on double-finish.

5. Encoder methods need guard after Finish()

The W3C spec defines an encoder state machine (index.bs:10455-10491): commands on an "ended" encoder must generate a validation error. Currently only Finish() checks e.finished — the 6 draw/state methods (SetPipeline, SetBindGroup, SetVertexBuffer, SetIndexBuffer, Draw, DrawIndexed) pass straight through to HAL after finish.

Go doesn't have Rust's ownership semantics that prevent this at compile time, so a runtime check is needed:

func (e *RenderBundleEncoder) SetPipeline(pipeline *RenderPipeline) {
    if e.finished.Load() {
        return // or log warning
    }
    e.hal.SetPipeline(pipeline.hal)
}

---

Overall the code quality is good — you clearly understand the wgpu architecture. These changes will make the API complete and usable. Happy to help if you have questions!

[gusevgrishaem1]

Hi! Thanks for the review! This is my first open-source contribution, so I really appreciate your feedback. I’ll update the MR according to your comments soon.

Thanks for the PR! Good structural foundation — build tags, lifecycle pattern (atomic.Bool + idempotent Release), and HAL delegation are all correct.

A few issues to address before merge:

1. QueryType duplication

QueryType is already defined in hal/descriptor.go:547-555 with identical values. The PR adds a second independent definition in types.go, then casts between them in toHAL() via hal.QueryType(d.Type).

In Rust wgpu, QueryType lives in wgpu-types (the shared types crate) — single source of truth (wgpu-types/src/lib.rs:450). Our equivalent is gputypes, but for now a type alias works:

type QueryType = hal.QueryType

const (
    QueryTypeOcclusion = hal.QueryTypeOcclusion
    QueryTypeTimestamp  = hal.QueryTypeTimestamp
)

This eliminates the duplication and the fragile cast.

2. Incomplete API — QuerySet is unusable without companion methods

Creating a QuerySet is only half the story. The W3C WebGPU spec (index.bs:11239-11295) requires companion methods to actually use queries:

Method Spec Our HAL Public API
CommandEncoder.resolveQuerySet() Required ✅ hal/command.go:62-68 ❌ Missing
RenderPassDescriptor.timestampWrites Required ✅ hal/descriptor.go:481-482 ❌ Missing
RenderPassDescriptor.occlusionQuerySet Required ❌ Not in HAL ❌ Missing
RenderPass.beginOcclusionQuery() Required ❌ Not in HAL ❌ Missing
RenderPass.endOcclusionQuery() Required ❌ Not in HAL ❌ Missing
Without at least resolveQuerySet, a user creates a QuerySet but can never read results from it.

Suggestion: either add CommandEncoder.ResolveQuerySet() in this PR (the HAL implementation is ready on all backends), or add a doc comment stating "Phase 1 — companion methods in follow-up PR" with a link to #330.

3. Incomplete API — RenderBundle is unusable without ExecuteBundles

Same pattern: RenderBundleEncoder.Finish() produces a RenderBundle, but there's no way to execute it. The spec requires RenderPass.executeBundles() (index.bs:13935-13948), and our HAL already implements it (hal/command.go:149-151, all 6 backends).

Suggestion: add RenderPassEncoder.ExecuteBundles(bundles ...*RenderBundle) to the public API in this PR.

4. Finish() should accept a descriptor

Both the W3C spec and Rust wgpu pass a descriptor to finish:

• Spec: GPURenderBundle finish(optional GPURenderBundleDescriptor descriptor = {}) (index.bs:14075)
• Rust: pub fn finish(self, desc: &RenderBundleDescriptor) -> RenderBundle (render_bundle_encoder.rs:76)

The descriptor contains at minimum a label for the resulting bundle. Suggestion:

func (e *RenderBundleEncoder) Finish(desc *RenderBundleDescriptor) (*RenderBundle, error)

Returning (*RenderBundle, error) is more Go-idiomatic than returning nil on double-finish.

5. Encoder methods need guard after Finish()

The W3C spec defines an encoder state machine (index.bs:10455-10491): commands on an "ended" encoder must generate a validation error. Currently only Finish() checks e.finished — the 6 draw/state methods (SetPipeline, SetBindGroup, SetVertexBuffer, SetIndexBuffer, Draw, DrawIndexed) pass straight through to HAL after finish.

Go doesn't have Rust's ownership semantics that prevent this at compile time, so a runtime check is needed:

func (e *RenderBundleEncoder) SetPipeline(pipeline *RenderPipeline) {
    if e.finished.Load() {
        return // or log warning
    }
    e.hal.SetPipeline(pipeline.hal)
}

Overall the code quality is good — you clearly understand the wgpu architecture. These changes will make the API complete and usable. Happy to help if you have questions!

Hi! Thanks for the review! This is my first open-source contribution, so I really appreciate the feedback. I’ll address the comments and update the MR soon.

[codecov]

Codecov Report

❌ Patch coverage is 97.24771% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
encoder_native.go	85.71%	2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[gusevgrishaem1]

Hi! I've made the requested changes. Could you please take another look when you have time? Thanks!

[lkmavi]

Hi @gusevgrishaem1 — thanks again for the patience 🙏

#347 is now merged into main, so we can unblock #337. GitHub shows this PR as conflicting with main. Next step is a rebase/merge plus a small adaptation so we keep your APIs without duplicating QuerySet.

Really appreciate the work you already put in — especially completing the companion methods and RenderBundle surface after the first review round. Once this is synced with main, the remaining slice should be clean and close to merge-ready.

---

Goal after update

Keep from #337	Already on main (#347) — do not reintroduce
renderbundle_native.go	query_native.go (use main’s version)
Device.CreateRenderBundleEncoder	Device.CreateQuerySet / QuerySet / QuerySetDescriptor
RenderPassEncoder.ExecuteBundles	feature gates + core.ValidateQuerySetDescriptor
CommandEncoder.ResolveQuerySet	QuerySet-only unit tests already covered on main
RenderPassDescriptor.TimestampWrites
RenderBundle / Resolve / Timestamp tests

---

Step-by-step

1) Sync your fork branch with upstream main

git remote add upstream https://github.com/gogpu/wgpu.git   # skip if already added
git fetch upstream
git checkout feat/public-query-set-render-bundle
git merge upstream/main
# alternative: git rebase upstream/main

Expect conflicts mainly in:

• query_native.go (added in both)
• descriptor.go
• device_native.go
• renderpass_native.go

2) query_native.go — take main’s implementation

Keep the #347 version:

• core *core.QuerySet (not a raw hal.QuerySet field)
• Type() / Count()
• DestroyQueue-deferred Release()
• CreateQuerySet lives in this file on main

Quick check: the file should import github.com/gogpu/wgpu/core and call core.NewQuerySet(...).

3) device_native.go — drop duplicate CreateQuerySet, keep RenderBundle

• Remove your CreateQuerySet (already on main in query_native.go)
• Keep CreateRenderBundleEncoder

4) descriptor.go — don’t duplicate QuerySet types

• Keep main’s QuerySetDescriptor / QueryType aliases
• From your PR, add:
  • RenderBundleEncoderDescriptor
  • RenderBundleDescriptor
  • RenderPassTimestampWrites
  • TimestampWrites on RenderPassDescriptor + toHAL() wiring

5) Adapt ResolveQuerySet + TimestampWrites to main’s QuerySet

On main, QuerySet no longer has .hal or released atomic.Bool. Add a helper similar to Buffer.halBuffer():

func (qs *QuerySet) halQuerySet() hal.QuerySet {
	if qs == nil || qs.core == nil || qs.device == nil {
		return nil
	}
	guard := qs.device.core.SnatchLock().Read()
	defer guard.Release()
	return qs.core.Raw(guard)
}

Then update call sites:

// before (your PR)
if querySet.released.Load() || querySet.hal == nil { ... }
raw.ResolveQuerySet(querySet.hal, firstQuery, queryCount, dst, dstOffset)

// after (main QuerySet)
if querySet.released || querySet.halQuerySet() == nil { ... }
raw.ResolveQuerySet(querySet.halQuerySet(), firstQuery, queryCount, dst, dstOffset)

Same idea in RenderPassDescriptor.toHAL() for TimestampWrites.QuerySet.

6) Fix tests in query_renderbundle_native_test.go

• Remove / rewrite tests that construct the old QuerySet{hal: ...} shape or re-test CreateQuerySet in isolation (main already has query_native_test.go)
• Keep tests for RenderBundle Finish / post-Finish guards, ExecuteBundles, ResolveQuerySet, and TimestampWrites → HAL mapping
• For QuerySet objects in those tests, use main’s struct (core + device) or go through halQuerySet()

7) Verify locally, then push

go test ./... -count=1
go test . -run 'Query|RenderBundle|ResolveQuery|Timestamp' -count=1

git add -A
git commit -m "fix: rebase on main after #347; keep render bundle + resolve/timestamp"
git push origin feat/public-query-set-render-bundle
# if you rebased instead of merging:
# git push --force-with-lease

When CI is green, comment here and we’ll do a full re-review of the remaining slice (RenderBundle + ResolveQuerySet + TimestampWrites).

---

If you hit a tricky conflict hunk, a merge/rebase question, or a failing test — ping us right in this PR and we’ll sort it out together. Thanks again for pushing #337 forward; this next sync should make it a really solid, mergeable contribution 🚀

[gusevgrishaem1]

Thanks for the feedback! I’ve updated MR according to the "Step-by-step". Could you please take another look?

[lkmavi]

Hi @gusevgrishaem1 — thanks for following the step-by-step after #347 🙏

Sync looks good:

• query_native.go kept main’s #347 QuerySet (core + deferred Release) and only added halQuerySet()
• no duplicate CreateQuerySet / QuerySet types
• RenderBundle surface, ResolveQuerySet, Finish(desc), post-Finish guards, and tests are in good shape
• CI is green

One blocking issue before merge:

TimestampWrites never reach the HAL via BeginRenderPass

RenderPassDescriptor.TimestampWrites is wired only in (*RenderPassDescriptor).toHAL(), but BeginRenderPass does not call that path. It goes through convertRenderPassDesc → core.RenderPassDescriptor → core.convertRenderPassDescriptor, and neither core type nor converter carries timestamp writes today.

So setting TimestampWrites on a public render-pass descriptor has no effect at runtime. TestRenderPassDescriptorTimestampWrites only exercises .toHAL(), which is why this slipped past CI.

Please wire it through the real path, e.g.:

1. add TimestampWrites to core.RenderPassDescriptor (mirror the compute-pass pattern)
2. map it in convertRenderPassDesc
3. forward it in core.convertRenderPassDescriptor to hal.RenderPassTimestampWrites
4. add a test that asserts the HAL render-pass begin sees the writes (not only public toHAL())

Why this wasn’t caught in the earlier review rounds

This isn’t a regression from the #347 rebase — the gap was already present after the first companion-API fix. The first review checklist was effectively “spec / HAL / public API”: HAL already had TimestampWrites, so adding the public field + a toHAL() mapping looked complete.

But render passes don’t go desc.toHAL() → HAL (unlike many Device.Create* APIs). The live path is public → core → HAL. Public toHAL() is effectively unused by BeginRenderPass, so the mapping sat on a dead converter. After #347 we focused on QuerySet dedup / halQuerySet() / keeping the RenderBundle slice — we didn’t re-walk that call graph. ResolveQuerySet / ExecuteBundles are fine (they call HAL directly); TimestampWrites is the odd one out.

Non-blocking follow-up (nice to fix in this PR if easy)

RenderBundle.Release() destroys immediately. Prefer DestroyQueue-deferred destroy (ADR-056), same pattern as ShaderModule / Sampler / QuerySet, so a bundle still referenced by an in-flight submission isn’t torn down early. Also guard b.device == nil before halDevice().

Happy to re-review as soon as TimestampWrites actually flow through BeginRenderPass. Appreciate the careful rebase work — this is close!