From d8c55b4879d1e5d6d8459bc6fc226b40d476646d Mon Sep 17 00:00:00 2001 From: Mark Date: Wed, 15 Jul 2026 18:09:30 +0300 Subject: [PATCH 1/4] feat(vulkan): qualify surface adapters with present queues --- core/instance.go | 145 ++++++++++-- core/instance_surface_test.go | 125 ++++++++++ hal/vulkan/adapter.go | 351 +++++++++++++++++++++++++---- hal/vulkan/adapter_surface_test.go | 131 +++++++++++ hal/vulkan/api.go | 23 +- 5 files changed, 701 insertions(+), 74 deletions(-) create mode 100644 core/instance_surface_test.go create mode 100644 hal/vulkan/adapter_surface_test.go diff --git a/core/instance.go b/core/instance.go index 3e65c3b9..33a49821 100644 --- a/core/instance.go +++ b/core/instance.go @@ -27,6 +27,11 @@ type Instance struct { // adapters contains the registered adapter IDs. adapters []AdapterID + // surfaceAdapters contains request-local adapters created for a compatible + // surface. They are intentionally kept out of adapters so ordinary adapter + // enumeration and selection never retain surface-bound queue state. + surfaceAdapters []AdapterID + // halInstances tracks HAL instances created for each backend. // These are destroyed when the Instance is destroyed. halInstances []hal.Instance @@ -50,6 +55,14 @@ type Instance struct { useMock bool } +// surfaceAdapterQualifier is an optional backend capability. It intentionally +// stays private to core so the stable HAL Adapter interface does not grow a +// surface-specific method. Vulkan implements the method on its concrete +// adapter and returns a request-local wrapper with queue/surface state. +type surfaceAdapterQualifier interface { + QualifySurface(surface hal.Surface) (hal.Adapter, error) +} + // NewInstance creates a new WebGPU instance with the given descriptor. // If desc is nil, default settings are used. // @@ -257,16 +270,26 @@ func (i *Instance) RequestAdapter(options *gputypes.RequestAdapterOptions) (Adap i.enumerateDeferredGLES(nil) i.mu.RLock() - defer i.mu.RUnlock() + adapterIDs := append([]AdapterID(nil), i.adapters...) + i.mu.RUnlock() - if len(i.adapters) == 0 { + return selectAdapterIDs(options, adapterIDs) +} + +// selectAdapterIDs applies the public adapter selection policy to an explicit +// candidate list. Keeping the policy independent from Instance state lets a +// surface request select request-local adapters without exposing unqualified +// cached adapters to the selection pass. +func selectAdapterIDs(options *gputypes.RequestAdapterOptions, adapterIDs []AdapterID) (AdapterID, error) { //nolint:gocognit // adapter selection with GPU preference logic + if len(adapterIDs) == 0 { return AdapterID{}, fmt.Errorf("no adapters available") } + hub := GetGlobal().Hub() + // If no options specified, prefer non-CPU adapters (GPU > Software fallback). if options == nil { - hub := GetGlobal().Hub() - for _, adapterID := range i.adapters { + for _, adapterID := range adapterIDs { adapter, err := hub.GetAdapter(adapterID) if err != nil { continue @@ -275,14 +298,12 @@ func (i *Instance) RequestAdapter(options *gputypes.RequestAdapterOptions) (Adap return adapterID, nil } } - return i.adapters[0], nil // fallback to first (Software) + return adapterIDs[0], nil // fallback to first (Software) } - hub := GetGlobal().Hub() - - // ForceFallbackAdapter: return first CPU adapter directly + // ForceFallbackAdapter: return first CPU adapter directly. if options.ForceFallbackAdapter { - for _, adapterID := range i.adapters { + for _, adapterID := range adapterIDs { adapter, err := hub.GetAdapter(adapterID) if err != nil { continue @@ -304,7 +325,7 @@ func (i *Instance) RequestAdapter(options *gputypes.RequestAdapterOptions) (Adap hasGPUFallback := false // Pass 1: find preferred adapter + track fallbacks. - for _, adapterID := range i.adapters { + for _, adapterID := range adapterIDs { adapter, err := hub.GetAdapter(adapterID) if err != nil { continue @@ -318,13 +339,13 @@ func (i *Instance) RequestAdapter(options *gputypes.RequestAdapterOptions) (Adap continue } - // Track first non-CPU adapter as GPU fallback + // Track first non-CPU adapter as GPU fallback. if !hasGPUFallback { gpuFallback = adapterID hasGPUFallback = true } - // Check power preference — return immediately on exact match + // Check power preference — return immediately on exact match. if options.PowerPreference != gputypes.PowerPreferenceNone { if matchesPowerPreference(adapter.Info.DeviceType, options.PowerPreference) { return adapterID, nil @@ -355,11 +376,89 @@ func (i *Instance) RequestAdapter(options *gputypes.RequestAdapterOptions) (Adap // a GL context, which is required before EnumerateAdapters can return a real // adapter. Calling without a surface yields a zero-value Adapter (nil glCtx). func (i *Instance) RequestAdapterWithSurface(options *gputypes.RequestAdapterOptions, surfaceHint hal.Surface) (AdapterID, error) { - // Run deferred GLES enumeration with the real surface hint BEFORE RequestAdapter - // calls enumerateDeferredGLES(nil). Once glesEnumerated is true the nil call - // inside RequestAdapter becomes a no-op, so the surface-backed adapters win. + if surfaceHint == nil { + return i.RequestAdapter(options) + } + + // Remember which adapters existed before deferred enumeration. Adapters + // created by EnumerateAdapters(surfaceHint) are already surface-qualified by + // their backend and can be used directly. Cached adapters are qualified via + // the optional HAL SurfaceAdapter interface below. + i.mu.RLock() + before := make(map[AdapterID]struct{}, len(i.adapters)) + for _, adapterID := range i.adapters { + before[adapterID] = struct{}{} + } + i.mu.RUnlock() + + // Run deferred GLES enumeration with the real surface hint before collecting + // candidates. Once glesEnumerated is true this is a no-op on later calls. i.enumerateDeferredGLES(surfaceHint) - return i.RequestAdapter(options) + + i.mu.RLock() + allAdapterIDs := append([]AdapterID(nil), i.adapters...) + i.mu.RUnlock() + + hub := GetGlobal().Hub() + i.mu.RLock() + usingMock := i.useMock + i.mu.RUnlock() + candidates := make([]AdapterID, 0, len(allAdapterIDs)) + for _, adapterID := range allAdapterIDs { + adapter, err := hub.GetAdapter(adapterID) + if err != nil { + continue + } + + // A newly enumerated adapter was produced with the surface hint. Keep + // that backend-owned adapter as-is; this preserves existing GLES + // enumeration behavior and avoids a second context query. + if _, isNew := before[adapterID]; !isNew { + candidates = append(candidates, adapterID) + continue + } + + if adapter.halAdapter == nil { + if usingMock { + // Explicit mock instances have no native surface query. Preserve + // their established behavior instead of turning a test adapter + // request into an unexpected compatibility error. + candidates = append(candidates, adapterID) + } + continue + } + + if qualifier, ok := adapter.halAdapter.(surfaceAdapterQualifier); ok { + qualified, err := qualifier.QualifySurface(surfaceHint) + if err != nil || qualified == nil { + continue + } + + // Register a request-local core adapter. The cached adapter and its + // HAL object are left untouched; only the wrapper carries the queue + // family and checked surface snapshot into Open(). + qualifiedCore := adapter + qualifiedCore.halAdapter = qualified + qualifiedID := hub.RegisterAdapter(&qualifiedCore) + i.mu.Lock() + i.surfaceAdapters = append(i.surfaceAdapters, qualifiedID) + i.mu.Unlock() + candidates = append(candidates, qualifiedID) + continue + } + + // Backends without a request-local qualifier may still expose a + // checked surface capability query. Do not treat a nil result as + // compatible, and never fall back to an unqualified Vulkan adapter. + if adapter.halAdapter.SurfaceCapabilities(surfaceHint) != nil { + candidates = append(candidates, adapterID) + } + } + + if len(candidates) == 0 { + return AdapterID{}, fmt.Errorf("no adapters compatible with surface") + } + return selectAdapterIDs(options, candidates) } // enumerateDeferredGLES enumerates adapters for deferred GLES HAL instances. @@ -545,6 +644,20 @@ func (i *Instance) Destroy() { } i.adapters = nil + // Request-local surface adapters are separate from the ordinary adapter + // list, but still belong to this instance and must be released here. + for _, adapterID := range i.surfaceAdapters { + adapter, err := hub.GetAdapter(adapterID) + if err != nil { + continue + } + if adapter.halAdapter != nil { + adapter.halAdapter.Destroy() + } + _, _ = hub.UnregisterAdapter(adapterID) + } + i.surfaceAdapters = nil + // Destroy all HAL instances (includes deferred GLES instances). for _, halInstance := range i.halInstances { halInstance.Destroy() diff --git a/core/instance_surface_test.go b/core/instance_surface_test.go new file mode 100644 index 00000000..378564fe --- /dev/null +++ b/core/instance_surface_test.go @@ -0,0 +1,125 @@ +//go:build !(js && wasm) + +package core + +import ( + "errors" + "testing" + + "github.com/gogpu/gputypes" + "github.com/gogpu/wgpu/hal" +) + +type surfaceQualificationAdapter struct { + compatible bool + qualifies *int +} + +func (a *surfaceQualificationAdapter) Open(_ gputypes.Features, _ gputypes.Limits) (hal.OpenDevice, error) { + return hal.OpenDevice{}, nil +} + +func (a *surfaceQualificationAdapter) TextureFormatCapabilities(_ gputypes.TextureFormat) hal.TextureFormatCapabilities { + return hal.TextureFormatCapabilities{} +} + +func (a *surfaceQualificationAdapter) SurfaceCapabilities(_ hal.Surface) *hal.SurfaceCapabilities { + return nil +} + +func (a *surfaceQualificationAdapter) Destroy() {} + +func (a *surfaceQualificationAdapter) QualifySurface(_ hal.Surface) (hal.Adapter, error) { + if a.qualifies != nil { + (*a.qualifies)++ + } + if !a.compatible { + return nil, errors.New("surface is not supported") + } + return &surfaceQualificationAdapter{compatible: true}, nil +} + +func TestRequestAdapterWithSurfaceUsesRequestLocalQualification(t *testing.T) { + GetGlobal().Clear() + hub := GetGlobal().Hub() + + firstCalls := 0 + secondCalls := 0 + firstHAL := &surfaceQualificationAdapter{qualifies: &firstCalls} + secondHAL := &surfaceQualificationAdapter{compatible: true, qualifies: &secondCalls} + firstID := hub.RegisterAdapter(&Adapter{ + Info: gputypes.AdapterInfo{DeviceType: gputypes.DeviceTypeDiscreteGPU, Backend: gputypes.BackendVulkan}, + Limits: gputypes.DefaultLimits(), + Backend: gputypes.BackendVulkan, + halAdapter: firstHAL, + }) + secondID := hub.RegisterAdapter(&Adapter{ + Info: gputypes.AdapterInfo{DeviceType: gputypes.DeviceTypeIntegratedGPU, Backend: gputypes.BackendVulkan}, + Limits: gputypes.DefaultLimits(), + Backend: gputypes.BackendVulkan, + halAdapter: secondHAL, + }) + + instance := &Instance{ + backends: gputypes.BackendsVulkan, + adapters: []AdapterID{firstID, secondID}, + } + defer instance.Destroy() + + selectedID, err := instance.RequestAdapterWithSurface(nil, &stubHALSurface{id: 7}) + if err != nil { + t.Fatalf("RequestAdapterWithSurface() error: %v", err) + } + if selectedID == firstID || selectedID == secondID { + t.Fatalf("selected cached adapter %v; want request-local qualified adapter", selectedID) + } + if firstCalls != 1 || secondCalls != 1 { + t.Fatalf("qualification calls = (%d, %d), want (1, 1)", firstCalls, secondCalls) + } + + selected, err := hub.GetAdapter(selectedID) + if err != nil { + t.Fatalf("GetAdapter(selected) error: %v", err) + } + if selected.halAdapter == secondHAL { + t.Fatal("selected adapter retained cached HAL adapter") + } + if selected.Info.DeviceType != gputypes.DeviceTypeIntegratedGPU { + t.Fatalf("selected device type = %v, want integrated GPU", selected.Info.DeviceType) + } + if len(instance.adapters) != 2 || len(instance.surfaceAdapters) != 1 { + t.Fatalf("adapter ownership = (%d ordinary, %d surface), want (2, 1)", len(instance.adapters), len(instance.surfaceAdapters)) + } + + ordinaryID, err := instance.RequestAdapter(nil) + if err != nil { + t.Fatalf("ordinary RequestAdapter() error: %v", err) + } + if ordinaryID != firstID { + t.Fatalf("ordinary RequestAdapter() = %v, want cached first adapter %v", ordinaryID, firstID) + } + + selectedAgain, err := instance.RequestAdapterWithSurface(nil, &stubHALSurface{id: 8}) + if err != nil { + t.Fatalf("second RequestAdapterWithSurface() error: %v", err) + } + if selectedAgain == firstID || selectedAgain == secondID { + t.Fatal("second surface request returned a cached adapter") + } + if len(instance.adapters) != 2 || len(instance.surfaceAdapters) != 2 { + t.Fatalf("adapter ownership after second request = (%d ordinary, %d surface), want (2, 2)", len(instance.adapters), len(instance.surfaceAdapters)) + } + if firstCalls != 2 || secondCalls != 2 { + t.Fatalf("qualification calls after second request = (%d, %d), want (2, 2)", firstCalls, secondCalls) + } + + // The cached adapter remains unchanged and is still available for a + // surface-independent request. + cached, err := hub.GetAdapter(secondID) + if err != nil { + t.Fatalf("GetAdapter(cached) error: %v", err) + } + if cached.halAdapter != secondHAL { + t.Fatal("surface request mutated cached adapter") + } +} diff --git a/hal/vulkan/adapter.go b/hal/vulkan/adapter.go index 0a972e37..7fa89c8b 100644 --- a/hal/vulkan/adapter.go +++ b/hal/vulkan/adapter.go @@ -25,6 +25,13 @@ type Adapter struct { // Open creates a logical device with the requested features and limits. func (a *Adapter) Open(features gputypes.Features, limits gputypes.Limits) (hal.OpenDevice, error) { + return a.open(features, limits, nil) +} + +// open creates a logical device, optionally constraining it to one queue +// family. Surface-qualified adapters use the constrained path so the queue +// selected during the surface query is the same queue passed into Open. +func (a *Adapter) open(features gputypes.Features, limits gputypes.Limits, requestedQueueFamily *uint32) (hal.OpenDevice, error) { // Find queue families var queueFamilyCount uint32 vkGetPhysicalDeviceQueueFamilyProperties(a.instance, a.physicalDevice, &queueFamilyCount, nil) @@ -36,12 +43,28 @@ func (a *Adapter) Open(features gputypes.Features, limits gputypes.Limits) (hal. queueFamilies := make([]vk.QueueFamilyProperties, queueFamilyCount) vkGetPhysicalDeviceQueueFamilyProperties(a.instance, a.physicalDevice, &queueFamilyCount, &queueFamilies[0]) - // Find graphics queue family + // Find a graphics queue family. A surface-qualified adapter supplies the + // exact family selected by QualifySurface; unconstrained opens preserve the + // existing headless behavior and choose the first graphics family. graphicsFamily := int32(-1) - for i, family := range queueFamilies { - if family.QueueFlags&vk.QueueFlags(vk.QueueGraphicsBit) != 0 { - graphicsFamily = int32(i) - break + if requestedQueueFamily != nil { + familyIndex := *requestedQueueFamily + if familyIndex >= queueFamilyCount || familyIndex >= uint32(len(queueFamilies)) { + return hal.OpenDevice{}, fmt.Errorf("vulkan: requested queue family %d is unavailable", familyIndex) + } + if queueFamilies[familyIndex].QueueCount == 0 { + return hal.OpenDevice{}, fmt.Errorf("vulkan: requested queue family %d has no queues", familyIndex) + } + if queueFamilies[familyIndex].QueueFlags&vk.QueueFlags(vk.QueueGraphicsBit) == 0 { + return hal.OpenDevice{}, fmt.Errorf("vulkan: requested queue family %d has no graphics queue", familyIndex) + } + graphicsFamily = int32(familyIndex) + } else { + for i, family := range queueFamilies { + if family.QueueCount > 0 && family.QueueFlags&vk.QueueFlags(vk.QueueGraphicsBit) != 0 { + graphicsFamily = int32(i) + break + } } } @@ -242,70 +265,306 @@ func (a *Adapter) TextureFormatCapabilities(format gputypes.TextureFormat) hal.T // SurfaceCapabilities returns surface capabilities. func (a *Adapter) SurfaceCapabilities(surface hal.Surface) *hal.SurfaceCapabilities { vkSurface, ok := surface.(*Surface) - if !ok || vkSurface == nil || vkSurface.handle == 0 { + if !ok || vkSurface == nil || vkSurface.instance == nil || vkSurface.instance != a.instance { return nil } - // Query surface capabilities for alpha modes - var surfaceCaps vk.SurfaceCapabilitiesKHR - a.instance.cmds.GetPhysicalDeviceSurfaceCapabilitiesKHR( - a.physicalDevice, vkSurface.handle, &surfaceCaps) + snapshot, err := a.querySurfaceSnapshot(vkSurface) + if err != nil { + return nil + } + return cloneSurfaceCapabilities(snapshot.public) +} - // Query supported surface formats - var formatCount uint32 - a.instance.cmds.GetPhysicalDeviceSurfaceFormatsKHR( - a.physicalDevice, vkSurface.handle, &formatCount, nil) +// surfaceSnapshot keeps the exact Vulkan format/color-space pairs alongside +// the public projection. The public HAL shape predates color spaces, so the +// raw pairs stay private to the Vulkan adapter and are never silently reduced +// to a fabricated format list. +type surfaceSnapshot struct { + capabilities vk.SurfaceCapabilitiesKHR + formats []vk.SurfaceFormatKHR + presentModes []vk.PresentModeKHR + public hal.SurfaceCapabilities +} - formats := make([]gputypes.TextureFormat, 0, formatCount) - if formatCount > 0 { - vkFormats := make([]vk.SurfaceFormatKHR, formatCount) - a.instance.cmds.GetPhysicalDeviceSurfaceFormatsKHR( - a.physicalDevice, vkSurface.handle, &formatCount, &vkFormats[0]) +// qualifiedAdapter is a request-local Vulkan adapter. It carries the queue +// family proven to support both graphics and presentation for one surface and +// opens the underlying physical device with that family unchanged. +type qualifiedAdapter struct { + base *Adapter + surface *Surface + queueFamily uint32 + snapshot surfaceSnapshot +} - for _, f := range vkFormats { - if tf := vkFormatToTextureFormat(f.Format); tf != gputypes.TextureFormatUndefined { - formats = append(formats, tf) - } +func (a *qualifiedAdapter) Open(features gputypes.Features, limits gputypes.Limits) (hal.OpenDevice, error) { + return a.base.open(features, limits, &a.queueFamily) +} + +func (a *qualifiedAdapter) TextureFormatCapabilities(format gputypes.TextureFormat) hal.TextureFormatCapabilities { + return a.base.TextureFormatCapabilities(format) +} + +func (a *qualifiedAdapter) SurfaceCapabilities(surface hal.Surface) *hal.SurfaceCapabilities { + vkSurface, ok := surface.(*Surface) + if !ok || vkSurface != a.surface { + return nil + } + return cloneSurfaceCapabilities(a.snapshot.public) +} + +// Destroy is intentionally a no-op. The cached physical adapter owns no +// Vulkan object, and the core instance retains ownership of the base adapter. +func (a *qualifiedAdapter) Destroy() {} + +// QualifySurface selects a same-family graphics+present queue and records a +// checked surface capability snapshot without mutating the cached adapter. +func (a *Adapter) QualifySurface(surface hal.Surface) (hal.Adapter, error) { + vkSurface, ok := surface.(*Surface) + if !ok || vkSurface == nil || vkSurface.handle == 0 { + return nil, fmt.Errorf("vulkan: invalid surface for adapter qualification") + } + if vkSurface.instance == nil || vkSurface.instance != a.instance { + return nil, fmt.Errorf("vulkan: surface belongs to a different instance") + } + + queueFamilies, err := a.queueFamilies() + if err != nil { + return nil, err + } + queueFamily, err := a.presentGraphicsQueueFamily(vkSurface, queueFamilies) + if err != nil { + return nil, err + } + + snapshot, err := a.querySurfaceSnapshot(vkSurface) + if err != nil { + return nil, err + } + + return &qualifiedAdapter{ + base: a, + surface: vkSurface, + queueFamily: queueFamily, + snapshot: snapshot, + }, nil +} + +func (a *Adapter) queueFamilies() ([]vk.QueueFamilyProperties, error) { + var count uint32 + vkGetPhysicalDeviceQueueFamilyProperties(a.instance, a.physicalDevice, &count, nil) + if count == 0 { + return nil, fmt.Errorf("vulkan: no queue families found") + } + + families := make([]vk.QueueFamilyProperties, count) + vkGetPhysicalDeviceQueueFamilyProperties(a.instance, a.physicalDevice, &count, &families[0]) + if count == 0 { + return nil, fmt.Errorf("vulkan: no queue families found") + } + if count < uint32(len(families)) { + families = families[:count] + } + return families, nil +} + +func (a *Adapter) presentGraphicsQueueFamily(surface *Surface, families []vk.QueueFamilyProperties) (uint32, error) { + supports := make([]bool, len(families)) + for index, family := range families { + if family.QueueCount == 0 || family.QueueFlags&vk.QueueFlags(vk.QueueGraphicsBit) == 0 { + continue } + + var supported vk.Bool32 + result := a.instance.cmds.GetPhysicalDeviceSurfaceSupportKHR( + a.physicalDevice, uint32(index), surface.handle, &supported) + if result != vk.Success { + return 0, fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfaceSupportKHR(queueFamily=%d) failed: %d", index, result) + } + supports[index] = supported != 0 } + return selectPresentGraphicsQueueFamily(families, supports) +} - // Fallback if no formats found - if len(formats) == 0 { - formats = []gputypes.TextureFormat{ - gputypes.TextureFormatBGRA8Unorm, - gputypes.TextureFormatRGBA8Unorm, +func selectPresentGraphicsQueueFamily(families []vk.QueueFamilyProperties, supports []bool) (uint32, error) { + if len(supports) != len(families) { + return 0, fmt.Errorf("vulkan: queue family support query is incomplete") + } + for index, family := range families { + if family.QueueCount > 0 && family.QueueFlags&vk.QueueFlags(vk.QueueGraphicsBit) != 0 && supports[index] { + return uint32(index), nil } } + return 0, fmt.Errorf("vulkan: no graphics queue family supports presentation for surface") +} - // Query supported present modes - var modeCount uint32 - a.instance.cmds.GetPhysicalDeviceSurfacePresentModesKHR( - a.physicalDevice, vkSurface.handle, &modeCount, nil) +func (a *Adapter) querySurfaceSnapshot(surface *Surface) (surfaceSnapshot, error) { + if surface == nil || surface.handle == 0 { + return surfaceSnapshot{}, fmt.Errorf("vulkan: invalid surface") + } + + var capabilities vk.SurfaceCapabilitiesKHR + result := a.instance.cmds.GetPhysicalDeviceSurfaceCapabilitiesKHR( + a.physicalDevice, surface.handle, &capabilities) + if result != vk.Success { + return surfaceSnapshot{}, fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfaceCapabilitiesKHR failed: %d", result) + } + + formats, err := querySurfaceFormats(a.instance, a.physicalDevice, surface.handle) + if err != nil { + return surfaceSnapshot{}, err + } + + presentModes, err := queryPresentModes(a.instance, a.physicalDevice, surface.handle) + if err != nil { + return surfaceSnapshot{}, err + } + return makeSurfaceSnapshot(capabilities, formats, presentModes) +} + +func querySurfaceFormats(instance *Instance, device vk.PhysicalDevice, surface vk.SurfaceKHR) ([]vk.SurfaceFormatKHR, error) { + return querySurfaceFormatsWith(func(count *uint32, formats *vk.SurfaceFormatKHR) vk.Result { + return instance.cmds.GetPhysicalDeviceSurfaceFormatsKHR(device, surface, count, formats) + }) +} - presentModes := make([]hal.PresentMode, 0, modeCount) - if modeCount > 0 { - vkModes := make([]vk.PresentModeKHR, modeCount) - a.instance.cmds.GetPhysicalDeviceSurfacePresentModesKHR( - a.physicalDevice, vkSurface.handle, &modeCount, &vkModes[0]) +func querySurfaceFormatsWith(query func(count *uint32, formats *vk.SurfaceFormatKHR) vk.Result) ([]vk.SurfaceFormatKHR, error) { + for attempt := 0; attempt < 2; attempt++ { + var count uint32 + result := query(&count, nil) + if result != vk.Success && result != vk.Incomplete { + return nil, fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfaceFormatsKHR (count) failed: %d", result) + } + if count == 0 { + return nil, nil //nolint:nilnil // an empty query is a checked incompatible result + } + + formats := make([]vk.SurfaceFormatKHR, count) + returned := count + result = query(&returned, &formats[0]) + if result != vk.Success && result != vk.Incomplete { + return nil, fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfaceFormatsKHR failed: %d", result) + } + if result == vk.Incomplete || returned > uint32(len(formats)) { + continue + } + return formats[:returned], nil + } + return nil, fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfaceFormatsKHR returned an unstable count") +} - for _, m := range vkModes { - presentModes = append(presentModes, vkPresentModeToHAL(m)) +func queryPresentModes(instance *Instance, device vk.PhysicalDevice, surface vk.SurfaceKHR) ([]vk.PresentModeKHR, error) { + return queryPresentModesWith(func(count *uint32, modes *vk.PresentModeKHR) vk.Result { + return instance.cmds.GetPhysicalDeviceSurfacePresentModesKHR(device, surface, count, modes) + }) +} + +func queryPresentModesWith(query func(count *uint32, modes *vk.PresentModeKHR) vk.Result) ([]vk.PresentModeKHR, error) { + for attempt := 0; attempt < 2; attempt++ { + var count uint32 + result := query(&count, nil) + if result != vk.Success && result != vk.Incomplete { + return nil, fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfacePresentModesKHR (count) failed: %d", result) + } + if count == 0 { + return nil, nil //nolint:nilnil // an empty query is a checked incompatible result + } + + modes := make([]vk.PresentModeKHR, count) + returned := count + result = query(&returned, &modes[0]) + if result != vk.Success && result != vk.Incomplete { + return nil, fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfacePresentModesKHR failed: %d", result) + } + if result == vk.Incomplete || returned > uint32(len(modes)) { + continue } + return modes[:returned], nil } + return nil, fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfacePresentModesKHR returned an unstable count") +} - // Fallback if no present modes found +func makeSurfaceSnapshot(capabilities vk.SurfaceCapabilitiesKHR, formats []vk.SurfaceFormatKHR, presentModes []vk.PresentModeKHR) (surfaceSnapshot, error) { + if len(formats) == 0 { + return surfaceSnapshot{}, fmt.Errorf("vulkan: surface returned no formats") + } if len(presentModes) == 0 { - presentModes = []hal.PresentMode{hal.PresentModeFifo} + return surfaceSnapshot{}, fmt.Errorf("vulkan: surface returned no present modes") } - // Convert composite alpha modes - alphaModes := vkCompositeAlphaToHAL(surfaceCaps.SupportedCompositeAlpha) + alphaModes := vkCompositeAlphaToHALChecked(capabilities.SupportedCompositeAlpha) + if len(alphaModes) == 0 { + return surfaceSnapshot{}, fmt.Errorf("vulkan: surface returned no supported composite alpha modes") + } - return &hal.SurfaceCapabilities{ - Formats: formats, - PresentModes: presentModes, + public := hal.SurfaceCapabilities{ + Formats: make([]gputypes.TextureFormat, 0, len(formats)), + PresentModes: make([]hal.PresentMode, 0, len(presentModes)), AlphaModes: alphaModes, } + for _, format := range formats { + if textureFormat := vkFormatToTextureFormat(format.Format); textureFormat != gputypes.TextureFormatUndefined { + public.Formats = append(public.Formats, textureFormat) + } + } + for _, mode := range presentModes { + if halMode, ok := vkPresentModeToHALChecked(mode); ok { + public.PresentModes = append(public.PresentModes, halMode) + } + } + if len(public.PresentModes) == 0 { + return surfaceSnapshot{}, fmt.Errorf("vulkan: surface returned no supported present modes") + } + if len(public.Formats) == 0 { + return surfaceSnapshot{}, fmt.Errorf("vulkan: surface returned no supported formats") + } + + return surfaceSnapshot{ + capabilities: capabilities, + formats: append([]vk.SurfaceFormatKHR(nil), formats...), + presentModes: append([]vk.PresentModeKHR(nil), presentModes...), + public: public, + }, nil +} + +func vkPresentModeToHALChecked(mode vk.PresentModeKHR) (hal.PresentMode, bool) { + switch mode { + case vk.PresentModeImmediateKhr: + return hal.PresentModeImmediate, true + case vk.PresentModeMailboxKhr: + return hal.PresentModeMailbox, true + case vk.PresentModeFifoKhr: + return hal.PresentModeFifo, true + case vk.PresentModeFifoRelaxedKhr: + return hal.PresentModeFifoRelaxed, true + default: + return 0, false + } +} + +func vkCompositeAlphaToHALChecked(flags vk.CompositeAlphaFlagsKHR) []hal.CompositeAlphaMode { + var modes []hal.CompositeAlphaMode + if vk.Flags(flags)&vk.Flags(vk.CompositeAlphaOpaqueBitKhr) != 0 { + modes = append(modes, hal.CompositeAlphaModeOpaque) + } + if vk.Flags(flags)&vk.Flags(vk.CompositeAlphaPreMultipliedBitKhr) != 0 { + modes = append(modes, hal.CompositeAlphaModePremultiplied) + } + if vk.Flags(flags)&vk.Flags(vk.CompositeAlphaPostMultipliedBitKhr) != 0 { + modes = append(modes, hal.CompositeAlphaModeUnpremultiplied) + } + if vk.Flags(flags)&vk.Flags(vk.CompositeAlphaInheritBitKhr) != 0 { + modes = append(modes, hal.CompositeAlphaModeInherit) + } + return modes +} + +func cloneSurfaceCapabilities(capabilities hal.SurfaceCapabilities) *hal.SurfaceCapabilities { + return &hal.SurfaceCapabilities{ + Formats: append([]gputypes.TextureFormat(nil), capabilities.Formats...), + PresentModes: append([]hal.PresentMode(nil), capabilities.PresentModes...), + AlphaModes: append([]hal.CompositeAlphaMode(nil), capabilities.AlphaModes...), + } } // Destroy releases the adapter. diff --git a/hal/vulkan/adapter_surface_test.go b/hal/vulkan/adapter_surface_test.go new file mode 100644 index 00000000..c18a4364 --- /dev/null +++ b/hal/vulkan/adapter_surface_test.go @@ -0,0 +1,131 @@ +//go:build !(js && wasm) + +package vulkan + +import ( + "testing" + + "github.com/gogpu/gputypes" + "github.com/gogpu/wgpu/hal/vulkan/vk" +) + +func TestSelectPresentGraphicsQueueFamilyRejectsSplitQueues(t *testing.T) { + families := []vk.QueueFamilyProperties{ + {QueueFlags: vk.QueueFlags(vk.QueueGraphicsBit), QueueCount: 1}, + {QueueFlags: vk.QueueFlags(vk.QueueTransferBit), QueueCount: 1}, + } + + if _, err := selectPresentGraphicsQueueFamily(families, []bool{false, true}); err == nil { + t.Fatal("split graphics/present queues were accepted") + } +} + +func TestSelectPresentGraphicsQueueFamilyUsesSameFamily(t *testing.T) { + families := []vk.QueueFamilyProperties{ + {QueueFlags: vk.QueueFlags(vk.QueueGraphicsBit), QueueCount: 1}, + {QueueFlags: vk.QueueFlags(vk.QueueGraphicsBit), QueueCount: 1}, + } + + got, err := selectPresentGraphicsQueueFamily(families, []bool{false, true}) + if err != nil { + t.Fatalf("selectPresentGraphicsQueueFamily() error: %v", err) + } + if got != 1 { + t.Fatalf("selected queue family = %d, want 1", got) + } +} + +func TestQuerySurfaceFormatsFailsClosed(t *testing.T) { + _, err := querySurfaceFormatsWith(func(_ *uint32, _ *vk.SurfaceFormatKHR) vk.Result { + return vk.ErrorSurfaceLostKhr + }) + if err == nil { + t.Fatal("failed surface format query was accepted") + } +} + +func TestQuerySurfaceFormatsRetriesIncomplete(t *testing.T) { + countCalls := 0 + fillCalls := 0 + formats, err := querySurfaceFormatsWith(func(count *uint32, formats *vk.SurfaceFormatKHR) vk.Result { + if formats == nil { + countCalls++ + *count = 1 + return vk.Success + } + fillCalls++ + if fillCalls == 1 { + return vk.Incomplete + } + *formats = vk.SurfaceFormatKHR{Format: vk.FormatR8g8b8a8Unorm, ColorSpace: vk.ColorSpaceSrgbNonlinearKhr} + *count = 1 + return vk.Success + }) + if err != nil { + t.Fatalf("querySurfaceFormatsWith() error: %v", err) + } + if len(formats) != 1 || countCalls != 2 || fillCalls != 2 { + t.Fatalf("query calls/results = (%d, %d, %d), want (2, 2, 1)", countCalls, fillCalls, len(formats)) + } +} + +func TestMakeSurfaceSnapshotDoesNotFabricateEmptyCapabilities(t *testing.T) { + _, err := makeSurfaceSnapshot(vk.SurfaceCapabilitiesKHR{}, nil, []vk.PresentModeKHR{vk.PresentModeFifoKhr}) + if err == nil { + t.Fatal("empty format query produced fabricated capabilities") + } + + _, err = makeSurfaceSnapshot(vk.SurfaceCapabilitiesKHR{}, []vk.SurfaceFormatKHR{{Format: vk.FormatR8g8b8a8Unorm}}, nil) + if err == nil { + t.Fatal("empty present-mode query produced fabricated capabilities") + } +} + +func TestMakeSurfaceSnapshotPreservesFormatColorSpacePairs(t *testing.T) { + formats := []vk.SurfaceFormatKHR{ + {Format: vk.FormatR8g8b8a8Unorm, ColorSpace: vk.ColorSpaceSrgbNonlinearKhr}, + {Format: vk.FormatR8g8b8a8Unorm, ColorSpace: vk.ColorSpaceDisplayP3NonlinearExt}, + } + snapshot, err := makeSurfaceSnapshot( + vk.SurfaceCapabilitiesKHR{SupportedCompositeAlpha: vk.CompositeAlphaFlagsKHR(vk.CompositeAlphaOpaqueBitKhr)}, + formats, + []vk.PresentModeKHR{vk.PresentModeFifoKhr}, + ) + if err != nil { + t.Fatalf("makeSurfaceSnapshot() error: %v", err) + } + if len(snapshot.formats) != len(formats) { + t.Fatalf("raw format pair count = %d, want %d", len(snapshot.formats), len(formats)) + } + for i := range formats { + if snapshot.formats[i] != formats[i] { + t.Errorf("raw format pair %d = %+v, want %+v", i, snapshot.formats[i], formats[i]) + } + } + if len(snapshot.public.Formats) != 2 { + t.Fatalf("public format projection count = %d, want 2", len(snapshot.public.Formats)) + } + if snapshot.public.Formats[0] != gputypes.TextureFormatRGBA8Unorm || snapshot.public.Formats[1] != gputypes.TextureFormatRGBA8Unorm { + t.Fatalf("public formats = %v, want two RGBA8 entries", snapshot.public.Formats) + } +} + +func TestMakeSurfaceSnapshotRejectsUnknownMappings(t *testing.T) { + _, err := makeSurfaceSnapshot( + vk.SurfaceCapabilitiesKHR{SupportedCompositeAlpha: vk.CompositeAlphaFlagsKHR(vk.CompositeAlphaOpaqueBitKhr)}, + []vk.SurfaceFormatKHR{{Format: vk.FormatR8g8b8a8Unorm, ColorSpace: vk.ColorSpaceSrgbNonlinearKhr}}, + []vk.PresentModeKHR{vk.PresentModeKHR(0x7fffffff)}, + ) + if err == nil { + t.Fatal("unknown present mode was converted into a fabricated FIFO capability") + } + + _, err = makeSurfaceSnapshot( + vk.SurfaceCapabilitiesKHR{}, + []vk.SurfaceFormatKHR{{Format: vk.FormatR8g8b8a8Unorm, ColorSpace: vk.ColorSpaceSrgbNonlinearKhr}}, + []vk.PresentModeKHR{vk.PresentModeFifoKhr}, + ) + if err == nil { + t.Fatal("unknown composite alpha flags were converted into fabricated opaque capability") + } +} diff --git a/hal/vulkan/api.go b/hal/vulkan/api.go index f995b56d..869c7b3f 100644 --- a/hal/vulkan/api.go +++ b/hal/vulkan/api.go @@ -171,17 +171,6 @@ func (i *Instance) EnumerateAdapters(surfaceHint hal.Surface) []hal.ExposedAdapt var features vk.PhysicalDeviceFeatures i.cmds.GetPhysicalDeviceFeatures(device, &features) - // Check surface support if surface hint provided - if surfaceHint != nil { - if s, ok := surfaceHint.(*Surface); ok && s.handle != 0 { - var supported vk.Bool32 - i.cmds.GetPhysicalDeviceSurfaceSupportKHR(device, 0, s.handle, &supported) - if supported == 0 { - continue // Skip devices that don't support this surface - } - } - } - // Convert device type deviceType := gputypes.DeviceTypeOther switch props.DeviceType { @@ -205,6 +194,16 @@ func (i *Instance) EnumerateAdapters(surfaceHint hal.Surface) []hal.ExposedAdapt features: features, } + adapterForExpose := hal.Adapter(adapter) + if surfaceHint != nil { + qualified, err := adapter.QualifySurface(surfaceHint) + if err != nil { + hal.Logger().Debug("vulkan: adapter rejected surface hint", "name", deviceName, "error", err) + continue + } + adapterForExpose = qualified + } + hal.Logger().Info("vulkan: adapter found", "name", deviceName, "type", deviceType, @@ -213,7 +212,7 @@ func (i *Instance) EnumerateAdapters(surfaceHint hal.Surface) []hal.ExposedAdapt ) adapters = append(adapters, hal.ExposedAdapter{ - Adapter: adapter, + Adapter: adapterForExpose, Info: gputypes.AdapterInfo{ Name: deviceName, Vendor: vendorIDToName(props.VendorID), From e38aa393199899dcb1e3739f3d122ba768399bfa Mon Sep 17 00:00:00 2001 From: Mark Date: Wed, 15 Jul 2026 19:56:08 +0300 Subject: [PATCH 2/4] fix(core): release request-local surface adapters --- adapter_native.go | 5 ++++- core/instance.go | 34 ++++++++++++++++++++++++++++++++++ core/instance_surface_test.go | 12 ++++++++++++ instance_native.go | 12 ++++++++++-- 4 files changed, 60 insertions(+), 3 deletions(-) diff --git a/adapter_native.go b/adapter_native.go index 689711bb..2b51c0aa 100644 --- a/adapter_native.go +++ b/adapter_native.go @@ -168,8 +168,11 @@ func (a *Adapter) GetSurfaceCapabilities(surface *Surface) *SurfaceCapabilities // Release releases the adapter. func (a *Adapter) Release() { - if a.released { + if a == nil || a.released { return } a.released = true + if a.instance != nil && a.instance.core != nil { + a.instance.core.ReleaseSurfaceAdapter(a.id) + } } diff --git a/core/instance.go b/core/instance.go index 33a49821..d711090f 100644 --- a/core/instance.go +++ b/core/instance.go @@ -461,6 +461,40 @@ func (i *Instance) RequestAdapterWithSurface(options *gputypes.RequestAdapterOpt return selectAdapterIDs(options, candidates) } +// ReleaseSurfaceAdapter releases a request-local adapter created by +// RequestAdapterWithSurface. Ordinary cached adapter IDs are instance-owned and +// intentionally ignored, so public Adapter.Release can use this method without +// special-casing the selected backend. +func (i *Instance) ReleaseSurfaceAdapter(id AdapterID) { + if i == nil { + return + } + i.mu.Lock() + defer i.mu.Unlock() + + index := -1 + for candidateIndex, candidateID := range i.surfaceAdapters { + if candidateID == id { + index = candidateIndex + break + } + } + if index < 0 { + return + } + + hub := GetGlobal().Hub() + if adapter, err := hub.GetAdapter(id); err == nil { + if adapter.halAdapter != nil { + adapter.halAdapter.Destroy() + } + _, _ = hub.UnregisterAdapter(id) + } + copy(i.surfaceAdapters[index:], i.surfaceAdapters[index+1:]) + i.surfaceAdapters[len(i.surfaceAdapters)-1] = AdapterID{} + i.surfaceAdapters = i.surfaceAdapters[:len(i.surfaceAdapters)-1] +} + // enumerateDeferredGLES enumerates adapters for deferred GLES HAL instances. // Called at most once per instance (guarded by glesEnumerated flag). // diff --git a/core/instance_surface_test.go b/core/instance_surface_test.go index 378564fe..299ef6c3 100644 --- a/core/instance_surface_test.go +++ b/core/instance_surface_test.go @@ -122,4 +122,16 @@ func TestRequestAdapterWithSurfaceUsesRequestLocalQualification(t *testing.T) { if cached.halAdapter != secondHAL { t.Fatal("surface request mutated cached adapter") } + + instance.ReleaseSurfaceAdapter(selectedID) + instance.ReleaseSurfaceAdapter(selectedAgain) + if len(instance.surfaceAdapters) != 0 { + t.Fatalf("surface adapter ownership after release = %d, want 0", len(instance.surfaceAdapters)) + } + if _, err := hub.GetAdapter(selectedID); err == nil { + t.Fatal("first released surface adapter remains registered") + } + if _, err := hub.GetAdapter(selectedAgain); err == nil { + t.Fatal("second released surface adapter remains registered") + } } diff --git a/instance_native.go b/instance_native.go index 28242aa1..fa89ef79 100644 --- a/instance_native.go +++ b/instance_native.go @@ -77,6 +77,12 @@ func (i *Instance) RequestAdapter(opts *RequestAdapterOptions) (*Adapter, error) if err != nil { return nil, err } + keepAdapter := false + defer func() { + if !keepAdapter { + i.core.ReleaseSurfaceAdapter(adapterID) + } + }() info, err := core.GetAdapterInfo(adapterID) if err != nil { @@ -103,14 +109,16 @@ func (i *Instance) RequestAdapter(opts *RequestAdapterOptions) (*Adapter, error) return nil, fmt.Errorf("wgpu: failed to get adapter: %w", err) } - return &Adapter{ + adapter := &Adapter{ id: adapterID, core: &coreAdapter, info: info, features: features, limits: limits, instance: i, - }, nil + } + keepAdapter = true + return adapter, nil } // Release releases the instance and all associated resources. From d4387193374b6454f7f7c6a8d79fcd5101283598 Mon Sep 17 00:00:00 2001 From: Mark Date: Wed, 15 Jul 2026 20:01:07 +0300 Subject: [PATCH 3/4] refactor(vulkan): share checked surface enumeration --- hal/vulkan/adapter.go | 117 +++++++++++++++++++----------------------- 1 file changed, 52 insertions(+), 65 deletions(-) diff --git a/hal/vulkan/adapter.go b/hal/vulkan/adapter.go index 7fa89c8b..afff50bf 100644 --- a/hal/vulkan/adapter.go +++ b/hal/vulkan/adapter.go @@ -24,14 +24,14 @@ type Adapter struct { } // Open creates a logical device with the requested features and limits. -func (a *Adapter) Open(features gputypes.Features, limits gputypes.Limits) (hal.OpenDevice, error) { - return a.open(features, limits, nil) +func (a *Adapter) Open(_ gputypes.Features, _ gputypes.Limits) (hal.OpenDevice, error) { + return a.open(nil) } // open creates a logical device, optionally constraining it to one queue // family. Surface-qualified adapters use the constrained path so the queue // selected during the surface query is the same queue passed into Open. -func (a *Adapter) open(features gputypes.Features, limits gputypes.Limits, requestedQueueFamily *uint32) (hal.OpenDevice, error) { +func (a *Adapter) open(requestedQueueFamily *uint32) (hal.OpenDevice, error) { // Find queue families var queueFamilyCount uint32 vkGetPhysicalDeviceQueueFamilyProperties(a.instance, a.physicalDevice, &queueFamilyCount, nil) @@ -43,40 +43,19 @@ func (a *Adapter) open(features gputypes.Features, limits gputypes.Limits, reque queueFamilies := make([]vk.QueueFamilyProperties, queueFamilyCount) vkGetPhysicalDeviceQueueFamilyProperties(a.instance, a.physicalDevice, &queueFamilyCount, &queueFamilies[0]) - // Find a graphics queue family. A surface-qualified adapter supplies the - // exact family selected by QualifySurface; unconstrained opens preserve the - // existing headless behavior and choose the first graphics family. - graphicsFamily := int32(-1) - if requestedQueueFamily != nil { - familyIndex := *requestedQueueFamily - if familyIndex >= queueFamilyCount || familyIndex >= uint32(len(queueFamilies)) { - return hal.OpenDevice{}, fmt.Errorf("vulkan: requested queue family %d is unavailable", familyIndex) - } - if queueFamilies[familyIndex].QueueCount == 0 { - return hal.OpenDevice{}, fmt.Errorf("vulkan: requested queue family %d has no queues", familyIndex) - } - if queueFamilies[familyIndex].QueueFlags&vk.QueueFlags(vk.QueueGraphicsBit) == 0 { - return hal.OpenDevice{}, fmt.Errorf("vulkan: requested queue family %d has no graphics queue", familyIndex) - } - graphicsFamily = int32(familyIndex) - } else { - for i, family := range queueFamilies { - if family.QueueCount > 0 && family.QueueFlags&vk.QueueFlags(vk.QueueGraphicsBit) != 0 { - graphicsFamily = int32(i) - break - } - } + if queueFamilyCount < uint32(len(queueFamilies)) { + queueFamilies = queueFamilies[:queueFamilyCount] } - - if graphicsFamily < 0 { - return hal.OpenDevice{}, fmt.Errorf("vulkan: no graphics queue family found") + graphicsFamily, err := selectGraphicsQueueFamily(queueFamilies, requestedQueueFamily) + if err != nil { + return hal.OpenDevice{}, err } // Create device with graphics queue queuePriority := float32(1.0) queueCreateInfo := vk.DeviceQueueCreateInfo{ SType: vk.StructureTypeDeviceQueueCreateInfo, - QueueFamilyIndex: uint32(graphicsFamily), + QueueFamilyIndex: graphicsFamily, QueueCount: 1, PQueuePriorities: &queuePriority, } @@ -163,13 +142,13 @@ func (a *Adapter) open(features gputypes.Features, limits gputypes.Limits, reque // Get queue handle var queue vk.Queue - vkGetDeviceQueue(&deviceCmds, device, uint32(graphicsFamily), 0, &queue) + vkGetDeviceQueue(&deviceCmds, device, graphicsFamily, 0, &queue) dev := &Device{ handle: device, physicalDevice: a.physicalDevice, instance: a.instance, - graphicsFamily: uint32(graphicsFamily), + graphicsFamily: graphicsFamily, cmds: &deviceCmds, supportsIncrementalPresent: hasIncrementalPresent, } @@ -214,7 +193,7 @@ func (a *Adapter) open(features gputypes.Features, limits gputypes.Limits, reque q := &Queue{ handle: queue, device: dev, - familyIndex: uint32(graphicsFamily), + familyIndex: graphicsFamily, relay: relay, } @@ -237,6 +216,31 @@ func (a *Adapter) open(features gputypes.Features, limits gputypes.Limits, reque }, nil } +// selectGraphicsQueueFamily preserves the exact family chosen during surface +// qualification, while keeping the ordinary headless path first-graphics. +func selectGraphicsQueueFamily(families []vk.QueueFamilyProperties, requested *uint32) (uint32, error) { + if requested != nil { + index := *requested + if index >= uint32(len(families)) { + return 0, fmt.Errorf("vulkan: requested queue family %d is unavailable", index) + } + family := families[index] + if family.QueueCount == 0 { + return 0, fmt.Errorf("vulkan: requested queue family %d has no queues", index) + } + if family.QueueFlags&vk.QueueFlags(vk.QueueGraphicsBit) == 0 { + return 0, fmt.Errorf("vulkan: requested queue family %d has no graphics queue", index) + } + return index, nil + } + for index, family := range families { + if family.QueueCount > 0 && family.QueueFlags&vk.QueueFlags(vk.QueueGraphicsBit) != 0 { + return uint32(index), nil + } + } + return 0, fmt.Errorf("vulkan: no graphics queue family found") +} + // TextureFormatCapabilities returns capabilities for a texture format. func (a *Adapter) TextureFormatCapabilities(format gputypes.TextureFormat) hal.TextureFormatCapabilities { vkFormat := textureFormatToVk(format) @@ -297,8 +301,8 @@ type qualifiedAdapter struct { snapshot surfaceSnapshot } -func (a *qualifiedAdapter) Open(features gputypes.Features, limits gputypes.Limits) (hal.OpenDevice, error) { - return a.base.open(features, limits, &a.queueFamily) +func (a *qualifiedAdapter) Open(_ gputypes.Features, _ gputypes.Limits) (hal.OpenDevice, error) { + return a.base.open(&a.queueFamily) } func (a *qualifiedAdapter) TextureFormatCapabilities(format gputypes.TextureFormat) hal.TextureFormatCapabilities { @@ -429,28 +433,32 @@ func querySurfaceFormats(instance *Instance, device vk.PhysicalDevice, surface v } func querySurfaceFormatsWith(query func(count *uint32, formats *vk.SurfaceFormatKHR) vk.Result) ([]vk.SurfaceFormatKHR, error) { + return queryOptionalSurfaceValues("vkGetPhysicalDeviceSurfaceFormatsKHR", query) +} + +func queryOptionalSurfaceValues[T any](operation string, query func(count *uint32, values *T) vk.Result) ([]T, error) { for attempt := 0; attempt < 2; attempt++ { var count uint32 result := query(&count, nil) if result != vk.Success && result != vk.Incomplete { - return nil, fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfaceFormatsKHR (count) failed: %d", result) + return nil, fmt.Errorf("vulkan: %s count failed: %d", operation, result) } if count == 0 { - return nil, nil //nolint:nilnil // an empty query is a checked incompatible result + return []T{}, nil } - formats := make([]vk.SurfaceFormatKHR, count) + values := make([]T, count) returned := count - result = query(&returned, &formats[0]) + result = query(&returned, &values[0]) if result != vk.Success && result != vk.Incomplete { - return nil, fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfaceFormatsKHR failed: %d", result) + return nil, fmt.Errorf("vulkan: %s failed: %d", operation, result) } - if result == vk.Incomplete || returned > uint32(len(formats)) { + if result == vk.Incomplete || returned > uint32(len(values)) { continue } - return formats[:returned], nil + return values[:returned], nil } - return nil, fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfaceFormatsKHR returned an unstable count") + return nil, fmt.Errorf("vulkan: %s returned an unstable count", operation) } func queryPresentModes(instance *Instance, device vk.PhysicalDevice, surface vk.SurfaceKHR) ([]vk.PresentModeKHR, error) { @@ -460,28 +468,7 @@ func queryPresentModes(instance *Instance, device vk.PhysicalDevice, surface vk. } func queryPresentModesWith(query func(count *uint32, modes *vk.PresentModeKHR) vk.Result) ([]vk.PresentModeKHR, error) { - for attempt := 0; attempt < 2; attempt++ { - var count uint32 - result := query(&count, nil) - if result != vk.Success && result != vk.Incomplete { - return nil, fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfacePresentModesKHR (count) failed: %d", result) - } - if count == 0 { - return nil, nil //nolint:nilnil // an empty query is a checked incompatible result - } - - modes := make([]vk.PresentModeKHR, count) - returned := count - result = query(&returned, &modes[0]) - if result != vk.Success && result != vk.Incomplete { - return nil, fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfacePresentModesKHR failed: %d", result) - } - if result == vk.Incomplete || returned > uint32(len(modes)) { - continue - } - return modes[:returned], nil - } - return nil, fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfacePresentModesKHR returned an unstable count") + return queryOptionalSurfaceValues("vkGetPhysicalDeviceSurfacePresentModesKHR", query) } func makeSurfaceSnapshot(capabilities vk.SurfaceCapabilitiesKHR, formats []vk.SurfaceFormatKHR, presentModes []vk.PresentModeKHR) (surfaceSnapshot, error) { From a3e839f94a12edce98e2496d96e5bd8d3cdd2fc3 Mon Sep 17 00:00:00 2001 From: Mark Date: Wed, 15 Jul 2026 20:25:36 +0300 Subject: [PATCH 4/4] fix(core): bound surface adapter request lifetimes --- core/instance.go | 20 +++++++- core/instance_surface_test.go | 89 ++++++++++++++++++++++++++++++++++- 2 files changed, 106 insertions(+), 3 deletions(-) diff --git a/core/instance.go b/core/instance.go index d711090f..1bd7e050 100644 --- a/core/instance.go +++ b/core/instance.go @@ -21,6 +21,11 @@ import ( // Thread-safe for concurrent use. type Instance struct { mu sync.RWMutex + // surfaceRequestMu serializes surface-aware adapter requests. Deferred GLES + // enumeration is intentionally one-shot, so the snapshot that distinguishes + // adapters created for the current surface must be taken atomically with that + // enumeration. + surfaceRequestMu sync.Mutex backends gputypes.Backends flags gputypes.InstanceFlags @@ -379,6 +384,8 @@ func (i *Instance) RequestAdapterWithSurface(options *gputypes.RequestAdapterOpt if surfaceHint == nil { return i.RequestAdapter(options) } + i.surfaceRequestMu.Lock() + defer i.surfaceRequestMu.Unlock() // Remember which adapters existed before deferred enumeration. Adapters // created by EnumerateAdapters(surfaceHint) are already surface-qualified by @@ -404,6 +411,7 @@ func (i *Instance) RequestAdapterWithSurface(options *gputypes.RequestAdapterOpt usingMock := i.useMock i.mu.RUnlock() candidates := make([]AdapterID, 0, len(allAdapterIDs)) + qualifiedIDs := make([]AdapterID, 0, len(allAdapterIDs)) for _, adapterID := range allAdapterIDs { adapter, err := hub.GetAdapter(adapterID) if err != nil { @@ -444,6 +452,7 @@ func (i *Instance) RequestAdapterWithSurface(options *gputypes.RequestAdapterOpt i.surfaceAdapters = append(i.surfaceAdapters, qualifiedID) i.mu.Unlock() candidates = append(candidates, qualifiedID) + qualifiedIDs = append(qualifiedIDs, qualifiedID) continue } @@ -458,7 +467,13 @@ func (i *Instance) RequestAdapterWithSurface(options *gputypes.RequestAdapterOpt if len(candidates) == 0 { return AdapterID{}, fmt.Errorf("no adapters compatible with surface") } - return selectAdapterIDs(options, candidates) + selectedID, err := selectAdapterIDs(options, candidates) + for _, qualifiedID := range qualifiedIDs { + if err != nil || qualifiedID != selectedID { + i.ReleaseSurfaceAdapter(qualifiedID) + } + } + return selectedID, err } // ReleaseSurfaceAdapter releases a request-local adapter created by @@ -654,6 +669,9 @@ func (i *Instance) HALInstanceMap() map[gputypes.Backend]hal.Instance { // This includes unregistering all adapters and destroying HAL instances. // After calling Destroy, the instance should not be used. func (i *Instance) Destroy() { + i.surfaceRequestMu.Lock() + defer i.surfaceRequestMu.Unlock() + i.mu.Lock() defer i.mu.Unlock() diff --git a/core/instance_surface_test.go b/core/instance_surface_test.go index 299ef6c3..fff6c6b5 100644 --- a/core/instance_surface_test.go +++ b/core/instance_surface_test.go @@ -13,6 +13,7 @@ import ( type surfaceQualificationAdapter struct { compatible bool qualifies *int + destroys *int } func (a *surfaceQualificationAdapter) Open(_ gputypes.Features, _ gputypes.Limits) (hal.OpenDevice, error) { @@ -27,7 +28,11 @@ func (a *surfaceQualificationAdapter) SurfaceCapabilities(_ hal.Surface) *hal.Su return nil } -func (a *surfaceQualificationAdapter) Destroy() {} +func (a *surfaceQualificationAdapter) Destroy() { + if a.destroys != nil { + (*a.destroys)++ + } +} func (a *surfaceQualificationAdapter) QualifySurface(_ hal.Surface) (hal.Adapter, error) { if a.qualifies != nil { @@ -36,7 +41,87 @@ func (a *surfaceQualificationAdapter) QualifySurface(_ hal.Surface) (hal.Adapter if !a.compatible { return nil, errors.New("surface is not supported") } - return &surfaceQualificationAdapter{compatible: true}, nil + return &surfaceQualificationAdapter{compatible: true, destroys: a.destroys}, nil +} + +func TestRequestAdapterWithSurfaceReleasesUnselectedQualifiedAdapters(t *testing.T) { + GetGlobal().Clear() + hub := GetGlobal().Hub() + + destroys := 0 + firstHAL := &surfaceQualificationAdapter{compatible: true, destroys: &destroys} + secondHAL := &surfaceQualificationAdapter{compatible: true, destroys: &destroys} + firstID := hub.RegisterAdapter(&Adapter{ + Info: gputypes.AdapterInfo{DeviceType: gputypes.DeviceTypeDiscreteGPU, Backend: gputypes.BackendVulkan}, + Limits: gputypes.DefaultLimits(), + Backend: gputypes.BackendVulkan, + halAdapter: firstHAL, + }) + secondID := hub.RegisterAdapter(&Adapter{ + Info: gputypes.AdapterInfo{DeviceType: gputypes.DeviceTypeIntegratedGPU, Backend: gputypes.BackendVulkan}, + Limits: gputypes.DefaultLimits(), + Backend: gputypes.BackendVulkan, + halAdapter: secondHAL, + }) + + instance := &Instance{ + backends: gputypes.BackendsVulkan, + adapters: []AdapterID{firstID, secondID}, + } + selectedID, err := instance.RequestAdapterWithSurface(nil, &stubHALSurface{id: 9}) + if err != nil { + t.Fatalf("RequestAdapterWithSurface() error: %v", err) + } + if len(instance.surfaceAdapters) != 1 { + t.Fatalf("surface adapter ownership = %d, want 1", len(instance.surfaceAdapters)) + } + if selectedID != instance.surfaceAdapters[0] { + t.Fatalf("selected ID = %v, tracked surface adapter = %v", selectedID, instance.surfaceAdapters[0]) + } + if destroys != 1 { + t.Fatalf("qualified adapter destroys after selection = %d, want 1", destroys) + } + + instance.ReleaseSurfaceAdapter(selectedID) + if destroys != 2 { + t.Fatalf("qualified adapter destroys after selected release = %d, want 2", destroys) + } + instance.Destroy() +} + +func TestRequestAdapterWithSurfaceReleasesQualifiedAdaptersOnSelectionError(t *testing.T) { + GetGlobal().Clear() + hub := GetGlobal().Hub() + + destroys := 0 + firstID := hub.RegisterAdapter(&Adapter{ + Info: gputypes.AdapterInfo{DeviceType: gputypes.DeviceTypeDiscreteGPU, Backend: gputypes.BackendVulkan}, + Limits: gputypes.DefaultLimits(), + Backend: gputypes.BackendVulkan, + halAdapter: &surfaceQualificationAdapter{compatible: true, destroys: &destroys}, + }) + secondID := hub.RegisterAdapter(&Adapter{ + Info: gputypes.AdapterInfo{DeviceType: gputypes.DeviceTypeIntegratedGPU, Backend: gputypes.BackendVulkan}, + Limits: gputypes.DefaultLimits(), + Backend: gputypes.BackendVulkan, + halAdapter: &surfaceQualificationAdapter{compatible: true, destroys: &destroys}, + }) + + instance := &Instance{ + backends: gputypes.BackendsVulkan, + adapters: []AdapterID{firstID, secondID}, + } + _, err := instance.RequestAdapterWithSurface(&gputypes.RequestAdapterOptions{ForceFallbackAdapter: true}, &stubHALSurface{id: 10}) + if err == nil { + t.Fatal("RequestAdapterWithSurface() unexpectedly selected a non-fallback adapter") + } + if len(instance.surfaceAdapters) != 0 { + t.Fatalf("surface adapter ownership after selection error = %d, want 0", len(instance.surfaceAdapters)) + } + if destroys != 2 { + t.Fatalf("qualified adapter destroys after selection error = %d, want 2", destroys) + } + instance.Destroy() } func TestRequestAdapterWithSurfaceUsesRequestLocalQualification(t *testing.T) {