MultiSoundChanger icon indicating copy to clipboard operation
MultiSoundChanger copied to clipboard

Claude/rebuild x86 app to arm

Open solartrans opened this issue 3 months ago • 2 comments

works great, including media hotkeys now

solartrans avatar Nov 13 '25 21:11 solartrans

exciting! Looking forward to seeing this merged! I started getting occasional system panics after installing the release on my m1 mac, so hoping this addresses that. Not 100% sure it's related, but I hadn't gotten any in years until after this was installed.

edk avatar Nov 16 '25 03:11 edk

exciting! Looking forward to seeing this merged! I started getting occasional system panics after installing the release on my m1 mac, so hoping this addresses that. Not 100% sure it's related, but I hadn't gotten any in years until after this was installed.

i have not had any kernel panics on my m4 since i built this—you can compile my version and install it yourself if you'd like. i dont have it hosted anywhere though

solartrans avatar Nov 16 '25 06:11 solartrans

PR #39 Code Review: ARM64 Migration

Review performed by Claude Code

Overview

Migrates app from x86_64-only to universal binary by replacing the x86-only OSD.framework with a native Swift implementation. Bumps minimum macOS to 11.0.

User-Visible Changes

  • OSD overlay now uses custom in-app window instead of system OSD
  • App builds as universal binary (Intel + Apple Silicon)
  • Breaking: Minimum macOS raised from 10.10 to 11.0 - drops Catalina and earlier

Good

  • Clean removal of OSD.framework from pbxproj
  • Modern Swift: class → AnyObject for protocol constraints
  • SwiftLint script now graceful (exits 0 on Release, uses || true)
  • Thread safety: OSD dispatched to main queue
  • Memory management: [weak self], isReleasedWhenClosed = false
  • Multi-monitor support via display ID lookup
  • Thorough migration documentation

Blocking Issues

1. OSD Steals Focus (Critical)

NativeOSDManager.swift:167:

self.makeKeyAndOrderFront(nil)

This activates the app and yanks focus from the user's current window every time volume keys are pressed. Extremely disruptive.

Fix: Use orderFrontRegardless() instead.

2. Docs Claim Wrong Minimum Version (Critical)

ARM64_MIGRATION.md:166 states:

Minimum macOS Version: 10.10 (Yosemite) - unchanged

But Podfile:1 and project.pbxproj set MACOSX_DEPLOYMENT_TARGET = 11.0. This silently drops support for Catalina and earlier without warning.

Fix: Update docs to state minimum is 11.0, or evaluate if 10.15 would suffice (it supports ARM64 transition).

Should Fix

3. Window Recreation Causes Flicker

New OSDWindow created on every volume change. Rapid key presses can flicker/tear.

Fix: Reuse single window instance, just update content and reset fade timer.

4. Silent Fallback on Screen Lookup Failure

Falls back to NSScreen.main without logging when display ID not found.

Fix: Add Logger.warning to help debug multi-monitor issues.

5. Unused Variable

NativeOSDManager.swift:224:

_ = NSGraphicsContext.current?.cgContext

Remove entirely.

6. Dead Code

OSDWindow:134:

if let currentScreen = NSScreen.screens.first(where: { $0 == screen }) {

Always succeeds, currentScreen unused. Remove conditional.

QA Testing Plan

  • [ ] OSD appears on display under cursor with correct mute/unmute icon
  • [ ] Volume bars match current level
  • [ ] App doesn't steal focus when OSD shows
  • [ ] Overlay fades after ~1s without flicker on rapid key presses
  • [ ] Multi-monitor: OSD appears on correct screen
  • [ ] Apple Silicon: launches natively without Rosetta, media keys work
  • [ ] Intel: same functionality preserved

Verdict

Request changes. Fix the two blockers (focus stealing, version docs), then approve. Core approach is sound.

wodin avatar Dec 09 '25 10:12 wodin