From a9a8ecf71521659984087c18e64f948478761d3e Mon Sep 17 00:00:00 2001 From: Shay Prasad Date: Sat, 12 Sep 2026 14:40:38 +0100 Subject: [PATCH 1/2] Fix display resource ownership and brightness/build reliability --- .github/workflows/ci.yml | 3 + .../Extensions/NSScreen+Extension.swift | 3 +- MonitorControl/Model/Display.swift | 2 - MonitorControl/Support/Arm64DDC.swift | 23 ++-- MonitorControl/Support/DisplayManager.swift | 3 +- MonitorControl/Support/IntelDDC.swift | 15 ++- MonitorControlTests/SettingsPanesTests.swift | 17 +++ MonitorControlTests/XDRBrightnessTests.swift | 12 ++ build/build.sh | 17 ++- build/test-build.sh | 106 ++++++++++++++++++ 10 files changed, 179 insertions(+), 22 deletions(-) create mode 100755 build/test-build.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1fbf010..b80bd16 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,6 +67,9 @@ jobs: PROVISIONING_PROFILE_SPECIFIER= \ 2>&1 | tee build/xcodebuild-test.log + - name: Validate build script failure handling + run: build/test-build.sh + # actions/upload-artifact v4.6.2 - name: Upload test log if: failure() diff --git a/MonitorControl/Extensions/NSScreen+Extension.swift b/MonitorControl/Extensions/NSScreen+Extension.swift index b6cd0b7..3002855 100644 --- a/MonitorControl/Extensions/NSScreen+Extension.swift +++ b/MonitorControl/Extensions/NSScreen+Extension.swift @@ -43,10 +43,11 @@ public extension NSScreen { } defer { - assert(IOObjectRelease(servicePortIterator) == KERN_SUCCESS) + _ = IOObjectRelease(servicePortIterator) } while case let object = IOIteratorNext(servicePortIterator), object != 0 { + defer { _ = IOObjectRelease(object) } let dict = (IODisplayCreateInfoDictionary(object, UInt32(kIODisplayOnlyPreferredName)).takeRetainedValue() as NSDictionary as? [String: AnyObject])! if dict[kDisplayVendorID] as? UInt32 == self.vendorNumber, dict[kDisplayProductID] as? UInt32 == self.modelNumber, dict[kDisplaySerialNumber] as? UInt32 == self.serialNumber { diff --git a/MonitorControl/Model/Display.swift b/MonitorControl/Model/Display.swift index 526329e..095132e 100644 --- a/MonitorControl/Model/Display.swift +++ b/MonitorControl/Model/Display.swift @@ -180,7 +180,6 @@ class Display: Equatable { _ = self.setDirectBrightness(self.smoothBrightnessTransient, transient: true) self.smoothBrightnessRunning = false } - self.swBrightnessSemaphore.signal() return true } @@ -243,7 +242,6 @@ class Display: Equatable { DispatchQueue.global(qos: .userInteractive).async { for transientValue in stride(from: currentValue, to: newValue, by: 0.005 * (currentValue > newValue ? -1 : 1)) { guard app.reconfigureID == 0 else { - self.swBrightnessSemaphore.signal() return } if self.isVirtual || self.readPrefAsBool(key: .avoidGamma) { diff --git a/MonitorControl/Support/Arm64DDC.swift b/MonitorControl/Support/Arm64DDC.swift index e8d1495..d4390fa 100644 --- a/MonitorControl/Support/Arm64DDC.swift +++ b/MonitorControl/Support/Arm64DDC.swift @@ -163,23 +163,25 @@ class Arm64DDC: NSObject { return matchScore } - static func ioregIterateToNextObjectOfInterest(interests: [String], iterator: inout io_iterator_t) -> (name: String, entry: io_service_t, preceedingEntry: io_service_t)? { - var entry: io_service_t = IO_OBJECT_NULL - var preceedingEntry: io_service_t = IO_OBJECT_NULL + static func ioregIterateToNextObjectOfInterest(interests: [String], iterator: inout io_iterator_t) -> (name: String, entry: io_service_t)? { let name = UnsafeMutablePointer.allocate(capacity: MemoryLayout.size) defer { name.deallocate() } while true { - preceedingEntry = entry - entry = IOIteratorNext(iterator) - guard IORegistryEntryGetName(entry, name) == KERN_SUCCESS, entry != MACH_PORT_NULL else { + let entry = IOIteratorNext(iterator) + guard entry != IO_OBJECT_NULL else { + break + } + guard IORegistryEntryGetName(entry, name) == KERN_SUCCESS else { + _ = IOObjectRelease(entry) break } let nameString = String(cString: name) for interest in interests where entry != IO_OBJECT_NULL && nameString.contains(interest) { - return (nameString, entry, preceedingEntry) + return (nameString, entry) } + _ = IOObjectRelease(entry) } return nil } @@ -190,8 +192,10 @@ class Arm64DDC: NSObject { ioregService.edidUUID = edidUUID } let cpath = UnsafeMutablePointer.allocate(capacity: MemoryLayout.size) - IORegistryEntryGetPath(entry, kIOServicePlane, cpath) - ioregService.ioDisplayLocation = String(cString: cpath) + defer { cpath.deallocate() } + if IORegistryEntryGetPath(entry, kIOServicePlane, cpath) == KERN_SUCCESS { + ioregService.ioDisplayLocation = String(cString: cpath) + } if let unmanagedDisplayAttrs = IORegistryEntryCreateCFProperty(entry, "DisplayAttributes" as CFString, kCFAllocatorDefault, IOOptionBits(kIORegistryIterateRecursively)), let displayAttrs = unmanagedDisplayAttrs.takeRetainedValue() as? NSDictionary { ioregService.displayAttributes = displayAttrs if let productAttrs = displayAttrs.value(forKey: "ProductAttributes") as? NSDictionary { @@ -250,6 +254,7 @@ class Arm64DDC: NSObject { guard let objectOfInterest = ioregIterateToNextObjectOfInterest(interests: [keyDCPAVServiceProxy] + keysFramebuffer, iterator: &iterator) else { break } + defer { _ = IOObjectRelease(objectOfInterest.entry) } if keysFramebuffer.contains(objectOfInterest.name) { ioregService = self.getIORegServiceAppleCDC2Properties(entry: objectOfInterest.entry) serviceLocation += 1 diff --git a/MonitorControl/Support/DisplayManager.swift b/MonitorControl/Support/DisplayManager.swift index cf61e49..c92b24f 100644 --- a/MonitorControl/Support/DisplayManager.swift +++ b/MonitorControl/Support/DisplayManager.swift @@ -42,7 +42,6 @@ class DisplayManager { } var shades: [CGDirectDisplayID: NSWindow] = [:] - var shadeGrave: [NSWindow] = [] func isDisqualifiedFromShade(_ displayID: CGDirectDisplayID) -> Bool { if CGDisplayIsInHWMirrorSet(displayID) != 0 || CGDisplayIsInMirrorSet(displayID) != 0 { @@ -65,6 +64,7 @@ class DisplayManager { func createShadeOnDisplay(displayID: CGDirectDisplayID) -> NSWindow? { if let screen = DisplayManager.getByDisplayID(displayID: displayID) { let shade = NSWindow(contentRect: .init(origin: NSPoint(x: 0, y: 0), size: .init(width: 10, height: 1)), styleMask: [], backing: .buffered, defer: false) + shade.isReleasedWhenClosed = false shade.title = "LumaControl Window Shade for Display " + String(displayID) shade.isMovableByWindowBackground = false shade.backgroundColor = .clear @@ -117,7 +117,6 @@ class DisplayManager { func destroyShade(displayID: CGDirectDisplayID) -> Bool { if let shade = shades[displayID] { os_log("Destroying shade for display %{public}@", type: .info, String(displayID)) - self.shadeGrave.append(shade) self.shades.removeValue(forKey: displayID) shade.close() return true diff --git a/MonitorControl/Support/IntelDDC.swift b/MonitorControl/Support/IntelDDC.swift index ad8ead2..9e9bc5e 100644 --- a/MonitorControl/Support/IntelDDC.swift +++ b/MonitorControl/Support/IntelDDC.swift @@ -12,7 +12,7 @@ public class IntelDDC { var enabled: Bool = false deinit { - assert(IOObjectRelease(self.framebuffer) == KERN_SUCCESS) + _ = IOObjectRelease(self.framebuffer) } public init?(for displayId: CGDirectDisplayID, withReplyTransactionType replyTransactionType: IOOptionBits? = nil) { @@ -27,6 +27,7 @@ public class IntelDDC { self.replyTransactionType = replyTransactionType } else { os_log("No supported reply transaction type found for display with ID %u.", type: .error, displayId) + _ = IOObjectRelease(framebuffer) return nil } } @@ -126,9 +127,10 @@ public class IntelDDC { return nil } defer { - assert(IOObjectRelease(ioIterator) == KERN_SUCCESS) + _ = IOObjectRelease(ioIterator) } while case let ioService = IOIteratorNext(ioIterator), ioService != 0 { + defer { _ = IOObjectRelease(ioService) } var serviceProperties: Unmanaged? guard IORegistryEntryCreateCFProperties(ioService, &serviceProperties, kCFAllocatorDefault, IOOptionBits()) == KERN_SUCCESS, serviceProperties != nil else { continue @@ -164,6 +166,7 @@ public class IntelDDC { continue } var connect: IOI2CConnectRef? + defer { _ = IOObjectRelease(interface) } guard IOI2CInterfaceOpen(interface, IOOptionBits(), &connect) == KERN_SUCCESS else { os_log("Failed to connect to interface %u for framebuffer with ID %u.", type: .error, bus, framebuffer) continue @@ -190,9 +193,13 @@ public class IntelDDC { return nil } defer { - assert(IOObjectRelease(portIterator) == KERN_SUCCESS) + _ = IOObjectRelease(portIterator) } while case let port = IOIteratorNext(portIterator), port != 0 { + var transfersPortOwnership = false + defer { + if !transfersPortOwnership { _ = IOObjectRelease(port) } + } let dict = IODisplayCreateInfoDictionary(port, IOOptionBits(kIODisplayOnlyPreferredName)).takeRetainedValue() as NSDictionary let valueForKey = { (k: String) in (dict[k] as? CFIndex).flatMap { Int32(exactly: $0) }.flatMap { UInt32(bitPattern: $0) } ?? 0 @@ -231,6 +238,7 @@ public class IntelDDC { os_log("Vendor ID: %u, Product ID: %u, Serial Number: %u", type: .info, portVendorId, portProductId, portSerialNumber) os_log("Unit Number: %u", type: .info, CGDisplayUnitNumber(displayId)) os_log("Service Port: %u", type: .info, port) + transfersPortOwnership = true return port } os_log("No service port found for display with ID %u.", type: .error, displayId) @@ -253,6 +261,7 @@ public class IntelDDC { var busCount: IOItemCount = 0 guard IOFBGetI2CInterfaceCount(servicePort, &busCount) == KERN_SUCCESS, busCount >= 1 else { os_log("No framebuffer port found for display with ID %u.", type: .error, displayId) + _ = IOObjectRelease(servicePort) return nil } return servicePort diff --git a/MonitorControlTests/SettingsPanesTests.swift b/MonitorControlTests/SettingsPanesTests.swift index 0247980..40ae2ef 100644 --- a/MonitorControlTests/SettingsPanesTests.swift +++ b/MonitorControlTests/SettingsPanesTests.swift @@ -27,6 +27,23 @@ import XCTest // These tests load the same scenes the same way `main.swift` does, so they fail loudly the // moment the two names drift apart again. They need no display and touch no hardware. final class SettingsPanesTests: XCTestCase { + func testDestroyedShadeDoesNotRetainClosedWindow() { + let displayID = CGDirectDisplayID.max + weak var weakShade: NSWindow? + + autoreleasepool { + let shade = NSWindow(contentRect: .zero, styleMask: [], backing: .buffered, defer: true) + shade.isReleasedWhenClosed = false + weakShade = shade + DisplayManager.shared.shades[displayID] = shade + + XCTAssertTrue(DisplayManager.shared.destroyShade(displayID: displayID)) + XCTAssertNil(DisplayManager.shared.shades[displayID]) + } + + XCTAssertNil(weakShade, "Destroying a shade must release the closed window after removing it from the manager.") + } + func testEverySettingsPaneLoadsFromTheStoryboard() { let storyboard = NSStoryboard(name: "Main", bundle: Bundle.main) diff --git a/MonitorControlTests/XDRBrightnessTests.swift b/MonitorControlTests/XDRBrightnessTests.swift index 0c4f55d..56badef 100644 --- a/MonitorControlTests/XDRBrightnessTests.swift +++ b/MonitorControlTests/XDRBrightnessTests.swift @@ -36,6 +36,7 @@ final class XDRBrightnessTests: XCTestCase { display?.removePref(key: .xdrMaxBrightness) display?.removePref(key: .xdrWarningAcknowledged) display?.removePref(key: .value, for: .brightness) + display?.removePref(key: .SwBrightness) } self.appleDisplay = nil self.stubbedDisplay = nil @@ -119,4 +120,15 @@ final class XDRBrightnessTests: XCTestCase { let roundTrip = self.appleDisplay.swBrightnessTransform(value: transformed, reverse: true) XCTAssertEqual(roundTrip, value, accuracy: 0.0001) } + + func testSmoothBrightnessDoesNotAddSemaphorePermits() { + self.otherDisplay.smoothBrightnessTransient = 0.5 + self.otherDisplay.savePref(Float(0.5), for: .brightness) + + XCTAssertTrue(self.otherDisplay.setSmoothBrightness()) + + XCTAssertEqual(self.otherDisplay.swBrightnessSemaphore.wait(timeout: .now()), .success) + defer { self.otherDisplay.swBrightnessSemaphore.signal() } + XCTAssertEqual(self.otherDisplay.swBrightnessSemaphore.wait(timeout: .now()), .timedOut) + } } diff --git a/build/build.sh b/build/build.sh index d8ebbb9..48582c8 100755 --- a/build/build.sh +++ b/build/build.sh @@ -18,6 +18,7 @@ export COPYFILE_DISABLE=1 export COPY_EXTENDED_ATTRIBUTES_DISABLE=1 BUILT_APP="$DERIVED_DATA_DIR/Build/Products/Release/$APP_NAME.app" +CODESIGN_BIN="${CODESIGN_BIN:-/usr/bin/codesign}" APP_ENTITLEMENTS="$DERIVED_DATA_DIR/Build/Intermediates.noindex/MonitorControl.build/Release/MonitorControl.build/$APP_NAME.app.xcent" USE_ADHOC_FALLBACK=0 @@ -67,6 +68,11 @@ if [ "$BUILD_STATUS" -ne 0 ]; then set -e fi +if [ "$BUILD_STATUS" -ne 0 ]; then + echo "Error: xcodebuild failed with status $BUILD_STATUS" + exit "$BUILD_STATUS" +fi + if [ -d "$BUILT_APP" ]; then echo "Copying $APP_NAME.app to build/ ..." rm -rf "$OUTPUT_DIR/$APP_NAME.app" @@ -78,13 +84,14 @@ if [ -d "$BUILT_APP" ]; then sanitize_bundle_metadata "$OUTPUT_DIR/$APP_NAME.app" if [ "$USE_ADHOC_FALLBACK" -eq 1 ] && [ -f "$APP_ENTITLEMENTS" ]; then echo "Finalizing ad-hoc app signature..." - /usr/bin/codesign --force --deep --sign - -o runtime --entitlements "$APP_ENTITLEMENTS" --timestamp=none --generate-entitlement-der "$OUTPUT_DIR/$APP_NAME.app" - if [ "$BUILD_STATUS" -ne 0 ]; then - echo "Recovered build output after Xcode codesign failure." - fi + "$CODESIGN_BIN" --force --deep --sign - -o runtime --entitlements "$APP_ENTITLEMENTS" --timestamp=none --generate-entitlement-der "$OUTPUT_DIR/$APP_NAME.app" + fi + if ! "$CODESIGN_BIN" --verify --deep --strict "$OUTPUT_DIR/$APP_NAME.app"; then + echo "Error: final app signature verification failed" + exit 1 fi echo "Done: $OUTPUT_DIR/$APP_NAME.app" else echo "Error: built app not found at $BUILT_APP" - exit "${BUILD_STATUS:-1}" + exit 1 fi diff --git a/build/test-build.sh b/build/test-build.sh new file mode 100755 index 0000000..f5a8722 --- /dev/null +++ b/build/test-build.sh @@ -0,0 +1,106 @@ +#!/bin/bash +# Regression tests for build.sh's artifact acceptance and signing gates. +set -euo pipefail + +SOURCE_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +ROOT_DIR="$(dirname "$SOURCE_DIR")" +HARNESS_DIR="$(mktemp -d "${TMPDIR:-/tmp}/lumacontrol-build-test.XXXXXX")" +trap 'rm -rf "$HARNESS_DIR"' EXIT +mkdir -p "$HARNESS_DIR/project/build" "$HARNESS_DIR/bin" +cp "$ROOT_DIR/build/build.sh" "$HARNESS_DIR/project/build/build.sh" +chmod +x "$HARNESS_DIR/project/build/build.sh" +touch "$HARNESS_DIR/project/MonitorControl.xcodeproj" + +cat > "$HARNESS_DIR/bin/security" <<'STUB' +#!/bin/bash +if [ "${BUILD_TEST_IDENTITY:-}" = valid ]; then + echo " 1) ABCDEF \"Test Identity\"" + echo " 1 valid identities found" +fi +STUB +cat > "$HARNESS_DIR/bin/xcodebuild" <<'STUB' +#!/bin/bash +set -u +derived="" +args=("$@") +for ((i=0; i<${#args[@]}; i++)); do + if [ "${args[$i]}" = -derivedDataPath ]; then derived="${args[$((i+1))]}"; fi +done +case "${BUILD_TEST_SCENARIO:-}" in + partial) + mkdir -p "$derived/Build/Products/Release/LumaControl.app" + exit 7 + ;; + fallback) + call_file="${BUILD_TEST_CALL_FILE:?}" + call=0 + if [ -f "$call_file" ]; then call=$(cat "$call_file"); fi + call=$((call + 1)) + printf "%s" "$call" > "$call_file" + if [ "$call" -eq 1 ]; then + mkdir -p "$derived/Build/Products/Release/LumaControl.app" + exit 9 + fi + mkdir -p "$derived/Build/Products/Release/LumaControl.app" + exit 0 + ;; + success|signfail) + mkdir -p "$derived/Build/Products/Release/LumaControl.app" + exit 0 + ;; + missing) + exit 0 + ;; + *) exit 99 ;; +esac +STUB +cat > "$HARNESS_DIR/bin/ditto" <<'STUB' +#!/bin/bash +if [ "$1" = --norsrc ]; then shift; fi +cp -R "$1" "$2" +STUB +cat > "$HARNESS_DIR/bin/xattr" <<'STUB' +#!/bin/bash +exit 0 +STUB +cat > "$HARNESS_DIR/bin/codesign" <<'STUB' +#!/bin/bash +if [ "${BUILD_TEST_SCENARIO:-}" = signfail ] && [ "${1:-}" = --verify ]; then exit 1; fi +exit 0 +STUB +chmod +x "$HARNESS_DIR/bin"/* + +run_case() { + local scenario="$1" expected="$2" identity="${3:-}" status + rm -rf "$HARNESS_DIR/project/build/DerivedData" "$HARNESS_DIR/project/build/LumaControl.app" + if BUILD_TEST_SCENARIO="$scenario" BUILD_TEST_IDENTITY="$identity" BUILD_TEST_CALL_FILE="$HARNESS_DIR/$scenario.calls" PATH="$HARNESS_DIR/bin:$PATH" CODESIGN_BIN="$HARNESS_DIR/bin/codesign" \ + "$HARNESS_DIR/project/build/build.sh" >"$HARNESS_DIR/$scenario.log" 2>&1; then + status=0 + else + status=$? + fi + if [ "$status" -ne "$expected" ]; then + echo "FAIL $scenario: expected exit $expected, got $status" >&2 + cat "$HARNESS_DIR/$scenario.log" >&2 + exit 1 + fi + if [ "$scenario" = partial ] && [ -e "$HARNESS_DIR/project/build/LumaControl.app" ]; then + echo "FAIL $scenario: copied an app after failure" >&2 + exit 1 + fi + if [ "$expected" -eq 0 ] && ! grep -q 'Done: ' "$HARNESS_DIR/$scenario.log"; then + echo "FAIL $scenario: missing success marker" >&2 + exit 1 + fi + if [ "$expected" -ne 0 ] && grep -q 'Done: ' "$HARNESS_DIR/$scenario.log"; then + echo "FAIL $scenario: reported success" >&2 + exit 1 + fi + echo "PASS $scenario (exit $status)" +} + +run_case partial 7 +run_case success 0 valid +run_case fallback 0 valid +run_case missing 1 +run_case signfail 1 valid From c6a41c0746f4b15f577f635a7a53769647670388 Mon Sep 17 00:00:00 2001 From: Shay Prasad Date: Sat, 12 Sep 2026 15:22:06 +0100 Subject: [PATCH 2/2] Fix stale brightness ramps and Intel DDC value handling --- MonitorControl.xcodeproj/project.pbxproj | 4 + MonitorControl/Model/Display.swift | 84 ++++++----- MonitorControl/Support/IntelDDC.swift | 65 +++++---- MonitorControl/Support/XDREngine.swift | 4 + MonitorControlTests/IntelDDCTests.swift | 21 +++ MonitorControlTests/XDRBrightnessTests.swift | 138 +++++++++++++++++++ 6 files changed, 258 insertions(+), 58 deletions(-) create mode 100644 MonitorControlTests/IntelDDCTests.swift diff --git a/MonitorControl.xcodeproj/project.pbxproj b/MonitorControl.xcodeproj/project.pbxproj index eedeab5..4f4a863 100644 --- a/MonitorControl.xcodeproj/project.pbxproj +++ b/MonitorControl.xcodeproj/project.pbxproj @@ -61,6 +61,7 @@ FE4E0896249D584C003A50BB /* OSDUtils.swift in Sources */ = {isa = PBXBuildFile; fileRef = FE4E0895249D584C003A50BB /* OSDUtils.swift */; }; E77C0DE000000000000000A3 /* XDRBrightnessTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E77C0DE000000000000000A1 /* XDRBrightnessTests.swift */; }; E77C0DE000000000000000B3 /* SettingsPanesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E77C0DE000000000000000B1 /* SettingsPanesTests.swift */; }; + E77C0DE000000000000000C3 /* IntelDDCTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E77C0DE000000000000000C1 /* IntelDDCTests.swift */; }; /* End PBXBuildFile section */ /* Begin PBXCopyFilesBuildPhase section */ @@ -187,6 +188,7 @@ FE4E0895249D584C003A50BB /* OSDUtils.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSDUtils.swift; sourceTree = ""; }; E77C0DE000000000000000A1 /* XDRBrightnessTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XDRBrightnessTests.swift; sourceTree = ""; }; E77C0DE000000000000000B1 /* SettingsPanesTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsPanesTests.swift; sourceTree = ""; }; + E77C0DE000000000000000C1 /* IntelDDCTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IntelDDCTests.swift; sourceTree = ""; }; E77C0DE000000000000000A2 /* MonitorControlTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; name = MonitorControlTests.xctest; path = MonitorControlTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; /* End PBXFileReference section */ @@ -393,6 +395,7 @@ children = ( E77C0DE000000000000000A1 /* XDRBrightnessTests.swift */, E77C0DE000000000000000B1 /* SettingsPanesTests.swift */, + E77C0DE000000000000000C1 /* IntelDDCTests.swift */, ); path = MonitorControlTests; sourceTree = ""; @@ -749,6 +752,7 @@ files = ( E77C0DE000000000000000A3 /* XDRBrightnessTests.swift in Sources */, E77C0DE000000000000000B3 /* SettingsPanesTests.swift in Sources */, + E77C0DE000000000000000C3 /* IntelDDCTests.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/MonitorControl/Model/Display.swift b/MonitorControl/Model/Display.swift index 095132e..9d441c9 100644 --- a/MonitorControl/Model/Display.swift +++ b/MonitorControl/Model/Display.swift @@ -15,6 +15,7 @@ class Display: Equatable { var smoothBrightnessRunning: Bool = false var smoothBrightnessSlow: Bool = false let swBrightnessSemaphore = DispatchSemaphore(value: 1) + private var swBrightnessGeneration: UInt64 = 0 static func == (lhs: Display, rhs: Display) -> Bool { lhs.identifier == rhs.identifier @@ -226,49 +227,64 @@ class Display: Equatable { func setSwBrightness(_ value: Float, smooth: Bool = false, noPrefSave: Bool = false) -> Bool { self.swBrightnessSemaphore.wait() + self.swBrightnessGeneration &+= 1 + let generation = self.swBrightnessGeneration let brightnessValue = min(1, value) - var currentValue = self.readPrefAsFloat(key: .SwBrightness) + let currentValue = self.swBrightnessTransform(value: self.readPrefAsFloat(key: .SwBrightness)) if !noPrefSave { self.savePref(brightnessValue, key: .SwBrightness) } - guard !self.isDummy else { - self.swBrightnessSemaphore.signal() - return true - } - var newValue = brightnessValue - currentValue = self.swBrightnessTransform(value: currentValue) - newValue = self.swBrightnessTransform(value: newValue) + let newValue = self.swBrightnessTransform(value: brightnessValue) if smooth { - DispatchQueue.global(qos: .userInteractive).async { - for transientValue in stride(from: currentValue, to: newValue, by: 0.005 * (currentValue > newValue ? -1 : 1)) { - guard app.reconfigureID == 0 else { - return - } - if self.isVirtual || self.readPrefAsBool(key: .avoidGamma) { - _ = DisplayManager.shared.setShadeAlpha(value: 1 - transientValue, displayID: DisplayManager.resolveEffectiveDisplayID(self.identifier)) - } else { - let gammaTableRed = self.defaultGammaTableRed.map { $0 * transientValue } - let gammaTableGreen = self.defaultGammaTableGreen.map { $0 * transientValue } - let gammaTableBlue = self.defaultGammaTableBlue.map { $0 * transientValue } - CGSetDisplayTransferByTable(self.identifier, self.defaultGammaTableSampleCount, gammaTableRed, gammaTableGreen, gammaTableBlue) - } - Thread.sleep(forTimeInterval: 0.001) // Let's make things quick if not performed in the background - } + self.swBrightnessSemaphore.signal() + DispatchQueue.main.asyncAfter(deadline: .now() + 0.001) { + self.runSmoothSwBrightnessStep(currentValue: currentValue, targetValue: newValue, generation: generation) } + return true } else { - if self.isVirtual || self.readPrefAsBool(key: .avoidGamma) { - self.swBrightnessSemaphore.signal() - return DisplayManager.shared.setShadeAlpha(value: 1 - newValue, displayID: DisplayManager.resolveEffectiveDisplayID(self.identifier)) - } else { - let gammaTableRed = self.defaultGammaTableRed.map { $0 * newValue } - let gammaTableGreen = self.defaultGammaTableGreen.map { $0 * newValue } - let gammaTableBlue = self.defaultGammaTableBlue.map { $0 * newValue } - DisplayManager.shared.moveGammaActivityEnforcer(displayID: self.identifier) - CGSetDisplayTransferByTable(self.identifier, self.defaultGammaTableSampleCount, gammaTableRed, gammaTableGreen, gammaTableBlue) - DisplayManager.shared.enforceGammaActivity() - } + let result = self.applySwBrightnessValue(newValue) + self.swBrightnessSemaphore.signal() + return result } + } + + private func runSmoothSwBrightnessStep(currentValue: Float, targetValue: Float, generation: UInt64) { + self.swBrightnessSemaphore.wait() + guard generation == self.swBrightnessGeneration, app.sleepID == 0, app.reconfigureID == 0 else { + self.swBrightnessSemaphore.signal() + return + } + let difference = targetValue - currentValue + let nextValue = abs(difference) <= 0.005 ? targetValue : currentValue + (difference > 0 ? 0.005 : -0.005) + let result = self.applySwBrightnessValue(nextValue, enforceGammaActivity: false) self.swBrightnessSemaphore.signal() + guard result else { + return + } + if nextValue != targetValue { + DispatchQueue.main.asyncAfter(deadline: .now() + 0.001) { + self.runSmoothSwBrightnessStep(currentValue: nextValue, targetValue: targetValue, generation: generation) + } + } + } + + func applySwBrightnessValue(_ value: Float, enforceGammaActivity: Bool = true) -> Bool { + guard !self.isDummy else { + return true + } + if self.isVirtual || self.readPrefAsBool(key: .avoidGamma) { + return DisplayManager.shared.setShadeAlpha(value: 1 - value, displayID: DisplayManager.resolveEffectiveDisplayID(self.identifier)) + } + let gammaTableRed = self.defaultGammaTableRed.map { $0 * value } + let gammaTableGreen = self.defaultGammaTableGreen.map { $0 * value } + let gammaTableBlue = self.defaultGammaTableBlue.map { $0 * value } + if enforceGammaActivity { + DisplayManager.shared.moveGammaActivityEnforcer(displayID: self.identifier) + } + CGSetDisplayTransferByTable(self.identifier, self.defaultGammaTableSampleCount, gammaTableRed, gammaTableGreen, gammaTableBlue) + if enforceGammaActivity { + DisplayManager.shared.enforceGammaActivity() + } return true } diff --git a/MonitorControl/Support/IntelDDC.swift b/MonitorControl/Support/IntelDDC.swift index 9e9bc5e..93adc91 100644 --- a/MonitorControl/Support/IntelDDC.swift +++ b/MonitorControl/Support/IntelDDC.swift @@ -11,6 +11,10 @@ public class IntelDDC { let replyTransactionType: IOOptionBits var enabled: Bool = false + static func decodeDDCWord(high: UInt8, low: UInt8) -> UInt16 { + (UInt16(high) << 8) | UInt16(low) + } + deinit { _ = IOObjectRelease(self.framebuffer) } @@ -43,18 +47,23 @@ public class IntelDDC { data[4] = UInt8(value >> 8) data[5] = UInt8(value & 255) data[6] = 0x6E ^ data[0] ^ data[1] ^ data[2] ^ data[3] ^ data[4] ^ data[5] + let dataCount = UInt32(data.count) for _ in 1 ... numofWriteCycles { usleep(writeSleepTime) - var request = IOI2CRequest() - request.commFlags = 0 - request.sendAddress = 0x6E - request.sendTransactionType = IOOptionBits(kIOI2CSimpleTransactionType) - request.sendBuffer = withUnsafePointer(to: &data[0]) { vm_address_t(bitPattern: $0) } - request.sendBytes = UInt32(data.count) - request.replyTransactionType = IOOptionBits(kIOI2CNoTransactionType) - request.replyBytes = 0 - if IntelDDC.send(request: &request, to: self.framebuffer, errorRecoveryWaitTime: errorRecoveryWaitTime) { + let sent = data.withUnsafeMutableBytes { sendBuffer -> Bool in + guard let sendBaseAddress = sendBuffer.baseAddress else { return false } + var request = IOI2CRequest() + request.commFlags = 0 + request.sendAddress = 0x6E + request.sendTransactionType = IOOptionBits(kIOI2CSimpleTransactionType) + request.sendBuffer = vm_address_t(bitPattern: sendBaseAddress) + request.sendBytes = dataCount + request.replyTransactionType = IOOptionBits(kIOI2CNoTransactionType) + request.replyBytes = 0 + return IntelDDC.send(request: &request, to: self.framebuffer, errorRecoveryWaitTime: errorRecoveryWaitTime) + } + if sent { success = true } } @@ -70,24 +79,32 @@ public class IntelDDC { data[2] = 0x01 data[3] = command data[4] = 0x6E ^ data[0] ^ data[1] ^ data[2] ^ data[3] + let dataCount = UInt32(data.count) + let replyDataCount = UInt32(replyData.count) for i in 1 ... tries { usleep(writeSleepTime) usleep(errorRecoveryWaitTime ?? 0) - var request = IOI2CRequest() - request.commFlags = 0 - request.sendAddress = 0x6E - request.sendTransactionType = IOOptionBits(kIOI2CSimpleTransactionType) - request.sendBuffer = withUnsafePointer(to: &data[0]) { vm_address_t(bitPattern: $0) } - request.sendBytes = UInt32(data.count) - request.minReplyDelay = minReplyDelay ?? 10 - request.replyAddress = 0x6F - request.replySubAddress = 0x51 - request.replyTransactionType = self.replyTransactionType - request.replyBytes = UInt32(replyData.count) - request.replyBuffer = withUnsafePointer(to: &replyData[0]) { vm_address_t(bitPattern: $0) } + let sent = data.withUnsafeMutableBytes { sendBuffer -> Bool in + replyData.withUnsafeMutableBytes { replyBuffer -> Bool in + guard let sendBaseAddress = sendBuffer.baseAddress, let replyBaseAddress = replyBuffer.baseAddress else { return false } + var request = IOI2CRequest() + request.commFlags = 0 + request.sendAddress = 0x6E + request.sendTransactionType = IOOptionBits(kIOI2CSimpleTransactionType) + request.sendBuffer = vm_address_t(bitPattern: sendBaseAddress) + request.sendBytes = dataCount + request.minReplyDelay = minReplyDelay ?? 10 + request.replyAddress = 0x6F + request.replySubAddress = 0x51 + request.replyTransactionType = self.replyTransactionType + request.replyBytes = replyDataCount + request.replyBuffer = vm_address_t(bitPattern: replyBaseAddress) + return IntelDDC.send(request: &request, to: self.framebuffer, errorRecoveryWaitTime: errorRecoveryWaitTime) + } + } - if IntelDDC.send(request: &request, to: self.framebuffer, errorRecoveryWaitTime: errorRecoveryWaitTime) { + if sent { if replyData.count > 0 { let checksum = replyData.last! var calculated = UInt8(0x50) @@ -113,8 +130,8 @@ public class IntelDDC { os_log("Reading %{public}@ took %u tries.", type: .info, String(reflecting: command), i) } let (mh, ml, sh, sl) = (replyData[6], replyData[7], replyData[8], replyData[9]) - let maxValue = UInt16(mh << 8) + UInt16(ml) - let currentValue = UInt16(sh << 8) + UInt16(sl) + let maxValue = Self.decodeDDCWord(high: mh, low: ml) + let currentValue = Self.decodeDDCWord(high: sh, low: sl) return (currentValue, maxValue) } } diff --git a/MonitorControl/Support/XDREngine.swift b/MonitorControl/Support/XDREngine.swift index 1dd25a0..b0189ef 100644 --- a/MonitorControl/Support/XDREngine.swift +++ b/MonitorControl/Support/XDREngine.swift @@ -116,6 +116,10 @@ final class XDREngine { self.requestedBoost = wanted self.activeDisplay = displayID self.start() + guard self.window != nil, self.metalLayer != nil, self.commandQueue != nil else { + self.stop() + return + } self.applyBoost() } diff --git a/MonitorControlTests/IntelDDCTests.swift b/MonitorControlTests/IntelDDCTests.swift new file mode 100644 index 0000000..0c50bfa --- /dev/null +++ b/MonitorControlTests/IntelDDCTests.swift @@ -0,0 +1,21 @@ +// Copyright © MonitorControl. @JoniVR, @theOneyouseek, @waydabber and others + +import XCTest + +@testable import LumaControl + +final class IntelDDCTests: XCTestCase { + func testDecodeDDCWordPreservesBothBytes() { + let cases: [(UInt8, UInt8, UInt16)] = [ + (0x00, 0x00, 0x0000), + (0x00, 0x64, 0x0064), + (0x01, 0x00, 0x0100), + (0x12, 0x34, 0x1234), + (0xFF, 0xFF, 0xFFFF), + ] + + for (high, low, expected) in cases { + XCTAssertEqual(IntelDDC.decodeDDCWord(high: high, low: low), expected) + } + } +} diff --git a/MonitorControlTests/XDRBrightnessTests.swift b/MonitorControlTests/XDRBrightnessTests.swift index 56badef..bfc87d4 100644 --- a/MonitorControlTests/XDRBrightnessTests.swift +++ b/MonitorControlTests/XDRBrightnessTests.swift @@ -14,6 +14,17 @@ private class StubbedBrightnessAppleDisplay: AppleDisplay { } } +private final class RecordingBrightnessDisplay: Display { + var writes: [(value: Float, isMainThread: Bool)] = [] + var onWrite: ((Float) -> Void)? + + override func applySwBrightnessValue(_ value: Float, enforceGammaActivity: Bool) -> Bool { + self.writes.append((value, Thread.isMainThread)) + self.onWrite?(value) + return true + } +} + // Tests for the XDR extended brightness logic. Uses dummy displays so no real // display is ever touched from the test suite. final class XDRBrightnessTests: XCTestCase { @@ -131,4 +142,131 @@ final class XDRBrightnessTests: XCTestCase { defer { self.otherDisplay.swBrightnessSemaphore.signal() } XCTAssertEqual(self.otherDisplay.swBrightnessSemaphore.wait(timeout: .now()), .timedOut) } + + func testSmoothBrightnessLatestDirectRequestWins() { + let display = RecordingBrightnessDisplay(3, name: "Recording Display", vendorNumber: 8_001, modelNumber: 4, serialNumber: 3, isDummy: true) + display.savePref(Float(1), key: .SwBrightness) + defer { + display.onWrite = nil + _ = display.setSwBrightness(1) + display.removePref(key: .SwBrightness) + } + let superseded = expectation(description: "first ramp step emitted") + let completed = expectation(description: "direct replacement completed") + let quiet = expectation(description: "old ramp stays cancelled") + var didReplace = false + var writesAtEndpoint = 0 + display.onWrite = { value in + if !didReplace { + didReplace = true + superseded.fulfill() + DispatchQueue.main.async { XCTAssertTrue(display.setSwBrightness(0.8)) } + } + if value == display.swBrightnessTransform(value: 0.8) { + writesAtEndpoint = display.writes.count + completed.fulfill() + DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) { quiet.fulfill() } + } + } + + XCTAssertTrue(display.setSwBrightness(0.4, smooth: true)) + wait(for: [superseded, completed, quiet], timeout: 1.0) + + XCTAssertEqual(display.writes.count, writesAtEndpoint) + XCTAssertEqual(display.writes.last?.value ?? -1, display.swBrightnessTransform(value: 0.8), accuracy: 0.0001) + } + + func testSmoothBrightnessLatestSmoothRequestReachesExactEndpointOnMain() { + let display = RecordingBrightnessDisplay(4, name: "Recording Display 2", vendorNumber: 8_002, modelNumber: 5, serialNumber: 4, isDummy: true) + display.savePref(Float(1), key: .SwBrightness) + defer { + display.onWrite = nil + _ = display.setSwBrightness(1) + display.removePref(key: .SwBrightness) + } + let superseded = expectation(description: "first ramp step emitted") + let completed = expectation(description: "smooth replacement completed") + let quiet = expectation(description: "old ramp stays cancelled") + var didReplace = false + var writesAtEndpoint = 0 + display.onWrite = { value in + if !didReplace { + didReplace = true + superseded.fulfill() + DispatchQueue.main.async { XCTAssertTrue(display.setSwBrightness(0.7, smooth: true)) } + } + if value == display.swBrightnessTransform(value: 0.7) { + writesAtEndpoint = display.writes.count + completed.fulfill() + DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) { quiet.fulfill() } + } + } + + XCTAssertTrue(display.setSwBrightness(0.4, smooth: true)) + wait(for: [superseded, completed, quiet], timeout: 1.0) + + XCTAssertFalse(display.writes.isEmpty) + XCTAssertEqual(display.writes.count, writesAtEndpoint) + XCTAssertEqual(display.writes.last?.value ?? -1, display.swBrightnessTransform(value: 0.7), accuracy: 0.0001) + XCTAssertTrue(display.writes.allSatisfy { $0.isMainThread }) + } + + func testSmoothBrightnessWithNoDistanceWritesExactValueOnce() { + let display = RecordingBrightnessDisplay(5, name: "Recording Display 3", vendorNumber: 8_003, modelNumber: 6, serialNumber: 5, isDummy: true) + display.savePref(Float(0.5), key: .SwBrightness) + defer { + display.onWrite = nil + _ = display.setSwBrightness(1) + display.removePref(key: .SwBrightness) + } + let completed = expectation(description: "zero-distance write completed") + display.onWrite = { _ in completed.fulfill() } + + XCTAssertTrue(display.setSwBrightness(0.5, smooth: true)) + wait(for: [completed], timeout: 1.0) + + XCTAssertEqual(display.writes.count, 1) + guard let write = display.writes.first else { + return XCTFail("Expected one smooth brightness write") + } + XCTAssertEqual(write.value, display.swBrightnessTransform(value: 0.5), accuracy: 0.0001) + } + + func testSmoothBrightnessCancelsDuringReconfiguration() { + let display = RecordingBrightnessDisplay(6, name: "Recording Display 4", vendorNumber: 8_004, modelNumber: 7, serialNumber: 6, isDummy: true) + display.savePref(Float(1), key: .SwBrightness) + let previousReconfigureID = app.reconfigureID + defer { + display.onWrite = nil + _ = display.setSwBrightness(1) + app.reconfigureID = previousReconfigureID + display.removePref(key: .SwBrightness) + } + app.reconfigureID = 1 + + XCTAssertTrue(display.setSwBrightness(0.4, smooth: true)) + let settled = expectation(description: "cancellation callback settled") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.03) { settled.fulfill() } + wait(for: [settled], timeout: 1.0) + XCTAssertTrue(display.writes.isEmpty) + } + + func testSmoothBrightnessCancelsDuringSleep() { + let display = RecordingBrightnessDisplay(7, name: "Recording Display 5", vendorNumber: 8_005, modelNumber: 8, serialNumber: 7, isDummy: true) + display.savePref(Float(1), key: .SwBrightness) + let previousSleepID = app.sleepID + defer { + display.onWrite = nil + _ = display.setSwBrightness(1) + app.sleepID = previousSleepID + display.removePref(key: .SwBrightness) + } + app.sleepID = 1 + + XCTAssertTrue(display.setSwBrightness(0.4, smooth: true)) + let settled = expectation(description: "sleep cancellation callback settled") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.03) { settled.fulfill() } + wait(for: [settled], timeout: 1.0) + XCTAssertTrue(display.writes.isEmpty) + } }