diff --git a/Sources/TUIkit/Views/_ImageCore.swift b/Sources/TUIkit/Views/_ImageCore.swift index b994a255..21848787 100644 --- a/Sources/TUIkit/Views/_ImageCore.swift +++ b/Sources/TUIkit/Views/_ImageCore.swift @@ -13,7 +13,11 @@ private enum StateIndex { /// Stores the loading phase (`ImageLoadingPhase`). static let phase = 0 - /// Stores the last loaded source for change detection (`ImageSource`). + /// Stores the source of the last committed load (`ImageSource?`). + /// + /// Written exclusively by the mounted loading task, never during + /// traversal. A mismatch with the current source means the stored + /// phase belongs to an outdated source. static let lastSource = 1 } @@ -45,7 +49,6 @@ struct _ImageCore: View, Renderable, Layoutable { func renderToBuffer(context: RenderContext) -> FrameBuffer { let stateStorage = context.environment.stateStorage! - let lifecycle = context.environment.lifecycle! let identity = context.identity let width = context.availableWidth @@ -63,10 +66,6 @@ struct _ImageCore: View, Renderable, Layoutable { let aspectRatioOverride = context.environment.imageAspectRatio let placeholderText = context.environment.imagePlaceholderText let showSpinner = context.environment.imagePlaceholderSpinner - let maxPixelCount = context.environment.imageMaxPixelCount - let urlTimeout = context.environment.imageURLTimeout - let imageLoader = context.environment.imageLoader - let imageCache = context.environment.imageCache // Retrieve or create persistent phase state let phaseKey = StateStorage.StateKey(identity: identity, propertyIndex: StateIndex.phase) @@ -78,14 +77,18 @@ struct _ImageCore: View, Renderable, Layoutable { stateStorage.markActive(identity) } - // Track the last loaded source to detect changes + // Track the source of the last committed load let sourceKey = StateStorage.StateKey(identity: identity, propertyIndex: StateIndex.lastSource) let lastSourceBox: StateBox = stateStorage.storage(for: sourceKey, default: nil) - // Detect source change and show the placeholder for the new load. - // Task cancellation and restart are handled by the updateTask - // restart ID below (the source itself). - resetLoadingPhaseIfNeeded(phaseBox: phaseBox, lastSourceBox: lastSourceBox) + // The placeholder for a changed source is DERIVED instead of + // written: state writes during traversal violate the render-phase + // contract (they would emit RuntimeDiagnostics). The stored phase + // only counts while it belongs to the current source; the mounted + // task commits the new phase and source after the frame. + let phase: ImageLoadingPhase = lastSourceBox.value == source + ? phaseBox.value + : .loading // The loading task is a lifetime effect bound to this image's // structural identity. The SOURCE is the restart ID: an unchanged @@ -95,18 +98,13 @@ struct _ImageCore: View, Renderable, Layoutable { if context.phase == .render { mountLoadingTask( phaseBox: phaseBox, - identity: identity, - context: context, - lifecycle: lifecycle, - imageLoader: imageLoader, - imageCache: imageCache, - urlTimeout: urlTimeout, - maxPixelCount: maxPixelCount + lastSourceBox: lastSourceBox, + context: context ) } - // Render based on current phase - switch phaseBox.value { + // Render based on the derived phase + switch phase { case .loading: return renderPlaceholder( width: width, @@ -141,17 +139,22 @@ extension _ImageCore { /// Mounts the async loading task for the current source (lifetime /// effect: recorded for the frame commit inside RenderLoop passes, /// immediate on the live path). + /// + /// The task is the only writer of `phaseBox` and `lastSourceBox`; it + /// commits them together once the load finishes, from outside the + /// traversal window. Loader, cache, and limits are read from the + /// render context's environment. private func mountLoadingTask( phaseBox: StateBox, - identity: ViewIdentity, - context: RenderContext, - lifecycle: LifecycleManager, - imageLoader: any ImageLoader, - imageCache: URLImageCache, - urlTimeout: TimeInterval, - maxPixelCount: Int? + lastSourceBox: StateBox, + context: RenderContext ) { - let taskIdentity = identity.scoped("image.load") + let lifecycle = context.environment.lifecycle! + let imageLoader = context.environment.imageLoader + let imageCache = context.environment.imageCache + let urlTimeout = context.environment.imageURLTimeout + let maxPixelCount = context.environment.imageMaxPixelCount + let taskIdentity = context.identity.scoped("image.load") let src = source let mount = { [imageLoader, imageCache] in _ = lifecycle.updateTask( @@ -159,6 +162,7 @@ extension _ImageCore { id: src, priority: .userInitiated ) { + let loadedPhase: ImageLoadingPhase do { let rawImage: RGBAImage switch src { @@ -174,14 +178,23 @@ extension _ImageCore { } // Store the raw image; conversion happens per render pass. - // StateBox.didSet triggers setNeedsRender() automatically. - // The runtime delivers that invalidation to its async event loop. - phaseBox.value = .success(rawImage) + loadedPhase = .success(rawImage) } catch let loadError as ImageLoadError { - phaseBox.value = .failure(loadError.description) + loadedPhase = .failure(loadError.description) } catch { - phaseBox.value = .failure(error.localizedDescription) + loadedPhase = .failure(error.localizedDescription) } + + // A cancelled task lost its slot to a newer source; + // committing its result would overwrite the replacement's. + guard !Task.isCancelled else { return } + + // Phase first, then source: a render between the two writes + // still derives the placeholder instead of pairing the fresh + // phase with the outdated source. StateBox.didSet triggers + // setNeedsRender() for each write. + phaseBox.value = loadedPhase + lastSourceBox.value = src } } if let pendingEffects = context.environment.pendingFrameEffects { @@ -190,20 +203,6 @@ extension _ImageCore { mount() } } - - /// Resets the loading phase when the image source changes. - /// - /// Mutates state during rendering on purpose: the placeholder must show - /// in the SAME frame the new source appears, not one frame later. - private func resetLoadingPhaseIfNeeded( - phaseBox: StateBox, - lastSourceBox: StateBox - ) { - if let lastSource = lastSourceBox.value, lastSource != source { - phaseBox.value = .loading - } - lastSourceBox.value = source - } } // MARK: - Image Rendering diff --git a/Tests/TUIkitTests/ImageRenderPhaseTests.swift b/Tests/TUIkitTests/ImageRenderPhaseTests.swift new file mode 100644 index 00000000..89a310ff --- /dev/null +++ b/Tests/TUIkitTests/ImageRenderPhaseTests.swift @@ -0,0 +1,131 @@ +// 🖥️ TUIKit — Terminal UI Kit for Swift +// ImageRenderPhaseTests.swift +// +// License: MIT + +import Foundation +import Testing + +@testable import TUIkit + +/// Specifies the render-phase contract for `_ImageCore` (issue #27 slice): +/// rendering an image must never mutate state inside the traversal window. +/// The placeholder for a changed source is DERIVED while rendering; the +/// mounted loading task commits phase and last source from outside the +/// window, so no frame ever emits a body-side-effect diagnostic. +@MainActor +@Suite("Image Render Phase", .serialized) +struct ImageRenderPhaseTests { + + @Test("Rendering an image emits no traversal diagnostics", .timeLimit(.minutes(1))) + func renderingEmitsNoTraversalDiagnostics() async throws { + let tuiContext = TUIContext(imageLoader: StubImageLoader()) + let harness = FrameHarness(app: SwappableImageApp(), tuiContext: tuiContext) + + harness.renderFrame() + + #expect(tuiContext.runtimeDiagnostics.messages.isEmpty) + + let phaseBox = try #require(imagePhaseBox(in: tuiContext)) + while case .loading = phaseBox.value { await Task.yield() } + harness.renderFrame() + + #expect(tuiContext.runtimeDiagnostics.messages.isEmpty) + } + + @Test("Source change derives the placeholder without state writes", .timeLimit(.minutes(1))) + func sourceChangeDerivesPlaceholder() async throws { + let tuiContext = TUIContext(imageLoader: StubImageLoader()) + let app = SwappableImageApp() + let harness = FrameHarness(app: app, tuiContext: tuiContext) + + harness.renderFrame() + let phaseBox = try #require(imagePhaseBox(in: tuiContext)) + while case .loading = phaseBox.value { await Task.yield() } + harness.renderFrame() + + app.cell.source = .file("/stub/b.png") + let outputCountBeforeChange = harness.terminal.writtenOutput.count + harness.renderFrame() + + // The placeholder appears in the SAME frame as the new source … + let changeFrameOutput = harness.terminal.writtenOutput[outputCountBeforeChange...].joined() + #expect(changeFrameOutput.contains(placeholderSpinnerGlyph)) + // … and nothing mutated state during the traversal. + #expect(tuiContext.runtimeDiagnostics.messages.isEmpty) + + // The replacement task commits phase and source after the frame. + let sourceBox = try #require(imageLastSourceBox(in: tuiContext)) + while sourceBox.value != .file("/stub/b.png") { await Task.yield() } + harness.renderFrame() + + #expect(tuiContext.runtimeDiagnostics.messages.isEmpty) + } +} + +// MARK: - State Access + +/// The spinner glyph `_ImageCore.renderPlaceholder` draws while loading. +private let placeholderSpinnerGlyph = "⠋" + +/// Returns the image view's phase box (`_ImageCore` property index 0). +/// +/// The fixtures render exactly one image, so the single stored identity is +/// the image's. Returns `nil` when no frame has hydrated state yet. +@MainActor +private func imagePhaseBox(in tuiContext: TUIContext) -> StateBox? { + guard let identity = tuiContext.stateStorage.storedIdentities.first else { return nil } + return tuiContext.stateStorage.storage( + for: StateStorage.StateKey(identity: identity, propertyIndex: 0), + default: .loading + ) +} + +/// Returns the image view's last-source box (`_ImageCore` property index 1). +@MainActor +private func imageLastSourceBox(in tuiContext: TUIContext) -> StateBox? { + guard let identity = tuiContext.stateStorage.storedIdentities.first else { return nil } + return tuiContext.stateStorage.storage( + for: StateStorage.StateKey(identity: identity, propertyIndex: 1), + default: nil + ) +} + +// MARK: - Fixtures + +/// Serves a deterministic single-pixel image for any request, keeping the +/// file system and network out of the tests. +private struct StubImageLoader: ImageLoader { + private let image = RGBAImage( + width: 1, + height: 1, + pixels: [RGBA(r: 255, g: 255, b: 255)] + ) + + func loadImage(from path: String) throws -> RGBAImage { + image + } + + func loadImage(from data: Data) throws -> RGBAImage { + image + } +} + +/// Mutable source slot shared between a test and its app fixture. +@MainActor +private final class ImageSourceCell { + var source: ImageSource = .file("/stub/a.png") +} + +/// Renders one image whose source the test swaps between frames. +private struct SwappableImageApp: App { + let cell = ImageSourceCell() + + init() {} + + var body: some Scene { + WindowGroup { + Image(cell.source) + } + } +} diff --git a/Tests/TUIkitTests/Support/FrameHarness.swift b/Tests/TUIkitTests/Support/FrameHarness.swift index c683fe84..0e582704 100644 --- a/Tests/TUIkitTests/Support/FrameHarness.swift +++ b/Tests/TUIkitTests/Support/FrameHarness.swift @@ -15,6 +15,9 @@ /// The default terminal is 40×24 with system status-bar items hidden, so a /// frame's content height is `24 − headerHeight` and height-gated fixtures /// can distinguish the measurement pass (full 24 rows) from output passes. +/// +/// Tests that need deterministic services (stub image loaders, injected +/// clocks, …) pass their own `TUIContext`; by default a fresh one is created. @MainActor final class FrameHarness { let app: A @@ -23,8 +26,7 @@ final class FrameHarness { private let renderLoop: RenderLoop - init(app: A, width: Int = 40, height: Int = 24) { - let tuiContext = TUIContext() + init(app: A, width: Int = 40, height: Int = 24, tuiContext: TUIContext = TUIContext()) { let terminal = MockTerminal() terminal.size = (width, height) tuiContext.statusBar.showSystemItems = false