Skip to content

Fix PiP return animation via swizzling! #747

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix PiP return animation via swizzling! #747

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 106 additions & 11 deletions PlayerUI/Views/PUIPlayerView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import AVKit
import Combine
import SwiftUI
import ConfUIFoundation
import ObjectiveC

public final class PUIPlayerView: NSView {

Expand Down Expand Up @@ -88,6 +89,8 @@ public final class PUIPlayerView: NSView {
}

public init(player: AVPlayer) {
PIPFixUp.installPIPReturnRectInterposer()

self.player = player
if AVPictureInPictureController.isPictureInPictureSupported() {
self.pipController = AVPictureInPictureController(contentSource: .init(playerLayer: playerLayer))
Expand Down Expand Up @@ -180,8 +183,6 @@ public final class PUIPlayerView: NSView {
return player?.currentItem?.asset
}

private let playerLayer = PUIBoringPlayerLayer()

private func setupPlayer(_ player: AVPlayer) {
/// User settings are applied before setting up player observations, avoiding accidental overrides when initial values come in.
applyUserSettings(to: player)
Expand Down Expand Up @@ -505,6 +506,16 @@ public final class PUIPlayerView: NSView {
private var timeRemainingPlaceholder = "−00:00"
private var durationPlaceholder = "00:00"

private let playerLayer = PUIBoringPlayerLayer()
private lazy var playerView: NSView = {
let playerView = NSView()
playerView.translatesAutoresizingMaskIntoConstraints = false
playerView.wantsLayer = true
playerView.layer = playerLayer
playerLayer.backgroundColor = .clear
return playerView
}()

/// Displays the elapsed time.
/// This is a button for consistency with `trailingTimeButton`, but it doesn't have an action.
private lazy var leadingTimeButton: NSButton = {
Expand Down Expand Up @@ -671,11 +682,6 @@ public final class PUIPlayerView: NSView {
private func setupControls() {
addLayoutGuide(videoLayoutGuide)

let playerView = NSView()
playerView.translatesAutoresizingMaskIntoConstraints = false
playerView.wantsLayer = true
playerView.layer = playerLayer
playerLayer.backgroundColor = .clear
addSubview(playerView)
playerView.leadingAnchor.constraint(equalTo: leadingAnchor).isActive = true
playerView.trailingAnchor.constraint(equalTo: trailingAnchor).isActive = true
Expand Down Expand Up @@ -741,7 +747,7 @@ public final class PUIPlayerView: NSView {
controlsContainerView = NSStackView(views: [
timelineContainerView,
centerButtonsContainerView
])
])

controlsContainerView.orientation = .vertical
controlsContainerView.spacing = 12
Expand Down Expand Up @@ -802,7 +808,7 @@ public final class PUIPlayerView: NSView {

speedButton.$isEditingCustomSpeed.sink { [weak self] isEditing in
guard let self else { return }

showControls(animated: false)
resetMouseIdleTimer()
}
Expand Down Expand Up @@ -1382,6 +1388,9 @@ public final class PUIPlayerView: NSView {
appearanceDelegate?.presentDetachedStatus(.fullScreen.snapshot(using: snapshotClosure), for: self)

fullScreenButton.isHidden = true
// when the player is in full screen, PiP doesn't make any sense
// this does not prevent the scenario of the app being full screen and entering PiP
pipButton.isHidden = true
updateTopTrailingMenuPosition()
}

Expand All @@ -1394,7 +1403,9 @@ public final class PUIPlayerView: NSView {
/// The transition looks nicer if there's no background color, otherwise the player looks like it attaches
/// to the whole shelf area with black bars depending on the aspect ratio.
backgroundColor = .clear


pipButton.isHidden = false

if let d = appearanceDelegate {
fullScreenButton.isHidden = !d.playerViewShouldShowFullScreenButton(self)
}
Expand Down Expand Up @@ -1641,7 +1652,16 @@ extension PUIPlayerView: AVPictureInPictureControllerDelegate {

fullScreenButton.isHidden = false

completionHandler(true)
let videoRectInWindow = playerView.convert(playerLayer.videoRect, to: nil)
let videoRectInScreen = playerView.window?.convertToScreen(videoRectInWindow)

if let videoRectInScreen {
PIPFixUp.withReturnRect(videoRectInScreen) {
completionHandler(true)
}
} else {
completionHandler(false)
}
}

// Called Last
Expand All @@ -1652,6 +1672,81 @@ extension PUIPlayerView: AVPictureInPictureControllerDelegate {
}
}

/// The PIPFixUp swizzles ``NSWindow.convertToScreen`` so that we can inject a corrected return value to work around an
/// issue introduced in macOS 26.
///
/// The issue is that the pip return animation always animates the PiP window to the window's origin (0,0) which is bottom left
/// instead of to the position of the PiP's source AVPlayerLayer.
///
/// The fix up is applied as narrowly as possible to avoid unintended side effects.
///
/// We only override the return value while the `completionHandler(true)` in
/// `restoreUserInterfaceForPictureInPictureStopWithCompletionHandler` is being invoked. (Interestingly, this precludes us using the
/// the `async` overlay)
///
/// And we only install the swizzled method if we're on a platform version that is affected by the issue (26+ currently, with a feedback filed)
///
/// FB20159253
///
@available(macOS, deprecated: 26, message: "Check to make sure this still works, then raise the deprecation")
private final class PIPFixUp {
static private let lock = NSRecursiveLock()
static nonisolated(unsafe) private var _pipReturnDestination: NSRect?
static var pipReturnDestination: NSRect? {
lock.withLock {
_pipReturnDestination
}
}

static func withReturnRect(_ rect: NSRect, _ whileLocked: () -> Void) {
lock.withLock {
_pipReturnDestination = rect

whileLocked()

_pipReturnDestination = nil
}
}

@MainActor
static func installPIPReturnRectInterposer() {
// ensure one-time
struct Did {
@MainActor
static var once = false
}
guard !Did.once else { return }
Did.once = true

guard #available(macOS 26.0, *) else { return }

let targetClass: AnyClass = NSWindow.self
let originalSelector = #selector(NSWindow.convertToScreen)
let swizzledSelector = #selector(NSWindow.swz_convertToScreen)

guard
let original = class_getInstanceMethod(targetClass, originalSelector),
let swizzled = class_getInstanceMethod(targetClass, swizzledSelector)
else {
assertionFailure("convertRectToScreen: not found on NSWindow")
return
}

method_exchangeImplementations(original, swizzled)
}
}

extension NSWindow {
@objc
func swz_convertToScreen(_ rect: NSRect) -> NSRect {
guard let pipReturnRect = PIPFixUp.pipReturnDestination else {
return swz_convertToScreen(rect)
}

return pipReturnRect
}
}

#if DEBUG
struct PUIPlayerView_Previews: PreviewProvider {
static var previews: some View {
Expand Down
35 changes: 34 additions & 1 deletion WWDC/Controllers/Sessions/Playback/VideoPlayerWindowController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,29 @@ final class VideoPlayerWindowController: NSWindowController, NSWindowDelegate {

originalContainer?.layer?.backgroundColor = .black

let styleMask: NSWindow.StyleMask = [.titled, .closable, .miniaturizable, .resizable, .fullSizeContentView]
var styleMask: NSWindow.StyleMask = [.titled, .closable, .miniaturizable, .resizable, .fullSizeContentView]

if fullscreenOnly {
// fullscreen_transition: Remove these so we can make the window appear without any noticeable transition
styleMask.remove(.titled)
styleMask.insert(.borderless)
}

var rect = PUIPlayerWindow.bestScreenRectFromDetachingContainer(playerViewController.view, layoutGuide: playerViewController.playerView.videoLayoutGuide)
if rect == NSRect.zero { rect = PUIPlayerWindow.centerRectForProposedContentRect(playerViewController.view.bounds) }

let window = PUIPlayerWindow(contentRect: rect, styleMask: styleMask, backing: .buffered, defer: false)
window.isReleasedWhenClosed = true

if fullscreenOnly {
// We must insert this behavior because `styleMask.remove(.titled)` above disables it
window.collectionBehavior.insert(.fullScreenPrimary)
// fullscreen_transition: So we can make the window appear without any noticeable transition
// calling showWindow() has a default animation that makes it scale slightly
window.animationBehavior = .none
window.hasShadow = false
}

super.init(window: window)

window.delegate = self
Expand All @@ -62,16 +77,34 @@ final class VideoPlayerWindowController: NSWindowController, NSWindowDelegate {
fatalError("VideoPlayerWindowController can't be initialized with a coder")
}

/// `fullscreen_transition`: This exists because `showWindow(_:)` returns well before the window is actually visible on screen.
/// Which means when we want to enter fullscreen right away, the animation starts at screen (0,0) and looks bad.
///
/// So we use ``windowDidChangeOcclusionState`` to detect when the window is actually visible and only then enter fullscreen.
///
/// This API behavior may be OS version dependent. Currently confirmed on 26.0
weak var deferredEnterFullScreenSender: AnyObject?
override func showWindow(_ sender: Any?) {
super.showWindow(sender)

if !fullscreenOnly {
playerWindow?.applySizePreset(.half)
} else if let sender = sender as? AnyObject {
deferredEnterFullScreenSender = sender
} else {
window?.toggleFullScreen(sender)
}
}

func windowDidChangeOcclusionState(_ notification: Notification) {
guard let window else { return }

if let deferredEnterFullScreenSender, fullscreenOnly && window.occlusionState.contains(.visible) {
self.deferredEnterFullScreenSender = nil
window.toggleFullScreen(deferredEnterFullScreenSender)
}
}

// MARK: - Reattachment and fullscreen support

var windowWasAskedToClose = false
Expand Down