diff --git a/hal/vulkan/api.go b/hal/vulkan/api.go index f995b56..5ba690d 100644 --- a/hal/vulkan/api.go +++ b/hal/vulkan/api.go @@ -272,6 +272,12 @@ type Surface struct { // This commonly happens when the window is minimized or not yet fully visible. // Wait until the window has valid dimensions before calling Configure again. func (s *Surface) Configure(device hal.Device, config *hal.SurfaceConfiguration) error { + if s == nil { + return fmt.Errorf("vulkan: surface is nil") + } + if config == nil { + return fmt.Errorf("vulkan: surface configuration is nil") + } // Validate dimensions first (before any side effects). // This matches wgpu-core behavior which returns ConfigureSurfaceError::ZeroArea. if config.Width == 0 || config.Height == 0 { @@ -303,8 +309,23 @@ func (s *Surface) ActualExtent() (width, height uint32) { // Unconfigure removes surface configuration. func (s *Surface) Unconfigure(_ hal.Device) { + if s == nil { + return + } if s.swapchain != nil { - s.swapchain.Destroy() + swapchain := s.swapchain + if err := swapchain.destroyWithError(); err != nil { + hal.Logger().Error("vulkan: failed to destroy swapchain during unconfigure", "error", err) + return + } + if s.device != nil && s.device.queue != nil { + s.device.queue.mu.Lock() + if s.device.queue.activeSwapchain == swapchain { + s.device.queue.activeSwapchain = nil + s.device.queue.acquireUsed = false + } + s.device.queue.mu.Unlock() + } s.swapchain = nil } s.device = nil @@ -313,6 +334,9 @@ func (s *Surface) Unconfigure(_ hal.Device) { // AcquireTexture acquires the next surface texture for rendering. // Returns hal.ErrNotReady if no image is available (non-blocking mode). func (s *Surface) AcquireTexture(_ hal.Fence) (*hal.AcquiredSurfaceTexture, error) { + if s == nil { + return nil, fmt.Errorf("vulkan: surface is nil") + } if s.swapchain == nil { return nil, fmt.Errorf("vulkan: surface not configured") } @@ -331,8 +355,10 @@ func (s *Surface) AcquireTexture(_ hal.Fence) (*hal.AcquiredSurfaceTexture, erro // This ensures the queue waits for image acquisition before rendering // and signals completion before present. if s.device != nil && s.device.queue != nil { + s.device.queue.mu.Lock() s.device.queue.activeSwapchain = s.swapchain s.device.queue.acquireUsed = false // Reset for new frame + s.device.queue.mu.Unlock() } return &hal.AcquiredSurfaceTexture{ @@ -343,15 +369,41 @@ func (s *Surface) AcquireTexture(_ hal.Fence) (*hal.AcquiredSurfaceTexture, erro // DiscardTexture discards a surface texture without presenting it. func (s *Surface) DiscardTexture(_ hal.SurfaceTexture) { - if s.swapchain != nil { - s.swapchain.imageAcquired = false + if s == nil { + return + } + if s.swapchain != nil && s.swapchain.imageAcquired { + s.swapchain.markBroken(fmt.Errorf("vulkan: acquired surface texture was discarded without presentation")) + if s.device != nil && s.device.queue != nil { + s.device.queue.mu.Lock() + if s.device.queue.activeSwapchain == s.swapchain { + s.device.queue.activeSwapchain = nil + s.device.queue.acquireUsed = false + } + s.device.queue.mu.Unlock() + } } } // Destroy releases the surface. func (s *Surface) Destroy() { + if s == nil { + return + } if s.swapchain != nil { - s.swapchain.Destroy() + swapchain := s.swapchain + if err := swapchain.destroyWithError(); err != nil { + hal.Logger().Error("vulkan: failed to destroy swapchain", "error", err) + return + } + if s.device != nil && s.device.queue != nil { + s.device.queue.mu.Lock() + if s.device.queue.activeSwapchain == swapchain { + s.device.queue.activeSwapchain = nil + s.device.queue.acquireUsed = false + } + s.device.queue.mu.Unlock() + } s.swapchain = nil } if s.handle != 0 && s.instance != nil { diff --git a/hal/vulkan/queue.go b/hal/vulkan/queue.go index 7257afb..e3d7dd0 100644 --- a/hal/vulkan/queue.go +++ b/hal/vulkan/queue.go @@ -138,6 +138,34 @@ type Queue struct { savedAcquireUsed bool } +func validateSwapchainSubmission(swapchain *Swapchain, device *Device) error { + if swapchain == nil { + return nil + } + if swapchain.device != device { + return fmt.Errorf("vulkan: active swapchain belongs to a different device") + } + if swapchain.destroyed { + return fmt.Errorf("vulkan: active swapchain has been destroyed") + } + if swapchain.broken { + if swapchain.failureErr != nil { + return fmt.Errorf("vulkan: active swapchain is broken: %w", swapchain.failureErr) + } + return fmt.Errorf("vulkan: active swapchain is broken") + } + if !swapchain.imageAcquired { + return fmt.Errorf("vulkan: no swapchain image acquired for submission") + } + if swapchain.currentAcquireIdx < 0 || swapchain.currentAcquireIdx >= len(swapchain.acquireSemaphores) || swapchain.currentAcquireIdx >= len(swapchain.acquireFenceValues) { + return fmt.Errorf("vulkan: swapchain acquire semaphore state is inconsistent") + } + if swapchain.currentImage >= uint32(len(swapchain.presentSemaphores)) { + return fmt.Errorf("vulkan: swapchain present image index is out of range") + } + return nil +} + // Submit submits command buffers to the GPU. // Returns a monotonically increasing submission index for tracking completion. // The HAL internally manages fence/timeline semaphore synchronization. @@ -151,6 +179,9 @@ func (q *Queue) Submit(commandBuffers []hal.CommandBuffer) (uint64, error) { if len(commandBuffers) == 0 { return 0, nil } + if err := validateSwapchainSubmission(q.activeSwapchain, q.device); err != nil { + return 0, err + } // Convert command buffers to Vulkan handles. // Use sync.Pool to avoid per-frame heap allocation (VK-PERF-001). @@ -189,11 +220,22 @@ func (q *Queue) Submit(commandBuffers []hal.CommandBuffer) (uint64, error) { // Subsequent submits in the same frame run without semaphore synchronization. consumedAcquire := false if q.activeSwapchain != nil && !q.acquireUsed { + if q.activeSwapchain.currentAcquireSem == 0 || q.activeSwapchain.currentImage >= uint32(len(q.activeSwapchain.presentSemaphores)) { + err := fmt.Errorf("vulkan: active swapchain semaphore state is invalid") + q.activeSwapchain.markBroken(err) + return 0, err + } + presentSemaphore := q.activeSwapchain.presentSemaphores[q.activeSwapchain.currentImage] + if presentSemaphore == 0 { + err := fmt.Errorf("vulkan: active swapchain present semaphore is destroyed") + q.activeSwapchain.markBroken(err) + return 0, err + } waitSems[waitCount] = q.activeSwapchain.currentAcquireSem waitStages[waitCount] = vk.PipelineStageFlags(vk.PipelineStageColorAttachmentOutputBit) waitCount++ - signalSems[signalCount] = q.activeSwapchain.presentSemaphores[q.activeSwapchain.currentImage] + signalSems[signalCount] = presentSemaphore signalCount++ q.acquireUsed = true @@ -206,6 +248,9 @@ func (q *Queue) Submit(commandBuffers []hal.CommandBuffer) (uint64, error) { if q.relay != nil { relayWait, relaySignal, err := q.relay.advance(q.device.cmds, q.device.handle) if err != nil { + if q.activeSwapchain != nil { + q.activeSwapchain.markBroken(fmt.Errorf("vulkan: relay semaphore advance: %w", err)) + } return 0, fmt.Errorf("vulkan: relay semaphore advance: %w", err) } if relayWait != 0 { @@ -272,6 +317,9 @@ func (q *Queue) Submit(commandBuffers []hal.CommandBuffer) (uint64, error) { result := vkQueueSubmit(q, 1, &submitInfo, vk.Fence(0)) if result != vk.Success { + if consumedAcquire { + q.activeSwapchain.markBroken(fmt.Errorf("vulkan: vkQueueSubmit failed: %d", result)) + } return 0, fmt.Errorf("vulkan: vkQueueSubmit failed: %d", result) } return signalValue, nil @@ -289,12 +337,18 @@ func (q *Queue) Submit(commandBuffers []hal.CommandBuffer) (uint64, error) { } poolFence, err := pool.signal(q.device.cmds, q.device.handle, signalValue) if err != nil { + if consumedAcquire { + q.activeSwapchain.markBroken(fmt.Errorf("vulkan: Submit fencePool signal: %w", err)) + } return 0, fmt.Errorf("vulkan: Submit fencePool signal: %w", err) } // Single vkQueueSubmit with pool fence — no more double submit. result := vkQueueSubmit(q, 1, &submitInfo, poolFence) if result != vk.Success { + if consumedAcquire { + q.activeSwapchain.markBroken(fmt.Errorf("vulkan: vkQueueSubmit failed: %d", result)) + } return 0, fmt.Errorf("vulkan: vkQueueSubmit failed: %d", result) } return signalValue, nil @@ -337,6 +391,30 @@ func (q *Queue) SubmitForPresent(commandBuffers []hal.CommandBuffer, swapchain * if len(commandBuffers) == 0 { return nil } + if swapchain == nil { + return fmt.Errorf("vulkan: swapchain is nil") + } + if err := validateSwapchainSubmission(swapchain, q.device); err != nil { + return err + } + if q.activeSwapchain != nil && q.activeSwapchain != swapchain { + return fmt.Errorf("vulkan: queue has a different active swapchain") + } + if q.activeSwapchain == swapchain && q.acquireUsed { + swapchain.markBroken(fmt.Errorf("vulkan: swapchain acquire semaphore was already consumed")) + return fmt.Errorf("vulkan: swapchain acquire semaphore was already consumed") + } + if swapchain.currentAcquireSem == 0 || swapchain.currentImage >= uint32(len(swapchain.presentSemaphores)) { + err := fmt.Errorf("vulkan: swapchain semaphore state is invalid") + swapchain.markBroken(err) + return err + } + presentSemaphore := swapchain.presentSemaphores[swapchain.currentImage] + if presentSemaphore == 0 { + err := fmt.Errorf("vulkan: swapchain present semaphore is destroyed") + swapchain.markBroken(err) + return err + } // Convert command buffers to Vulkan handles. // Use sync.Pool to avoid per-frame heap allocation (VK-PERF-001). @@ -374,13 +452,14 @@ func (q *Queue) SubmitForPresent(commandBuffers []hal.CommandBuffer, swapchain * waitCount++ // Present semaphore: always present for SubmitForPresent. - signalSems[signalCount] = swapchain.presentSemaphores[swapchain.currentImage] + signalSems[signalCount] = presentSemaphore signalCount++ // VK-SYNC-001: Add relay semaphores for GPU-side submission ordering. if q.relay != nil { relayWait, relaySignal, err := q.relay.advance(q.device.cmds, q.device.handle) if err != nil { + swapchain.markBroken(fmt.Errorf("vulkan: relay semaphore advance: %w", err)) return fmt.Errorf("vulkan: relay semaphore advance: %w", err) } if relayWait != 0 { @@ -433,8 +512,11 @@ func (q *Queue) SubmitForPresent(commandBuffers []hal.CommandBuffer, swapchain * result := vkQueueSubmit(q, 1, &submitInfo, vk.Fence(0)) if result != vk.Success { + swapchain.markBroken(fmt.Errorf("vulkan: vkQueueSubmit failed: %d", result)) return fmt.Errorf("vulkan: vkQueueSubmit failed: %d", result) } + q.activeSwapchain = swapchain + q.acquireUsed = true return nil } @@ -448,14 +530,18 @@ func (q *Queue) SubmitForPresent(commandBuffers []hal.CommandBuffer, swapchain * poolFence, err := pool.signal(q.device.cmds, q.device.handle, signalValue) if err != nil { + swapchain.markBroken(fmt.Errorf("vulkan: SubmitForPresent fencePool signal: %w", err)) return fmt.Errorf("vulkan: SubmitForPresent fencePool signal: %w", err) } result := vkQueueSubmit(q, 1, &submitInfo, poolFence) if result != vk.Success { + swapchain.markBroken(fmt.Errorf("vulkan: vkQueueSubmit failed: %d", result)) return fmt.Errorf("vulkan: vkQueueSubmit failed: %d", result) } + q.activeSwapchain = swapchain + q.acquireUsed = true return nil } @@ -699,8 +785,12 @@ func (q *Queue) Present(surface hal.Surface, _ hal.SurfaceTexture, damageRects [ return fmt.Errorf("vulkan: surface not configured") } - err := vkSurface.swapchain.present(q, damageRects) - q.activeSwapchain = nil + swapchain := vkSurface.swapchain + err := swapchain.present(q, damageRects) + if q.activeSwapchain == swapchain { + q.activeSwapchain = nil + q.acquireUsed = false + } return err } diff --git a/hal/vulkan/swapchain.go b/hal/vulkan/swapchain.go index 7568ad4..c5fc12f 100644 --- a/hal/vulkan/swapchain.go +++ b/hal/vulkan/swapchain.go @@ -6,6 +6,7 @@ package vulkan import ( + "errors" "fmt" "image" "unsafe" @@ -60,6 +61,13 @@ type Swapchain struct { // pending (VUID-vkResetCommandPool-commandPool-00040). Created lazily // alongside barrierPool. barrierFence vk.Fence + + // broken is set after a synchronization, layout, or presentation failure. + // A broken swapchain cannot safely reuse its binary semaphores; callers must + // reconfigure or destroy it before attempting another frame. + broken bool + failureErr error + destroyed bool } // SwapchainTexture wraps a swapchain image as a SurfaceTexture. @@ -87,29 +95,333 @@ func (t *SwapchainTexture) NativeHandle() uintptr { return uintptr(t.handle) } +func (sc *Swapchain) markBroken(err error) { + if err == nil { + err = fmt.Errorf("vulkan: swapchain synchronization failed") + } + sc.broken = true + sc.failureErr = err + sc.imageAcquired = false +} + +func (sc *Swapchain) stateError(operation string) error { + if sc == nil || sc.destroyed { + return fmt.Errorf("vulkan: cannot %s a destroyed swapchain", operation) + } + if sc.broken { + if sc.failureErr != nil { + return fmt.Errorf("vulkan: cannot %s a broken swapchain: %w", operation, sc.failureErr) + } + return fmt.Errorf("vulkan: cannot %s a broken swapchain", operation) + } + return nil +} + +type swapchainSurfaceSnapshot struct { + capabilities vk.SurfaceCapabilitiesKHR + formats []vk.SurfaceFormatKHR + presentModes []vk.PresentModeKHR +} + +func (snapshot swapchainSurfaceSnapshot) formatFor(requested gputypes.TextureFormat) (vk.SurfaceFormatKHR, error) { + requestedVk := textureFormatToVk(requested) + if requestedVk == vk.FormatUndefined { + return vk.SurfaceFormatKHR{}, fmt.Errorf("vulkan: unsupported surface format %v", requested) + } + + var fallback *vk.SurfaceFormatKHR + for i := range snapshot.formats { + format := snapshot.formats[i] + if format.Format != requestedVk { + continue + } + if format.ColorSpace == vk.ColorSpaceSrgbNonlinearKhr { + return format, nil + } + if fallback == nil { + fallback = &format + } + } + if fallback == nil { + return vk.SurfaceFormatKHR{}, fmt.Errorf("vulkan: surface does not support format %v", requested) + } + return *fallback, nil +} + +func (snapshot swapchainSurfaceSnapshot) presentModeFor(requested hal.PresentMode) (vk.PresentModeKHR, error) { + requestedVk, ok := presentModeToVkChecked(requested) + if !ok { + return 0, fmt.Errorf("vulkan: unsupported present mode %v", requested) + } + for _, mode := range snapshot.presentModes { + if mode == requestedVk { + return mode, nil + } + } + return 0, fmt.Errorf("vulkan: surface does not support present mode %v", requested) +} + +func compositeAlphaFor(flags vk.CompositeAlphaFlagsKHR, requested hal.CompositeAlphaMode) (vk.CompositeAlphaFlagBitsKHR, error) { + available := func(flag vk.CompositeAlphaFlagBitsKHR) bool { + return vk.Flags(flags)&vk.Flags(flag) != 0 + } + + if requested == hal.CompositeAlphaModeAuto { + // Prefer opaque only when the surface reports it; otherwise use the + // first Vulkan-supported mode in a stable order. + for _, mode := range []vk.CompositeAlphaFlagBitsKHR{ + vk.CompositeAlphaOpaqueBitKhr, + vk.CompositeAlphaPreMultipliedBitKhr, + vk.CompositeAlphaPostMultipliedBitKhr, + vk.CompositeAlphaInheritBitKhr, + } { + if available(mode) { + return mode, nil + } + } + return 0, fmt.Errorf("vulkan: surface reports no composite alpha mode") + } + + requestedFlag, ok := mapCompositeAlphaToVk(requested) + if !ok || !available(requestedFlag) { + return 0, fmt.Errorf("vulkan: surface does not support composite alpha mode %v", requested) + } + return requestedFlag, nil +} + +func mapCompositeAlphaToVk(mode hal.CompositeAlphaMode) (vk.CompositeAlphaFlagBitsKHR, bool) { + switch mode { + case hal.CompositeAlphaModeOpaque: + return vk.CompositeAlphaOpaqueBitKhr, true + case hal.CompositeAlphaModePremultiplied: + return vk.CompositeAlphaPreMultipliedBitKhr, true + case hal.CompositeAlphaModeUnpremultiplied: + return vk.CompositeAlphaPostMultipliedBitKhr, true + case hal.CompositeAlphaModeInherit: + return vk.CompositeAlphaInheritBitKhr, true + default: + return 0, false + } +} + +func presentModeToVkChecked(mode hal.PresentMode) (vk.PresentModeKHR, bool) { + switch mode { + case hal.PresentModeImmediate: + return vk.PresentModeImmediateKhr, true + case hal.PresentModeMailbox: + return vk.PresentModeMailboxKhr, true + case hal.PresentModeFifo: + return vk.PresentModeFifoKhr, true + case hal.PresentModeFifoRelaxed: + return vk.PresentModeFifoRelaxedKhr, true + default: + return 0, false + } +} + +func swapchainImageUsage(usage gputypes.TextureUsage, supported vk.ImageUsageFlags) (vk.ImageUsageFlags, error) { + if usage.ContainsUnknownBits() { + return 0, fmt.Errorf("vulkan: surface configuration has unknown texture usage bits") + } + // Surface textures are renderable by contract, even when callers omit the + // bit in a legacy descriptor. Every requested usage is still checked against + // the driver-reported flags before creating the swapchain. + effective := usage | gputypes.TextureUsageRenderAttachment + requested := textureUsageToVk(effective) + if vk.Flags(supported)&vk.Flags(requested) != vk.Flags(requested) { + return 0, fmt.Errorf("vulkan: surface does not support requested texture usage %v", effective) + } + return requested, nil +} + +func validateSurfaceSnapshot(snapshot swapchainSurfaceSnapshot, config *hal.SurfaceConfiguration) (vk.SurfaceFormatKHR, vk.PresentModeKHR, vk.CompositeAlphaFlagBitsKHR, vk.ImageUsageFlags, error) { + if config == nil { + return vk.SurfaceFormatKHR{}, 0, 0, 0, fmt.Errorf("vulkan: surface configuration is nil") + } + if len(snapshot.formats) == 0 { + return vk.SurfaceFormatKHR{}, 0, 0, 0, fmt.Errorf("vulkan: surface returned no formats") + } + if len(snapshot.presentModes) == 0 { + return vk.SurfaceFormatKHR{}, 0, 0, 0, fmt.Errorf("vulkan: surface returned no present modes") + } + format, err := snapshot.formatFor(config.Format) + if err != nil { + return vk.SurfaceFormatKHR{}, 0, 0, 0, err + } + presentMode, err := snapshot.presentModeFor(config.PresentMode) + if err != nil { + return vk.SurfaceFormatKHR{}, 0, 0, 0, err + } + alphaMode, err := compositeAlphaFor(snapshot.capabilities.SupportedCompositeAlpha, config.AlphaMode) + if err != nil { + return vk.SurfaceFormatKHR{}, 0, 0, 0, err + } + usage, err := swapchainImageUsage(config.Usage, snapshot.capabilities.SupportedUsageFlags) + if err != nil { + return vk.SurfaceFormatKHR{}, 0, 0, 0, err + } + if snapshot.capabilities.CurrentTransform == 0 { + return vk.SurfaceFormatKHR{}, 0, 0, 0, fmt.Errorf("vulkan: surface returned no current transform") + } + if vk.Flags(snapshot.capabilities.SupportedTransforms)&vk.Flags(snapshot.capabilities.CurrentTransform) == 0 { + return vk.SurfaceFormatKHR{}, 0, 0, 0, fmt.Errorf("vulkan: surface current transform is not supported") + } + return format, presentMode, alphaMode, usage, nil +} + +func querySwapchainSurfaceSnapshot(instance *Instance, device vk.PhysicalDevice, surface vk.SurfaceKHR) (swapchainSurfaceSnapshot, error) { + var capabilities vk.SurfaceCapabilitiesKHR + result := vkGetPhysicalDeviceSurfaceCapabilitiesKHR(instance, device, surface, &capabilities) + if result != vk.Success { + return swapchainSurfaceSnapshot{}, surfaceQueryError("vkGetPhysicalDeviceSurfaceCapabilitiesKHR", result) + } + + formats, err := querySwapchainFormats(instance, device, surface) + if err != nil { + return swapchainSurfaceSnapshot{}, err + } + presentModes, err := querySwapchainPresentModes(instance, device, surface) + if err != nil { + return swapchainSurfaceSnapshot{}, err + } + return swapchainSurfaceSnapshot{ + capabilities: capabilities, + formats: formats, + presentModes: presentModes, + }, nil +} + +func surfaceQueryError(operation string, result vk.Result) error { + switch result { + case vk.ErrorSurfaceLostKhr: + return fmt.Errorf("vulkan: %s failed: %w", operation, hal.ErrSurfaceLost) + case vk.ErrorOutOfDateKhr: + return fmt.Errorf("vulkan: %s failed: %w", operation, hal.ErrSurfaceOutdated) + default: + return fmt.Errorf("vulkan: %s failed: %d", operation, result) + } +} + +func querySwapchainFormats(instance *Instance, device vk.PhysicalDevice, surface vk.SurfaceKHR) ([]vk.SurfaceFormatKHR, error) { + return querySwapchainFormatsWith(func(count *uint32, formats *vk.SurfaceFormatKHR) vk.Result { + return instance.cmds.GetPhysicalDeviceSurfaceFormatsKHR(device, surface, count, formats) + }) +} + +func querySwapchainFormatsWith(query func(count *uint32, formats *vk.SurfaceFormatKHR) vk.Result) ([]vk.SurfaceFormatKHR, error) { + return queryRequiredSwapchainValues( + "vkGetPhysicalDeviceSurfaceFormatsKHR", + "vkGetPhysicalDeviceSurfaceFormatsKHR", + "vulkan: surface returned no formats", + query, + ) +} + +func queryRequiredSwapchainValues[T any](countOperation, valuesOperation, emptyMessage 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, surfaceQueryError(countOperation+" (count)", result) + } + if count == 0 { + return nil, errors.New(emptyMessage) + } + values := make([]T, count) + returned := count + result = query(&returned, &values[0]) + if result != vk.Success && result != vk.Incomplete { + return nil, surfaceQueryError(valuesOperation, result) + } + if result == vk.Incomplete || returned > uint32(len(values)) { + continue + } + if returned == 0 { + return nil, errors.New(emptyMessage) + } + return values[:returned], nil + } + return nil, fmt.Errorf("vulkan: %s returned an unstable count", valuesOperation) +} + +func querySwapchainPresentModes(instance *Instance, device vk.PhysicalDevice, surface vk.SurfaceKHR) ([]vk.PresentModeKHR, error) { + return querySwapchainPresentModesWith(func(count *uint32, modes *vk.PresentModeKHR) vk.Result { + return instance.cmds.GetPhysicalDeviceSurfacePresentModesKHR(device, surface, count, modes) + }) +} + +func querySwapchainPresentModesWith(query func(count *uint32, modes *vk.PresentModeKHR) vk.Result) ([]vk.PresentModeKHR, error) { + return queryRequiredSwapchainValues( + "vkGetPhysicalDeviceSurfacePresentModesKHR", + "vkGetPhysicalDeviceSurfacePresentModesKHR", + "vulkan: surface returned no present modes", + query, + ) +} + +func querySwapchainImages(device *Device, swapchain vk.SwapchainKHR) ([]vk.Image, error) { + return querySwapchainImagesWith(func(count *uint32, images *vk.Image) vk.Result { + return vkGetSwapchainImagesKHR(device, swapchain, count, images) + }) +} + +func querySwapchainImagesWith(query func(count *uint32, images *vk.Image) vk.Result) ([]vk.Image, error) { + return queryRequiredSwapchainValues( + "vkGetSwapchainImagesKHR", + "vkGetSwapchainImagesKHR (images)", + "vulkan: swapchain returned no images", + query, + ) +} + // createSwapchain creates a new swapchain for the surface. // //nolint:maintidx // Vulkan swapchain setup requires many sequential steps func (s *Surface) createSwapchain(device *Device, config *hal.SurfaceConfiguration) error { - if s.handle == 0 { + if s == nil || s.handle == 0 { return fmt.Errorf("vulkan: cannot create swapchain for null surface") } + if config == nil { + return fmt.Errorf("vulkan: cannot create swapchain with nil configuration") + } - // Get surface capabilities - var capabilities vk.SurfaceCapabilitiesKHR - result := vkGetPhysicalDeviceSurfaceCapabilitiesKHR(s.instance, device.physicalDevice, s.handle, &capabilities) - if result != vk.Success { - if result == vk.ErrorSurfaceLostKhr { - return hal.ErrSurfaceLost - } - return fmt.Errorf("vulkan: vkGetPhysicalDeviceSurfaceCapabilitiesKHR failed: %d", result) + if s.instance == nil || device == nil || device.instance == nil || device.instance != s.instance { + return fmt.Errorf("vulkan: device does not belong to surface instance") + } + + // Query every capability used by VkSwapchainCreateInfoKHR and validate the + // requested configuration before changing an existing swapchain. + snapshot, err := querySwapchainSurfaceSnapshot(s.instance, device.physicalDevice, s.handle) + if err != nil { + return err + } + selectedFormat, presentMode, compositeAlpha, imageUsage, err := validateSurfaceSnapshot(snapshot, config) + if err != nil { + return err } + capabilities := snapshot.capabilities // Determine image count - imageCount := capabilities.MinImageCount + 1 + if capabilities.MinImageCount == 0 { + return fmt.Errorf("vulkan: surface returned invalid minimum image count") + } + if capabilities.MaxImageArrayLayers == 0 { + return fmt.Errorf("vulkan: surface does not support one image array layer") + } + if capabilities.CurrentExtent.Width != 0xFFFFFFFF && + (capabilities.MinImageExtent.Width > capabilities.MaxImageExtent.Width || capabilities.MinImageExtent.Height > capabilities.MaxImageExtent.Height) { + return fmt.Errorf("vulkan: surface returned invalid image extent range") + } + imageCount := capabilities.MinImageCount + if imageCount < ^uint32(0) { + imageCount++ + } if capabilities.MaxImageCount > 0 && imageCount > capabilities.MaxImageCount { imageCount = capabilities.MaxImageCount } + if imageCount < capabilities.MinImageCount || imageCount == 0 { + return fmt.Errorf("vulkan: surface returned invalid image count range") + } // Use config dimensions as primary source (matching Rust wgpu-hal behavior). // CurrentExtent from the driver is used only for clamping to the valid range. @@ -154,32 +466,22 @@ func (s *Surface) createSwapchain(device *Device, config *hal.SurfaceConfigurati return hal.ErrZeroArea } - // Convert format - vkFormat := textureFormatToVk(config.Format) - - // Convert present mode - presentMode := presentModeToVk(config.PresentMode) - - // Convert usage - imageUsage := vk.ImageUsageFlags(vk.ImageUsageColorAttachmentBit) - if config.Usage&gputypes.TextureUsageCopySrc != 0 { - imageUsage |= vk.ImageUsageFlags(vk.ImageUsageTransferSrcBit) - } - if config.Usage&gputypes.TextureUsageCopyDst != 0 { - imageUsage |= vk.ImageUsageFlags(vk.ImageUsageTransferDstBit) - } + vkFormat := selectedFormat.Format - // Handle old swapchain - destroy resources (semaphores + image views) BEFORE creating new. - // Using destroyResources() instead of releaseSyncResources() ensures image views from - // the old swapchain are properly cleaned up, preventing "VkImageView has not been - // destroyed" validation errors on device destruction. - var oldSwapchain vk.SwapchainKHR + // Wait for an existing swapchain before passing it as OldSwapchain. This + // keeps its semaphores and views intact if creation or image enumeration + // fails; reconfiguration is transactional until the replacement is ready. + var oldSwapchain *Swapchain + var oldSwapchainHandle vk.SwapchainKHR if s.swapchain != nil { - oldSwapchain = s.swapchain.handle - // Destroy semaphores AND image views BEFORE creating new swapchain. - // This does vkDeviceWaitIdle + destroy semaphores + destroy image views, - // but NOT the swapchain handle (destroyed after new one is created). - s.swapchain.destroyResources() + if s.swapchain.device != device { + return fmt.Errorf("vulkan: cannot reconfigure a surface with a different device") + } + oldSwapchain = s.swapchain + oldSwapchainHandle = oldSwapchain.handle + if result := vkDeviceWaitIdle(device); result != vk.Success { + return fmt.Errorf("vulkan: vkDeviceWaitIdle before reconfigure failed: %d", result) + } } // Create swapchain (passing old handle for seamless transition) @@ -188,49 +490,34 @@ func (s *Surface) createSwapchain(device *Device, config *hal.SurfaceConfigurati Surface: s.handle, MinImageCount: imageCount, ImageFormat: vkFormat, - ImageColorSpace: vk.ColorSpaceSrgbNonlinearKhr, + ImageColorSpace: selectedFormat.ColorSpace, ImageExtent: extent, ImageArrayLayers: 1, ImageUsage: imageUsage, ImageSharingMode: vk.SharingModeExclusive, PreTransform: capabilities.CurrentTransform, - CompositeAlpha: vk.CompositeAlphaOpaqueBitKhr, + CompositeAlpha: compositeAlpha, PresentMode: presentMode, Clipped: vk.True, - OldSwapchain: oldSwapchain, + OldSwapchain: oldSwapchainHandle, } var swapchainHandle vk.SwapchainKHR - result = vkCreateSwapchainKHR(device, &createInfo, nil, &swapchainHandle) + result := vkCreateSwapchainKHR(device, &createInfo, nil, &swapchainHandle) if result != vk.Success { - switch result { - case vk.ErrorSurfaceLostKhr, vk.ErrorInitializationFailed: + if result == vk.ErrorSurfaceLostKhr { return hal.ErrSurfaceLost - default: - return fmt.Errorf("vulkan: vkCreateSwapchainKHR failed: %d", result) } - } - - // Destroy old swapchain AFTER creating new (Vulkan requirement) - if oldSwapchain != 0 { - vkDestroySwapchainKHR(device, oldSwapchain, nil) - s.swapchain = nil + return fmt.Errorf("vulkan: vkCreateSwapchainKHR failed: %d", result) } // Get swapchain images - var swapchainImageCount uint32 - result = vkGetSwapchainImagesKHR(device, swapchainHandle, &swapchainImageCount, nil) - if result != vk.Success { + images, err := querySwapchainImages(device, swapchainHandle) + if err != nil { vkDestroySwapchainKHR(device, swapchainHandle, nil) - return fmt.Errorf("vulkan: vkGetSwapchainImagesKHR (count) failed: %d", result) - } - - images := make([]vk.Image, swapchainImageCount) - result = vkGetSwapchainImagesKHR(device, swapchainHandle, &swapchainImageCount, &images[0]) - if result != vk.Success { - vkDestroySwapchainKHR(device, swapchainHandle, nil) - return fmt.Errorf("vulkan: vkGetSwapchainImagesKHR (images) failed: %d", result) + return err } + swapchainImageCount := uint32(len(images)) // Log actual swapchain creation result (wgpu#185: HiDPI diagnostic). hal.Logger().Info("vulkan: swapchain created", @@ -291,8 +578,8 @@ func (s *Surface) createSwapchain(device *Device, config *hal.SurfaceConfigurati } // Create arrays for rotating semaphores (same count as images). - acquireSemaphores := make([]vk.Semaphore, imageCount) - presentSemaphores := make([]vk.Semaphore, imageCount) + acquireSemaphores := make([]vk.Semaphore, len(images)) + presentSemaphores := make([]vk.Semaphore, len(images)) // Create acquire semaphores for i := range acquireSemaphores { @@ -394,37 +681,75 @@ func (s *Surface) createSwapchain(device *Device, config *hal.SurfaceConfigurati SType: vk.StructureTypeFenceCreateInfo, } fenceResult := device.cmds.CreateFence(device.handle, &fenceInfo, nil, &acquireFence) - if fenceResult == vk.Success { - swapchain.acquireFence = acquireFence + if fenceResult != vk.Success { + _ = swapchain.destroyResources() + vkDestroySwapchainKHR(device, swapchainHandle, nil) + return fmt.Errorf("vulkan: vkCreateFence (acquire) failed: %d", fenceResult) } + swapchain.acquireFence = acquireFence // Link swapchain to surface textures for _, tex := range surfaceTextures { tex.swapchain = swapchain } + // Retire the old swapchain only after the replacement has fully succeeded. + // Its handle remains valid until its views and synchronization primitives are + // released, as required by Vulkan's OldSwapchain transition rules. + if err := retireOldSwapchain(device, oldSwapchain, oldSwapchainHandle); err != nil { + _ = swapchain.destroyResources() + vkDestroySwapchainKHR(device, swapchainHandle, nil) + return err + } + s.swapchain = swapchain s.device = device return nil } -// releaseSyncResources releases synchronization primitives (semaphores) BEFORE -// creating a new swapchain. This must be called before vkCreateSwapchainKHR -// when reconfiguring, as semaphores may be in pending state. -// Does NOT destroy the swapchain handle - that's done after creating the new one. -func (sc *Swapchain) releaseSyncResources() { - if sc.device == nil { - return +func retireOldSwapchain(device *Device, oldSwapchain *Swapchain, oldHandle vk.SwapchainKHR) error { + if oldSwapchain == nil { + return nil } + if err := oldSwapchain.destroyResourcesAfterIdle(); err != nil { + return fmt.Errorf("vulkan: retire old swapchain: %w", err) + } + if oldHandle != 0 { + vkDestroySwapchainKHR(device, oldHandle, nil) + } + oldSwapchain.handle = 0 + oldSwapchain.destroyed = true + if device.queue == nil { + return nil + } + device.queue.mu.Lock() + if device.queue.activeSwapchain == oldSwapchain { + device.queue.activeSwapchain = nil + device.queue.acquireUsed = false + } + device.queue.mu.Unlock() + return nil +} - // Wait for device idle before destroying semaphores. - // This is required because semaphores may be in pending state. - // TODO: For better responsiveness, implement render thread architecture - // like Ebiten (separate threads for events, game logic, rendering). - vkDeviceWaitIdle(sc.device) +// releaseSyncResources releases synchronization primitives after the device is +// idle. It keeps the idle wait explicit so callers can propagate failures and +// reconfiguration can remain transactional. +func (sc *Swapchain) releaseSyncResources() error { + if sc == nil || sc.device == nil { + return nil + } + if result := vkDeviceWaitIdle(sc.device); result != vk.Success { + return fmt.Errorf("vulkan: vkDeviceWaitIdle before releasing swapchain synchronization failed: %d", result) + } + sc.releaseSyncResourcesAfterIdle() + return nil +} - // Destroy acquire semaphores +func (sc *Swapchain) releaseSyncResourcesAfterIdle() { + if sc == nil || sc.device == nil { + return + } for i, sem := range sc.acquireSemaphores { if sem != 0 { vkDestroySemaphore(sc.device, sem, nil) @@ -433,7 +758,6 @@ func (sc *Swapchain) releaseSyncResources() { } sc.acquireSemaphores = nil - // Destroy present semaphores for i, sem := range sc.presentSemaphores { if sem != 0 { vkDestroySemaphore(sc.device, sem, nil) @@ -441,22 +765,29 @@ func (sc *Swapchain) releaseSyncResources() { } } sc.presentSemaphores = nil - - // Reset state + sc.acquireFenceValues = nil sc.imageAcquired = false } -// destroyResources destroys swapchain resources (image views) after the -// swapchain handle has been destroyed or replaced. -func (sc *Swapchain) destroyResources() { - if sc.device == nil { - return +// destroyResources destroys swapchain resources after waiting for the device. +func (sc *Swapchain) destroyResources() error { + if sc == nil || sc.device == nil { + return nil + } + if err := sc.releaseSyncResources(); err != nil { + return err } + return sc.destroyResourcesAfterIdle() +} - // Release sync resources if not already done - sc.releaseSyncResources() +// destroyResourcesAfterIdle is used by transactional reconfiguration after a +// successful vkDeviceWaitIdle. It performs no unchecked synchronization wait. +func (sc *Swapchain) destroyResourcesAfterIdle() error { + if sc == nil || sc.device == nil { + return nil + } + sc.releaseSyncResourcesAfterIdle() - // Destroy image views for _, view := range sc.imageViews { if view != 0 { vkDestroyImageViewSwapchain(sc.device, view, nil) @@ -466,32 +797,57 @@ func (sc *Swapchain) destroyResources() { sc.images = nil sc.surfaceTextures = nil - // Destroy post-acquire fence if sc.acquireFence != 0 { sc.device.cmds.DestroyFence(sc.device.handle, sc.acquireFence, nil) sc.acquireFence = 0 } - // BUG-WGPU-VK-006: Destroy barrier fence and command pool. if sc.barrierFence != 0 { sc.device.cmds.DestroyFence(sc.device.handle, sc.barrierFence, nil) sc.barrierFence = 0 } if sc.barrierPool != 0 { + // vkDestroyCommandPool is a void Vulkan command; there is no result to + // propagate. DeviceWaitIdle above establishes the required lifetime + // guarantee before this teardown. sc.device.cmds.DestroyCommandPool(sc.device.handle, sc.barrierPool, nil) sc.barrierPool = 0 } sc.imageLayouts = nil + sc.currentAcquireSem = 0 + sc.currentAcquireIdx = 0 + sc.nextAcquireIdx = 0 + return nil } -// Destroy destroys the swapchain completely. +// Destroy destroys the swapchain completely. It is idempotent. The public HAL +// method keeps its historical no-result signature; internal owners use +// destroyWithError when they need to preserve a failed synchronization result. func (sc *Swapchain) Destroy() { - sc.destroyResources() + if err := sc.destroyWithError(); err != nil { + hal.Logger().Error("vulkan: failed to destroy swapchain", "error", err) + } +} - if sc.handle != 0 && sc.device != nil { +func (sc *Swapchain) destroyWithError() error { + if sc == nil || sc.destroyed { + return nil + } + if sc.device == nil { + sc.destroyed = true + sc.handle = 0 + return nil + } + if err := sc.destroyResources(); err != nil { + return err + } + if sc.handle != 0 { vkDestroySwapchainKHR(sc.device, sc.handle, nil) sc.handle = 0 } + sc.destroyed = true + sc.broken = true + return nil } // acquireNextImage acquires the next available swapchain image. @@ -504,9 +860,25 @@ func (sc *Swapchain) Destroy() { // - Returns nil on timeout instead of blocking forever // - Caller should skip frame rendering on nil return func (sc *Swapchain) acquireNextImage() (*SwapchainTexture, bool, error) { + if err := sc.stateError("acquire an image from"); err != nil { + return nil, false, err + } if sc.imageAcquired { return nil, false, fmt.Errorf("vulkan: image already acquired") } + if sc.device == nil || sc.handle == 0 { + return nil, false, hal.ErrSurfaceLost + } + if len(sc.acquireSemaphores) == 0 || len(sc.acquireFenceValues) != len(sc.acquireSemaphores) { + err := fmt.Errorf("vulkan: swapchain acquire synchronization is unavailable") + sc.markBroken(err) + return nil, false, err + } + if len(sc.surfaceTextures) == 0 || len(sc.images) != len(sc.surfaceTextures) || len(sc.presentSemaphores) != len(sc.surfaceTextures) || len(sc.imageLayouts) != len(sc.surfaceTextures) { + err := fmt.Errorf("vulkan: swapchain image state is inconsistent") + sc.markBroken(err) + return nil, false, err + } // Timeout for acquire - match wgpu-core's FRAME_TIMEOUT_MS = 1000 // This is the proven timeout that works across drivers. @@ -515,7 +887,17 @@ func (sc *Swapchain) acquireNextImage() (*SwapchainTexture, bool, error) { // Get the acquire semaphore from the rotating pool. acquireIdx := sc.nextAcquireIdx + if acquireIdx < 0 || acquireIdx >= len(sc.acquireSemaphores) { + err := fmt.Errorf("vulkan: acquire semaphore index %d is out of range", acquireIdx) + sc.markBroken(err) + return nil, false, err + } acquireSem := sc.acquireSemaphores[acquireIdx] + if acquireSem == 0 { + err := fmt.Errorf("vulkan: acquire semaphore %d has been destroyed", acquireIdx) + sc.markBroken(err) + return nil, false, err + } // Pre-acquire wait: ensure the GPU has consumed this semaphore from // a previous frame's Submit before we pass it to vkAcquireNextImageKHR again. @@ -523,9 +905,12 @@ func (sc *Swapchain) acquireNextImage() (*SwapchainTexture, bool, error) { // violating VUID-vkAcquireNextImageKHR-semaphore-01779. // See: wgpu-hal/src/vulkan/swapchain/native.rs — previously_used_submission_index if prevValue := sc.acquireFenceValues[acquireIdx]; prevValue > 0 { - _ = sc.device.timelineFence.waitForValue( + if err := sc.device.timelineFence.waitForValue( sc.device.cmds, sc.device.handle, prevValue, timeout, - ) + ); err != nil { + sc.markBroken(fmt.Errorf("vulkan: wait for acquire semaphore %d: %w", acquireIdx, err)) + return nil, false, sc.failureErr + } } // Pass acquireFence to vkAcquireNextImageKHR for post-acquire frame pacing. @@ -545,13 +930,22 @@ func (sc *Swapchain) acquireNextImage() (*SwapchainTexture, bool, error) { case vk.NotReady, vk.ErrorOutOfDateKhr: // Surface needs reconfiguration // (wgpu: returns Err(Outdated)) + if result == vk.ErrorOutOfDateKhr { + sc.markBroken(hal.ErrSurfaceOutdated) + } return nil, false, hal.ErrSurfaceOutdated case vk.ErrorSurfaceLostKhr: // Surface destroyed (e.g., Wayland compositor killed it). // (wgpu: returns Err(Lost)) + sc.markBroken(hal.ErrSurfaceLost) return nil, false, hal.ErrSurfaceLost + case vk.ErrorDeviceLost: + sc.markBroken(hal.ErrDeviceLost) + return nil, false, hal.ErrDeviceLost default: - return nil, false, fmt.Errorf("vulkan: vkAcquireNextImageKHR failed: %d", result) + err := fmt.Errorf("vulkan: vkAcquireNextImageKHR failed: %d", result) + sc.markBroken(err) + return nil, false, err } // Post-acquire fence wait: sync with presentation engine for proper frame pacing. @@ -562,11 +956,19 @@ func (sc *Swapchain) acquireNextImage() (*SwapchainTexture, bool, error) { if fence != 0 { waitResult := sc.device.cmds.WaitForFences(sc.device.handle, 1, &fence, vk.True, timeout) if waitResult != vk.Success { - // Non-fatal: if wait fails, continue without frame pacing. - // This handles drivers that don't support post-acquire fence properly. - _ = waitResult + sc.markBroken(fmt.Errorf("vulkan: vkWaitForFences after acquire failed: %d", waitResult)) + return nil, false, sc.failureErr + } + resetResult := sc.device.cmds.ResetFences(sc.device.handle, 1, &fence) + if resetResult != vk.Success { + sc.markBroken(fmt.Errorf("vulkan: vkResetFences after acquire failed: %d", resetResult)) + return nil, false, sc.failureErr } - _ = sc.device.cmds.ResetFences(sc.device.handle, 1, &fence) + } + if imageIndex >= uint32(len(sc.surfaceTextures)) { + err := fmt.Errorf("vulkan: acquired image index %d is out of range", imageIndex) + sc.markBroken(err) + return nil, false, err } // Store the current acquire index and semaphore for use in Submit. @@ -582,9 +984,7 @@ func (sc *Swapchain) acquireNextImage() (*SwapchainTexture, bool, error) { // BUG-WGPU-VK-006: Per Vulkan spec, acquired swapchain images are in // UNDEFINED layout regardless of previous usage. Track this so present() // can insert a barrier if no render pass transitions to PRESENT_SRC_KHR. - if int(imageIndex) < len(sc.imageLayouts) { - sc.imageLayouts[imageIndex] = vk.ImageLayoutUndefined - } + sc.imageLayouts[imageIndex] = vk.ImageLayoutUndefined return sc.surfaceTextures[imageIndex], result == vk.SuboptimalKhr, nil } @@ -597,12 +997,31 @@ func (sc *Swapchain) acquireNextImage() (*SwapchainTexture, bool, error) { // structure is chained into VkPresentInfoKHR.PNext as a compositor hint. // When empty or unsupported, the present path is identical to a full present. func (sc *Swapchain) present(queue *Queue, damageRects []image.Rectangle) error { + if err := sc.stateError("present from"); err != nil { + return err + } if !sc.imageAcquired { return fmt.Errorf("vulkan: no image acquired to present") } - if sc.handle == 0 { + if queue == nil || queue.device != sc.device { + return fmt.Errorf("vulkan: present queue does not belong to swapchain device") + } + if queue.activeSwapchain != sc || !queue.acquireUsed { + err := fmt.Errorf("vulkan: no successful swapchain submission is ready for presentation") + sc.markBroken(err) + return err + } + if sc.handle == 0 || sc.currentImage >= uint32(len(sc.presentSemaphores)) || sc.currentImage >= uint32(len(sc.images)) || sc.images[sc.currentImage] == 0 { + err := hal.ErrSurfaceLost + sc.markBroken(err) return hal.ErrSurfaceLost } + presentSem := sc.presentSemaphores[sc.currentImage] + if presentSem == 0 { + err := fmt.Errorf("vulkan: present semaphore for image %d has been destroyed", sc.currentImage) + sc.markBroken(err) + return err + } // BUG-WGPU-VK-006: Ensure the swapchain image is in PRESENT_SRC_KHR layout // before vkQueuePresentKHR. When a render pass directly targets the swapchain @@ -611,16 +1030,14 @@ func (sc *Swapchain) present(queue *Queue, damageRects []image.Rectangle) error // differently (blit-only, offscreen-only, resolve target without PRESENT_SRC), // this inserts an explicit pipeline barrier to transition the layout. if err := sc.ensurePresentLayout(queue); err != nil { - hal.Logger().Warn("vulkan: present layout transition failed", + sc.markBroken(fmt.Errorf("vulkan: present layout transition failed: %w", err)) + hal.Logger().Error("vulkan: present layout transition failed", "err", err, "imageIndex", sc.currentImage) - // Non-fatal: attempt present anyway. The validation layer will report - // the layout mismatch, but many drivers tolerate it. + // The image's semaphore/layout state is no longer safe to reuse. A + // reconfigure or destroy must drain the device before cleanup. + return sc.failureErr } - // Use the present semaphore for the current image. - // Submit signals this, and present waits on it. - presentSem := sc.presentSemaphores[sc.currentImage] - presentInfo := vk.PresentInfoKHR{ SType: vk.StructureTypePresentInfoKhr, WaitSemaphoreCount: 1, @@ -668,11 +1085,18 @@ func (sc *Swapchain) present(queue *Queue, damageRects []image.Rectangle) error // Suboptimal but presented successfully return nil case vk.ErrorOutOfDateKhr: + sc.markBroken(hal.ErrSurfaceOutdated) return hal.ErrSurfaceOutdated case vk.ErrorSurfaceLostKhr: + sc.markBroken(hal.ErrSurfaceLost) return hal.ErrSurfaceLost + case vk.ErrorDeviceLost: + sc.markBroken(hal.ErrDeviceLost) + return hal.ErrDeviceLost default: - return fmt.Errorf("vulkan: vkQueuePresentKHR failed: %d", result) + err := fmt.Errorf("vulkan: vkQueuePresentKHR failed: %d", result) + sc.markBroken(err) + return err } } @@ -706,7 +1130,10 @@ func (sc *Swapchain) SetImageLayout(imageIndex uint32, layout vk.ImageLayout) { func (sc *Swapchain) ensurePresentLayout(queue *Queue) error { idx := sc.currentImage if int(idx) >= len(sc.imageLayouts) { - return nil // defensive: out of range + return fmt.Errorf("vulkan: swapchain image layout index %d is out of range", idx) + } + if sc.device == nil || queue == nil || queue.device != sc.device { + return fmt.Errorf("vulkan: invalid queue for present layout transition") } currentLayout := sc.imageLayouts[idx] @@ -739,6 +1166,8 @@ func (sc *Swapchain) ensurePresentLayout(queue *Queue) error { var fence vk.Fence fenceResult := sc.device.cmds.CreateFence(sc.device.handle, &fenceInfo, nil, &fence) if fenceResult != vk.Success { + sc.device.cmds.DestroyCommandPool(sc.device.handle, pool, nil) + sc.barrierPool = 0 return fmt.Errorf("vulkan: vkCreateFence (barrier) failed: %d", fenceResult) } sc.device.setObjectName(vk.ObjectTypeFence, uint64(fence), "PresentBarrierFence") @@ -845,9 +1274,6 @@ func (sc *Swapchain) ensurePresentLayout(queue *Queue) error { return fmt.Errorf("vulkan: vkQueueSubmit (barrier) failed: %d", result) } - // Update tracked layout. - sc.imageLayouts[idx] = vk.ImageLayoutPresentSrcKhr - // Wait for the barrier submission to complete on the GPU before resetting // the command pool. Without this wait, the command buffer is still pending // and vkResetCommandPool violates VUID-vkResetCommandPool-commandPool-00040. @@ -857,12 +1283,25 @@ func (sc *Swapchain) ensurePresentLayout(queue *Queue) error { // swapchain image to PRESENT_SRC_KHR). In the common case (render pass with // finalLayout = PRESENT_SRC_KHR), ensurePresentLayout returns early above. const barrierTimeout = uint64(1_000_000_000) // 1 second - sc.device.cmds.WaitForFences(sc.device.handle, 1, &sc.barrierFence, vk.True, barrierTimeout) - sc.device.cmds.ResetFences(sc.device.handle, 1, &sc.barrierFence) + waitResult := sc.device.cmds.WaitForFences(sc.device.handle, 1, &sc.barrierFence, vk.True, barrierTimeout) + if waitResult != vk.Success { + return fmt.Errorf("vulkan: vkWaitForFences (barrier) failed: %d", waitResult) + } + resetResult := sc.device.cmds.ResetFences(sc.device.handle, 1, &sc.barrierFence) + if resetResult != vk.Success { + return fmt.Errorf("vulkan: vkResetFences (barrier) failed: %d", resetResult) + } // Reset the command pool so the buffer can be reused next frame. // Safe now because WaitForFences guarantees the command buffer is complete. - sc.device.cmds.ResetCommandPool(sc.device.handle, sc.barrierPool, 0) + resetPoolResult := sc.device.cmds.ResetCommandPool(sc.device.handle, sc.barrierPool, 0) + if resetPoolResult != vk.Success { + return fmt.Errorf("vulkan: vkResetCommandPool (barrier) failed: %d", resetPoolResult) + } + + // Update tracked layout only after the barrier completed and its command + // buffer was safely recycled. + sc.imageLayouts[idx] = vk.ImageLayoutPresentSrcKhr hal.Logger().Debug("vulkan: inserted PRESENT_SRC_KHR barrier", "imageIndex", idx, "oldLayout", currentLayout) diff --git a/hal/vulkan/swapchain_fail_closed_test.go b/hal/vulkan/swapchain_fail_closed_test.go new file mode 100644 index 0000000..bcb0d56 --- /dev/null +++ b/hal/vulkan/swapchain_fail_closed_test.go @@ -0,0 +1,301 @@ +//go:build !(js && wasm) + +package vulkan + +import ( + "errors" + "testing" + + "github.com/gogpu/gputypes" + "github.com/gogpu/wgpu/hal" + "github.com/gogpu/wgpu/hal/vulkan/vk" +) + +func validSwapchainConfig() *hal.SurfaceConfiguration { + return &hal.SurfaceConfiguration{ + Format: gputypes.TextureFormatRGBA8Unorm, + Usage: gputypes.TextureUsageRenderAttachment, + PresentMode: hal.PresentModeFifo, + AlphaMode: hal.CompositeAlphaModeOpaque, + } +} + +func validSwapchainSnapshot() swapchainSurfaceSnapshot { + return swapchainSurfaceSnapshot{ + capabilities: vk.SurfaceCapabilitiesKHR{ + SupportedTransforms: vk.SurfaceTransformFlagsKHR(vk.SurfaceTransformIdentityBitKhr), + CurrentTransform: vk.SurfaceTransformIdentityBitKhr, + SupportedCompositeAlpha: vk.CompositeAlphaFlagsKHR(vk.CompositeAlphaOpaqueBitKhr), + SupportedUsageFlags: vk.ImageUsageFlags(vk.ImageUsageColorAttachmentBit), + }, + formats: []vk.SurfaceFormatKHR{{ + Format: vk.FormatR8g8b8a8Unorm, + ColorSpace: vk.ColorSpaceSrgbNonlinearKhr, + }}, + presentModes: []vk.PresentModeKHR{vk.PresentModeFifoKhr}, + } +} + +func TestValidateSurfaceSnapshotUsesReportedCapabilities(t *testing.T) { + format, mode, alpha, usage, err := validateSurfaceSnapshot(validSwapchainSnapshot(), validSwapchainConfig()) + if err != nil { + t.Fatalf("validateSurfaceSnapshot() error: %v", err) + } + if format.Format != vk.FormatR8g8b8a8Unorm || format.ColorSpace != vk.ColorSpaceSrgbNonlinearKhr { + t.Fatalf("selected format = %+v, want exact reported pair", format) + } + if mode != vk.PresentModeFifoKhr { + t.Fatalf("selected present mode = %v, want FIFO", mode) + } + if alpha != vk.CompositeAlphaOpaqueBitKhr { + t.Fatalf("selected alpha mode = %v, want opaque", alpha) + } + if usage != vk.ImageUsageFlags(vk.ImageUsageColorAttachmentBit) { + t.Fatalf("selected image usage = %v, want color attachment", usage) + } +} + +func TestValidateSurfaceSnapshotRejectsUnsupportedRequests(t *testing.T) { + snapshot := validSwapchainSnapshot() + config := validSwapchainConfig() + config.Format = gputypes.TextureFormatBGRA8Unorm + if _, _, _, _, err := validateSurfaceSnapshot(snapshot, config); err == nil { + t.Fatal("unsupported format was accepted") + } + + config = validSwapchainConfig() + config.PresentMode = hal.PresentModeImmediate + if _, _, _, _, err := validateSurfaceSnapshot(snapshot, config); err == nil { + t.Fatal("unsupported present mode was accepted") + } + + config = validSwapchainConfig() + config.AlphaMode = hal.CompositeAlphaModePremultiplied + if _, _, _, _, err := validateSurfaceSnapshot(snapshot, config); err == nil { + t.Fatal("unsupported alpha mode was accepted") + } +} + +func TestValidateSurfaceSnapshotRejectsIncompleteCapabilities(t *testing.T) { + config := validSwapchainConfig() + snapshot := validSwapchainSnapshot() + + snapshot.formats = nil + if _, _, _, _, err := validateSurfaceSnapshot(snapshot, config); err == nil { + t.Fatal("empty format capabilities were accepted") + } + + snapshot = validSwapchainSnapshot() + snapshot.presentModes = nil + if _, _, _, _, err := validateSurfaceSnapshot(snapshot, config); err == nil { + t.Fatal("empty present-mode capabilities were accepted") + } + + snapshot = validSwapchainSnapshot() + snapshot.capabilities.SupportedCompositeAlpha = 0 + if _, _, _, _, err := validateSurfaceSnapshot(snapshot, config); err == nil { + t.Fatal("empty alpha capabilities were accepted") + } + + snapshot = validSwapchainSnapshot() + snapshot.capabilities.CurrentTransform = 0 + if _, _, _, _, err := validateSurfaceSnapshot(snapshot, config); err == nil { + t.Fatal("empty current transform was accepted") + } + + if _, _, _, _, err := validateSurfaceSnapshot(snapshot, nil); err == nil { + t.Fatal("nil surface configuration was accepted") + } +} + +func TestFormatSelectionPreservesNonlinearPreferenceAndExactPair(t *testing.T) { + snapshot := validSwapchainSnapshot() + snapshot.formats = []vk.SurfaceFormatKHR{ + {Format: vk.FormatR8g8b8a8Unorm, ColorSpace: vk.ColorSpaceDisplayP3NonlinearExt}, + {Format: vk.FormatR8g8b8a8Unorm, ColorSpace: vk.ColorSpaceSrgbNonlinearKhr}, + } + selected, err := snapshot.formatFor(gputypes.TextureFormatRGBA8Unorm) + if err != nil { + t.Fatalf("formatFor() error: %v", err) + } + if selected.ColorSpace != vk.ColorSpaceSrgbNonlinearKhr { + t.Fatalf("selected color space = %v, want reported sRGB nonlinear pair", selected.ColorSpace) + } + + snapshot.formats = snapshot.formats[:1] + selected, err = snapshot.formatFor(gputypes.TextureFormatRGBA8Unorm) + if err != nil { + t.Fatalf("formatFor(non-sRGB) error: %v", err) + } + if selected.ColorSpace != vk.ColorSpaceDisplayP3NonlinearExt { + t.Fatalf("selected color space = %v, want exact non-sRGB pair", selected.ColorSpace) + } +} + +func TestCompositeAlphaAutoRequiresReportedMode(t *testing.T) { + mode, err := compositeAlphaFor(vk.CompositeAlphaFlagsKHR(vk.CompositeAlphaPreMultipliedBitKhr), hal.CompositeAlphaModeAuto) + if err != nil { + t.Fatalf("compositeAlphaFor() error: %v", err) + } + if mode != vk.CompositeAlphaPreMultipliedBitKhr { + t.Fatalf("auto alpha = %v, want premultiplied", mode) + } + + if _, err := compositeAlphaFor(0, hal.CompositeAlphaModeAuto); err == nil { + t.Fatal("auto alpha fabricated an unsupported mode") + } +} + +func TestSwapchainImageUsageChecksEveryRequestedBit(t *testing.T) { + color := vk.ImageUsageFlags(vk.ImageUsageColorAttachmentBit) + if got, err := swapchainImageUsage(gputypes.TextureUsageNone, color); err != nil { + t.Fatalf("implicit render usage error: %v", err) + } else if got != color { + t.Fatalf("implicit render usage = %v, want %v", got, color) + } + + if _, err := swapchainImageUsage(gputypes.TextureUsageCopyDst, color); err == nil { + t.Fatal("unsupported copy-destination usage was accepted") + } + if _, err := swapchainImageUsage(gputypes.TextureUsage(1<<20), color); err == nil { + t.Fatal("unknown texture usage bit was accepted") + } +} + +func TestQuerySwapchainFormatsRetriesIncomplete(t *testing.T) { + countCalls, fillCalls := 0, 0 + formats, err := querySwapchainFormatsWith(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("querySwapchainFormatsWith() 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 TestQuerySwapchainPresentModesFailsClosed(t *testing.T) { + _, err := querySwapchainPresentModesWith(func(_ *uint32, _ *vk.PresentModeKHR) vk.Result { + return vk.ErrorSurfaceLostKhr + }) + if !errors.Is(err, hal.ErrSurfaceLost) { + t.Fatalf("present-mode query error = %v, want surface-lost error", err) + } + + _, err = querySwapchainPresentModesWith(func(count *uint32, modes *vk.PresentModeKHR) vk.Result { + if modes == nil { + *count = 1 + return vk.Success + } + return vk.Incomplete + }) + if err == nil { + t.Fatal("unstable present-mode query was accepted") + } +} + +func TestQuerySwapchainImagesUsesActualReturnedCount(t *testing.T) { + countCalls, fillCalls := 0, 0 + images, err := querySwapchainImagesWith(func(count *uint32, images *vk.Image) vk.Result { + if images == nil { + countCalls++ + *count = 3 + return vk.Success + } + fillCalls++ + *images = vk.Image(11) + *count = 1 + return vk.Success + }) + if err != nil { + t.Fatalf("querySwapchainImagesWith() error: %v", err) + } + if len(images) != 1 || images[0] != vk.Image(11) || countCalls != 1 || fillCalls != 1 { + t.Fatalf("image query = len %d, first %v, calls (%d, %d); want len 1, image 11, calls (1, 1)", len(images), images[0], countCalls, fillCalls) + } +} + +func TestQuerySwapchainImagesRetriesIncomplete(t *testing.T) { + countCalls, fillCalls := 0, 0 + images, err := querySwapchainImagesWith(func(count *uint32, images *vk.Image) vk.Result { + if images == nil { + countCalls++ + *count = 1 + return vk.Success + } + fillCalls++ + if fillCalls == 1 { + return vk.Incomplete + } + *images = vk.Image(17) + *count = 1 + return vk.Success + }) + if err != nil { + t.Fatalf("querySwapchainImagesWith() error: %v", err) + } + if len(images) != 1 || images[0] != vk.Image(17) || countCalls != 2 || fillCalls != 2 { + t.Fatalf("image query = len %d, first %v, calls (%d, %d); want len 1, image 17, calls (2, 2)", len(images), images[0], countCalls, fillCalls) + } +} + +func TestSwapchainLifecycleGuardsFailClosed(t *testing.T) { + destroyed := &Swapchain{destroyed: true} + if _, _, err := destroyed.acquireNextImage(); err == nil { + t.Fatal("acquire on destroyed swapchain was accepted") + } + + broken := &Swapchain{broken: true, failureErr: errors.New("layout transition failed")} + if err := broken.present(nil, nil); err == nil { + t.Fatal("present on broken swapchain was accepted") + } + device := &Device{} + withoutSubmission := &Swapchain{device: device, imageAcquired: true} + if err := withoutSubmission.present(&Queue{device: device}, nil); err == nil { + t.Fatal("present without a successful queue submission was accepted") + } + if !withoutSubmission.broken { + t.Fatal("present without a successful queue submission did not fail closed") + } + + valid := &Swapchain{ + device: device, + imageAcquired: true, + currentAcquireIdx: 0, + currentImage: 0, + acquireSemaphores: []vk.Semaphore{1}, + acquireFenceValues: []uint64{0}, + presentSemaphores: []vk.Semaphore{2}, + } + if err := validateSwapchainSubmission(valid, device); err != nil { + t.Fatalf("valid submission rejected: %v", err) + } + valid.currentAcquireIdx = 1 + if err := validateSwapchainSubmission(valid, device); err == nil { + t.Fatal("out-of-range acquire semaphore state was accepted") + } + valid.currentAcquireIdx = 0 + valid.broken = true + if err := validateSwapchainSubmission(valid, device); err == nil { + t.Fatal("broken swapchain submission was accepted") + } + + withoutDevice := &Swapchain{} + withoutDevice.Destroy() + if !withoutDevice.destroyed { + t.Fatal("Destroy() without a device did not mark the swapchain destroyed") + } + withoutDevice.Destroy() +}