From 45fefd47030b0ae10f31a64ca52a9e5f1f31c3bb Mon Sep 17 00:00:00 2001 From: Corbin Crutchley Date: Thu, 17 Sep 2026 16:17:06 -0700 Subject: [PATCH] fix: preserve settings and refresh changed displays --- ScreenSlanger/config.swift | 103 ++++++++++++--- ScreenSlanger/main.swift | 33 ++++- ScreenSlanger/overlay.swift | 8 ++ ScreenSlangerTests/ConfigSelectionTests.swift | 122 ++++++++++++++++++ 4 files changed, 247 insertions(+), 19 deletions(-) diff --git a/ScreenSlanger/config.swift b/ScreenSlanger/config.swift index a2211b3..f9d0d27 100644 --- a/ScreenSlanger/config.swift +++ b/ScreenSlanger/config.swift @@ -44,10 +44,61 @@ let defaultShaderSource: String = """ """ class Config: Codable { - var configVersion: Int = 4 // Bumped for multi-monitor support + static let currentVersion = 4 + static let supportedFrameRates = 1...240 + + var configVersion: Int = Config.currentVersion var shaderPath: String? = nil var active: Bool = false - var targetFPS: Int = 60 + private var storedTargetFPS: Int = 60 + var targetFPS: Int { + get { storedTargetFPS } + set { storedTargetFPS = min(max(newValue, Self.supportedFrameRates.lowerBound), Self.supportedFrameRates.upperBound) } + } + + private var fileURL: URL? + private var fileRequiringRecovery: URL? + private(set) var recoveryBackupURL: URL? + + private enum CodingKeys: String, CodingKey { + case configVersion, shaderPath, active, targetFPS, shaderParameters + case enabledDisplayIDs, displaySelectionIsExplicit + } + + init(fileURL: URL? = nil) { + self.fileURL = fileURL + } + + required init(from decoder: Decoder) throws { + let values = try decoder.container(keyedBy: CodingKeys.self) + let savedVersion = try values.decodeIfPresent(Int.self, forKey: .configVersion) ?? 1 + guard savedVersion <= Self.currentVersion else { + throw DecodingError.dataCorruptedError( + forKey: .configVersion, in: values, + debugDescription: "Configuration was saved by a newer version of ScreenSlanger") + } + + // New fields must use defaults when loading older configurations. Property + // initializers alone are not used by synthesized Codable decoding. + configVersion = Self.currentVersion + shaderPath = try values.decodeIfPresent(String.self, forKey: .shaderPath) + active = try values.decodeIfPresent(Bool.self, forKey: .active) ?? false + targetFPS = try values.decodeIfPresent(Int.self, forKey: .targetFPS) ?? 60 + shaderParameters = try values.decodeIfPresent([String: [String: Float]].self, forKey: .shaderParameters) ?? [:] + enabledDisplayIDs = try values.decodeIfPresent(Set.self, forKey: .enabledDisplayIDs) ?? [] + displaySelectionIsExplicit = try values.decodeIfPresent(Bool.self, forKey: .displaySelectionIsExplicit) + } + + func encode(to encoder: Encoder) throws { + var values = encoder.container(keyedBy: CodingKeys.self) + try values.encode(configVersion, forKey: .configVersion) + try values.encodeIfPresent(shaderPath, forKey: .shaderPath) + try values.encode(active, forKey: .active) + try values.encode(targetFPS, forKey: .targetFPS) + try values.encode(shaderParameters, forKey: .shaderParameters) + try values.encode(enabledDisplayIDs, forKey: .enabledDisplayIDs) + try values.encodeIfPresent(displaySelectionIsExplicit, forKey: .displaySelectionIsExplicit) + } /// Stored parameter values for RetroArch shaders, keyed by shader path then parameter name var shaderParameters: [String: [String: Float]] = [:] @@ -99,42 +150,62 @@ class Config: Codable { .first! let directory = appSupportDir.appendingPathComponent("ScreenSlanger", isDirectory: true) - if !fileManager.fileExists(atPath: directory.path) { - try? fileManager.createDirectory( - at: directory, withIntermediateDirectories: true, attributes: nil) - } - return directory.appendingPathComponent("config.json") } - func save() { + /// Returns false if the existing configuration could not be preserved or the + /// replacement could not be written. Callers should keep unsaved changes dirty. + @discardableResult + func save(fileURL: URL? = nil) -> Bool { do { - let fileURL = Config.getFileURL() + let destination = fileURL ?? self.fileURL ?? Config.getFileURL() let encoder = JSONEncoder() encoder.outputFormatting = .prettyPrinted let data = try encoder.encode(self) - try data.write(to: fileURL) - print("Saved config to \(fileURL)") + let fileManager = FileManager.default + try fileManager.createDirectory( + at: destination.deletingLastPathComponent(), withIntermediateDirectories: true) + + if let original = fileRequiringRecovery, + original.standardizedFileURL == destination.standardizedFileURL { + // A defaults-based session must not destroy an unreadable or future + // configuration. Refuse the save if its original bytes cannot be kept. + let backup = original.deletingPathExtension() + .appendingPathExtension("recovery-\(UUID().uuidString).json") + try fileManager.copyItem(at: original, to: backup) + recoveryBackupURL = backup + fileRequiringRecovery = nil + print("Preserved unreadable config at \(backup)") + } + + // Foundation writes a sibling temporary file and replaces the destination, + // so interrupted writes cannot leave a partially encoded configuration. + try data.write(to: destination, options: .atomic) + print("Saved config to \(destination)") + return true } catch { print("Failed to save config: \(error)") + return false } } - static func load() -> Config { - let fileURL = getFileURL() + static func load(fileURL: URL? = nil) -> Config { + let fileURL = fileURL ?? getFileURL() var config: Config if !FileManager.default.fileExists(atPath: fileURL.path) { print("No config file found at \(fileURL)") - config = Config() + config = Config(fileURL: fileURL) } else { do { let data = try Data(contentsOf: fileURL) let decoder = JSONDecoder() config = try decoder.decode(Config.self, from: data) + config.fileURL = fileURL print("Loaded config from \(fileURL)") } catch { - print("Failed to load config, creating new: \(error)") - config = Config() + print("Failed to load config; the original will be preserved before saving: \(error)") + config = Config(fileURL: fileURL) + config.fileRequiringRecovery = fileURL } } diff --git a/ScreenSlanger/main.swift b/ScreenSlanger/main.swift index e39bb9d..f59710c 100644 --- a/ScreenSlanger/main.swift +++ b/ScreenSlanger/main.swift @@ -10,6 +10,8 @@ class AppDelegate: NSObject, NSApplicationDelegate { private var overlayControllers: [CGDirectDisplayID: OverlayController] = [:] private var statusItem: NSStatusItem! private var configWindowController: ConfigWindowController? + private var configTimer: Timer? + private var metricsTimer: Timer? func applicationDidFinishLaunching(_ notification: Notification) { // ScreenCaptureKit requests permission when an effect is activated. Keep @@ -19,11 +21,19 @@ class AppDelegate: NSObject, NSApplicationDelegate { timeInterval: 1.0, target: self, selector: #selector(saveConfigIfNeeded), userInfo: nil, repeats: true) RunLoop.current.add(configTimer, forMode: .common) + self.configTimer = configTimer let metricsTimer = Timer.scheduledTimer( timeInterval: 10.0, target: self, selector: #selector(updateMetrics), userInfo: nil, repeats: true) RunLoop.current.add(metricsTimer, forMode: .common) + self.metricsTimer = metricsTimer + + NotificationCenter.default.addObserver( + self, selector: #selector(displaysChanged), + name: NSApplication.didChangeScreenParametersNotification, object: nil) + NSWorkspace.shared.notificationCenter.addObserver( + self, selector: #selector(displaysChanged), name: NSWorkspace.didWakeNotification, object: nil) setupMenuBar() createMenuBarIcon() @@ -35,12 +45,28 @@ class AppDelegate: NSObject, NSApplicationDelegate { } @objc private func saveConfigIfNeeded() { - if self.configChanged { - self.config.save() + if self.configChanged && self.config.save() { self.configChanged = false } } + func applicationWillTerminate(_ notification: Notification) { + configTimer?.invalidate() + metricsTimer?.invalidate() + saveConfigIfNeeded() + for controller in overlayControllers.values { controller.cleanup() } + NotificationCenter.default.removeObserver(self) + NSWorkspace.shared.notificationCenter.removeObserver(self) + } + + @objc private func displaysChanged(_ notification: Notification) { + if notification.name == NSWorkspace.didWakeNotification { + for controller in overlayControllers.values { controller.cleanup() } + overlayControllers.removeAll() + } + refreshConfig() + } + @objc private func updateMetrics() { self.metrics.updateStats() self.metrics.printStats() @@ -68,7 +94,8 @@ class AppDelegate: NSObject, NSApplicationDelegate { // Remove controllers for displays that are no longer enabled let existingIDs = Set(overlayControllers.keys) for displayID in existingIDs { - if !enabledDisplayIDs.contains(displayID) { + if !enabledDisplayIDs.contains(displayID) + || screensByID[displayID].map({ !overlayControllers[displayID]!.matchesDisplay($0) }) == true { // Explicitly cleanup before removing overlayControllers[displayID]?.cleanup() overlayControllers.removeValue(forKey: displayID) diff --git a/ScreenSlanger/overlay.swift b/ScreenSlanger/overlay.swift index dfbffdd..079d83f 100644 --- a/ScreenSlanger/overlay.swift +++ b/ScreenSlanger/overlay.swift @@ -10,12 +10,16 @@ class OverlayController: NSObject, @MainActor MTKViewDelegate { private var errorMessage: ErrorMessage private var window: NSWindow! private var screen: NSScreen + private let initialFrame: NSRect + private let initialScale: CGFloat private var screenCapture: ScreenCapture! private var renderer: MetalRenderer! private let pendingFrame = PendingCaptureFrame() private var isCleanedUp = false init(config: Config, metrics: Metrics, errorMessage: ErrorMessage, screen: NSScreen) { + self.initialFrame = screen.frame + self.initialScale = screen.backingScaleFactor self.config = config self.metrics = metrics self.errorMessage = errorMessage @@ -159,6 +163,10 @@ class OverlayController: NSObject, @MainActor MTKViewDelegate { func getScreen() -> NSScreen { return self.screen } + + func matchesDisplay(_ screen: NSScreen) -> Bool { + initialFrame == screen.frame && initialScale == screen.backingScaleFactor + } } /// Transfers the latest read-only capture surface from ScreenCaptureKit to the UI. diff --git a/ScreenSlangerTests/ConfigSelectionTests.swift b/ScreenSlangerTests/ConfigSelectionTests.swift index 698515c..59652b7 100644 --- a/ScreenSlangerTests/ConfigSelectionTests.swift +++ b/ScreenSlangerTests/ConfigSelectionTests.swift @@ -87,6 +87,128 @@ struct ConfigSelectionTests { #expect(!config.isDisplayEnabled(2)) } + @Test("Older configurations fill missing preferences with defaults") + func missingLegacyKeysReceiveDefaults() throws { + let data = Data(#"{"shaderPath":"/tmp/legacy.slang","active":true}"#.utf8) + let config = try JSONDecoder().decode(Config.self, from: data) + + #expect(config.configVersion == Config.currentVersion) + #expect(config.shaderPath == "/tmp/legacy.slang") + #expect(config.active) + #expect(config.targetFPS == 60) + #expect(config.shaderParameters.isEmpty) + #expect(config.isDisplayEnabled(1)) + #expect(config.displaySelectionIsExplicit == nil) + } + + @Test("Frame rates are bounded on decoding and assignment", arguments: [Int.min, -1, 0, 1, 60, 240, 241, Int.max]) + func frameRatesAreValidated(requested: Int) throws { + let data = try JSONSerialization.data(withJSONObject: ["targetFPS": requested]) + let decoded = try JSONDecoder().decode(Config.self, from: data) + let expected = min(max(requested, 1), 240) + #expect(decoded.targetFPS == expected) + + let assigned = Config() + assigned.targetFPS = requested + #expect(assigned.targetFPS == expected) + } + + @Test("Saving and reloading uses the injected URL and persists preferences") + func filePersistenceRoundTrip() throws { + let directory = temporaryDirectory() + defer { try? FileManager.default.removeItem(at: directory) } + let fileURL = directory.appendingPathComponent("nested/config.json") + let config = Config(fileURL: fileURL) + config.shaderPath = "/tmp/example.slangp" + config.active = true + config.targetFPS = 120 + config.setParameterValue(name: "GAIN", value: 0.75) + config.toggleDisplay(1, availableDisplayIDs: [1]) + + #expect(config.save()) + let loaded = Config.load(fileURL: fileURL) + #expect(loaded.shaderPath == config.shaderPath) + #expect(loaded.active) + #expect(loaded.targetFPS == 120) + #expect(loaded.getParameterValue(name: "GAIN") == 0.75) + #expect(!loaded.isDisplayEnabled(1)) + + loaded.active = false + #expect(loaded.save()) + #expect(!Config.load(fileURL: fileURL).active) + } + + @Test("A missing file loads defaults without creating directories") + func loadingMissingFileDoesNotWrite() { + let directory = temporaryDirectory() + let loaded = Config.load(fileURL: directory.appendingPathComponent("config.json")) + + #expect(loaded.targetFPS == 60) + #expect(!loaded.active) + #expect(!FileManager.default.fileExists(atPath: directory.path)) + } + + @Test("An encoding failure preserves the last saved configuration") + func failedSavePreservesPreviousBytes() throws { + let directory = temporaryDirectory() + defer { try? FileManager.default.removeItem(at: directory) } + let fileURL = directory.appendingPathComponent("config.json") + let config = Config(fileURL: fileURL) + config.shaderPath = "/tmp/example.slangp" + #expect(config.save()) + let original = try Data(contentsOf: fileURL) + + config.setParameterValue(name: "GAIN", value: .nan) + #expect(!config.save()) + #expect(try Data(contentsOf: fileURL) == original) + } + + @Test("An unreadable configuration is preserved before defaults replace it") + func malformedFileReceivesRecoveryCopy() throws { + let directory = temporaryDirectory() + defer { try? FileManager.default.removeItem(at: directory) } + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + let fileURL = directory.appendingPathComponent("config.json") + let original = Data(#"{"shaderPath": "incomplete""#.utf8) + try original.write(to: fileURL) + + let loaded = Config.load(fileURL: fileURL) + #expect(!loaded.active) + #expect(try Data(contentsOf: fileURL) == original) + #expect(loaded.save()) + let backup = try #require(loaded.recoveryBackupURL) + #expect(try Data(contentsOf: backup) == original) + #expect(try JSONDecoder().decode(Config.self, from: Data(contentsOf: fileURL)).targetFPS == 60) + + #expect(loaded.save()) + #expect(loaded.recoveryBackupURL == backup) + } + + @Test("Saving reports filesystem failures") + func savingToInvalidDirectoryFails() throws { + let directory = temporaryDirectory() + defer { try? FileManager.default.removeItem(at: directory) } + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + let parentFile = directory.appendingPathComponent("regular-file") + try Data("keep me".utf8).write(to: parentFile) + let config = Config(fileURL: parentFile.appendingPathComponent("config.json")) + + #expect(!config.save()) + #expect(try String(contentsOf: parentFile, encoding: .utf8) == "keep me") + } + + @Test("A newer configuration is not interpreted with older defaults") + func futureConfigurationIsRejected() throws { + let data = try JSONSerialization.data(withJSONObject: ["configVersion": Config.currentVersion + 1]) + #expect(throws: DecodingError.self) { + try JSONDecoder().decode(Config.self, from: data) + } + } + + private func temporaryDirectory() -> URL { + FileManager.default.temporaryDirectory.appendingPathComponent("ScreenSlangerConfigTests-\(UUID().uuidString)", isDirectory: true) + } + private func legacyConfig(displayIDs: [UInt32]) throws -> Config { // Match the version 4 format, before displaySelectionIsExplicit existed. let data = try JSONSerialization.data(withJSONObject: [ -- 2.51.2