From 50e007e17a877a748007d74cffa68784fd79b605 Mon Sep 17 00:00:00 2001 From: Jeremy Jones Date: Wed, 22 Jun 2016 21:17:29 +0000 Subject: [PATCH] Adopt commitPriority to get rid of the 2 AVPL solution for PiP https://bugs.webkit.org/show_bug.cgi?id=158949 rdar://problem/26867866 Reviewed by Simon Fraser. No new tests because there is no behavior change. This reverts changes from https://bugs.webkit.org/show_bug.cgi?id=158148 and instead uses -[CAContext commitPriority:] to prevent flicker when moving a layer between contexts. commitPriority allows the layer to be added to the destination context before it is removed from the source context. * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h: remove m_secondaryVideoLayer. * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm: ditto (WebCore::MediaPlayerPrivateAVFoundationObjC::setVideoFullscreenGravity): ditto. (WebCore::MediaPlayerPrivateAVFoundationObjC::syncTextTrackBounds): ditto. (WebCore::MediaPlayerPrivateAVFoundationObjC::destroyVideoLayer): ditto. (WebCore::MediaPlayerPrivateAVFoundationObjC::updateVideoLayerGravity): ditto. * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm: ditto (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::addDisplayLayer): ditto * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm: ditto (WebCore::MediaPlayerPrivateMediaStreamAVFObjC::createPreviewLayers):ditto * platform/graphics/avfoundation/objc/VideoFullscreenLayerManager.h: ditto * platform/graphics/avfoundation/objc/VideoFullscreenLayerManager.mm: ditto (WebCore::VideoFullscreenLayerManager::setVideoLayer): ditto (WebCore::VideoFullscreenLayerManager::setVideoFullscreenLayer): ditto and adopt commitPriority. (WebCore::VideoFullscreenLayerManager::setVideoFullscreenFrame): ditto (WebCore::VideoFullscreenLayerManager::setVideoLayers): Deleted. (WebCore::VideoFullscreenLayerManager::didDestroyVideoLayer): remove m_secondaryVideoLayer. * platform/spi/cocoa/QuartzCoreSPI.h: Add commitPriority. Canonical link: https://commits.webkit.org/177117@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@202350 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebCore/ChangeLog | 33 +++++++++++ .../objc/MediaPlayerPrivateAVFoundationObjC.h | 1 - .../MediaPlayerPrivateAVFoundationObjC.mm | 24 ++------ .../MediaPlayerPrivateMediaSourceAVFObjC.mm | 2 +- .../MediaPlayerPrivateMediaStreamAVFObjC.mm | 2 +- .../objc/VideoFullscreenLayerManager.h | 3 +- .../objc/VideoFullscreenLayerManager.mm | 55 +++++-------------- .../platform/spi/cocoa/QuartzCoreSPI.h | 9 +++ 8 files changed, 63 insertions(+), 66 deletions(-) diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index db890deb7c2e..dcea4229bdbc 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,36 @@ +2016-06-20 Jeremy Jones + + Adopt commitPriority to get rid of the 2 AVPL solution for PiP + https://bugs.webkit.org/show_bug.cgi?id=158949 + rdar://problem/26867866 + + Reviewed by Simon Fraser. + + No new tests because there is no behavior change. This reverts changes from + https://bugs.webkit.org/show_bug.cgi?id=158148 and instead uses -[CAContext commitPriority:] + to prevent flicker when moving a layer between contexts. + commitPriority allows the layer to be added to the destination context before it is + removed from the source context. + + * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h: remove m_secondaryVideoLayer. + * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm: ditto + (WebCore::MediaPlayerPrivateAVFoundationObjC::setVideoFullscreenGravity): ditto. + (WebCore::MediaPlayerPrivateAVFoundationObjC::syncTextTrackBounds): ditto. + (WebCore::MediaPlayerPrivateAVFoundationObjC::destroyVideoLayer): ditto. + (WebCore::MediaPlayerPrivateAVFoundationObjC::updateVideoLayerGravity): ditto. + * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm: ditto + (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::addDisplayLayer): ditto + * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm: ditto + (WebCore::MediaPlayerPrivateMediaStreamAVFObjC::createPreviewLayers):ditto + * platform/graphics/avfoundation/objc/VideoFullscreenLayerManager.h: ditto + * platform/graphics/avfoundation/objc/VideoFullscreenLayerManager.mm: ditto + (WebCore::VideoFullscreenLayerManager::setVideoLayer): ditto + (WebCore::VideoFullscreenLayerManager::setVideoFullscreenLayer): ditto and adopt commitPriority. + (WebCore::VideoFullscreenLayerManager::setVideoFullscreenFrame): ditto + (WebCore::VideoFullscreenLayerManager::setVideoLayers): Deleted. + (WebCore::VideoFullscreenLayerManager::didDestroyVideoLayer): remove m_secondaryVideoLayer. + * platform/spi/cocoa/QuartzCoreSPI.h: Add commitPriority. + 2016-06-22 Simon Fraser REGRESSION (r201629): Weird button glitching on github.com diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h index 98e557e6f26a..b53897fd24ce 100644 --- a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h +++ b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h @@ -340,7 +340,6 @@ class MediaPlayerPrivateAVFoundationObjC : public MediaPlayerPrivateAVFoundation RetainPtr m_avPlayer; RetainPtr m_avPlayerItem; RetainPtr m_videoLayer; - RetainPtr m_secondaryVideoLayer; #if PLATFORM(IOS) || (PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE)) std::unique_ptr m_videoFullscreenLayerManager; MediaPlayer::VideoGravity m_videoFullscreenGravity; diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm index 1f90623a4a82..a2f286074aaa 100644 --- a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm +++ b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm @@ -734,25 +734,17 @@ void receivedChallengeRejection(const AuthenticationChallenge&) override [m_videoLayer setPlayer:m_avPlayer.get()]; [m_videoLayer setBackgroundColor:cachedCGColor(Color::black)]; -#if PLATFORM(MAC) - m_secondaryVideoLayer = adoptNS([allocAVPlayerLayerInstance() init]); - [m_secondaryVideoLayer setPlayer:m_avPlayer.get()]; - [m_secondaryVideoLayer setBackgroundColor:cachedCGColor(Color::black)]; -#endif - #ifndef NDEBUG [m_videoLayer setName:@"MediaPlayerPrivate AVPlayerLayer"]; - [m_secondaryVideoLayer setName:@"MediaPlayerPrivate AVPlayerLayer secondary"]; #endif [m_videoLayer addObserver:m_objcObserver.get() forKeyPath:@"readyForDisplay" options:NSKeyValueObservingOptionNew context:(void *)MediaPlayerAVFoundationObservationContextAVPlayerLayer]; updateVideoLayerGravity(); [m_videoLayer setContentsScale:player()->client().mediaPlayerContentsScale()]; - [m_secondaryVideoLayer setContentsScale:player()->client().mediaPlayerContentsScale()]; IntSize defaultSize = snappedIntRect(player()->client().mediaPlayerContentBoxRect()).size(); LOG(Media, "MediaPlayerPrivateAVFoundationObjC::createVideoLayer(%p) - returning %p", this, m_videoLayer.get()); #if PLATFORM(IOS) || (PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE)) - m_videoFullscreenLayerManager->setVideoLayers(m_videoLayer.get(), m_secondaryVideoLayer.get(), defaultSize); + m_videoFullscreenLayerManager->setVideoLayer(m_videoLayer.get(), defaultSize); #if PLATFORM(IOS) if ([m_videoLayer respondsToSelector:@selector(setPIPModeEnabled:)]) @@ -760,7 +752,6 @@ void receivedChallengeRejection(const AuthenticationChallenge&) override #endif #else [m_videoLayer setFrame:CGRectMake(0, 0, defaultSize.width(), defaultSize.height())]; - [m_secondaryVideoLayer setFrame:CGRectMake(0, 0, defaultSize.width(), defaultSize.height())]; #endif } @@ -773,14 +764,12 @@ void receivedChallengeRejection(const AuthenticationChallenge&) override [m_videoLayer removeObserver:m_objcObserver.get() forKeyPath:@"readyForDisplay"]; [m_videoLayer setPlayer:nil]; - [m_secondaryVideoLayer setPlayer:nil]; #if PLATFORM(IOS) || (PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE)) m_videoFullscreenLayerManager->didDestroyVideoLayer(); #endif m_videoLayer = nil; - m_secondaryVideoLayer = nil; } MediaTime MediaPlayerPrivateAVFoundationObjC::getStartDate() const @@ -1231,8 +1220,7 @@ void receivedChallengeRejection(const AuthenticationChallenge&) override { m_videoFullscreenGravity = gravity; - auto activeLayer = m_secondaryVideoLayer.get() ?: m_videoLayer.get(); - if (!activeLayer) + if (!m_videoLayer) return; NSString *videoGravity = AVLayerVideoGravityResizeAspect; @@ -1245,10 +1233,10 @@ void receivedChallengeRejection(const AuthenticationChallenge&) override else ASSERT_NOT_REACHED(); - if ([activeLayer videoGravity] == videoGravity) + if ([m_videoLayer videoGravity] == videoGravity) return; - [activeLayer setVideoGravity:videoGravity]; + [m_videoLayer setVideoGravity:videoGravity]; syncTextTrackBounds(); } @@ -1901,7 +1889,6 @@ static void fulfillRequestWithKeyData(AVAssetResourceLoadingRequest *request, Ar [CATransaction setDisableActions:YES]; NSString* gravity = shouldMaintainAspectRatio() ? AVLayerVideoGravityResizeAspect : AVLayerVideoGravityResize; [m_videoLayer.get() setVideoGravity:gravity]; - [m_secondaryVideoLayer.get() setVideoGravity:gravity]; [CATransaction commit]; } @@ -2197,8 +2184,7 @@ void determineChangedTracksFromNewTracksAndOldItems(MediaSelectionGroupAVFObjC* return; FloatRect videoFullscreenFrame = m_videoFullscreenLayerManager->videoFullscreenFrame(); - auto activeLayer = m_secondaryVideoLayer.get() ?: m_videoLayer.get(); - CGRect textFrame = activeLayer ? [activeLayer videoRect] : CGRectMake(0, 0, videoFullscreenFrame.width(), videoFullscreenFrame.height()); + CGRect textFrame = m_videoLayer ? [m_videoLayer videoRect] : CGRectMake(0, 0, videoFullscreenFrame.width(), videoFullscreenFrame.height()); [m_textTrackRepresentationLayer setFrame:textFrame]; #endif } diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm index 05ab20d0f19b..6e1362b2a401 100644 --- a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm +++ b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm @@ -755,7 +755,7 @@ static void CMTimebaseEffectiveRateChangedCallback(CMNotificationCenterRef, cons m_player->firstVideoFrameAvailable(); #if PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE) - m_videoFullscreenLayerManager->setVideoLayers(m_sampleBufferDisplayLayer.get(), nil, snappedIntRect(m_player->client().mediaPlayerContentBoxRect()).size()); + m_videoFullscreenLayerManager->setVideoLayer(m_sampleBufferDisplayLayer.get(), snappedIntRect(m_player->client().mediaPlayerContentBoxRect()).size()); #endif } diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm index 84c7d62324f6..881c3e04871d 100644 --- a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm +++ b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm @@ -404,7 +404,7 @@ m_videoBackgroundLayer.get().name = @"MediaPlayerPrivateMediaStreamAVFObjC preview background layer"; #if PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE) - m_videoFullscreenLayerManager->setVideoLayers(m_videoBackgroundLayer.get(), nil, snappedIntRect(m_player->client().mediaPlayerContentBoxRect()).size()); + m_videoFullscreenLayerManager->setVideoLayer(m_videoBackgroundLayer.get(), snappedIntRect(m_player->client().mediaPlayerContentBoxRect()).size()); #endif } diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/VideoFullscreenLayerManager.h b/Source/WebCore/platform/graphics/avfoundation/objc/VideoFullscreenLayerManager.h index fe4007ee9925..27d56dfecfe7 100644 --- a/Source/WebCore/platform/graphics/avfoundation/objc/VideoFullscreenLayerManager.h +++ b/Source/WebCore/platform/graphics/avfoundation/objc/VideoFullscreenLayerManager.h @@ -44,7 +44,7 @@ class VideoFullscreenLayerManager { PlatformLayer *videoInlineLayer() const { return m_videoInlineLayer.get(); } PlatformLayer *videoFullscreenLayer() const { return m_videoFullscreenLayer.get(); } FloatRect videoFullscreenFrame() const { return m_videoFullscreenFrame; } - void setVideoLayers(PlatformLayer *, PlatformLayer *, IntSize contentSize); + void setVideoLayer(PlatformLayer *, IntSize contentSize); void setVideoFullscreenLayer(PlatformLayer *, std::function completionHandler); void setVideoFullscreenFrame(FloatRect); void didDestroyVideoLayer(); @@ -55,7 +55,6 @@ class VideoFullscreenLayerManager { RetainPtr m_videoInlineLayer; RetainPtr m_videoFullscreenLayer; RetainPtr m_videoLayer; - RetainPtr m_secondaryVideoLayer; FloatRect m_videoFullscreenFrame; }; diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/VideoFullscreenLayerManager.mm b/Source/WebCore/platform/graphics/avfoundation/objc/VideoFullscreenLayerManager.mm index 0cd172f1c83c..12fc3071bfd4 100644 --- a/Source/WebCore/platform/graphics/avfoundation/objc/VideoFullscreenLayerManager.mm +++ b/Source/WebCore/platform/graphics/avfoundation/objc/VideoFullscreenLayerManager.mm @@ -66,27 +66,22 @@ - (void)setPosition:(CGPoint)position { } -void VideoFullscreenLayerManager::setVideoLayers(PlatformLayer *videoLayer, PlatformLayer *secondaryVideoLayer, IntSize contentSize) +void VideoFullscreenLayerManager::setVideoLayer(PlatformLayer *videoLayer, IntSize contentSize) { m_videoLayer = videoLayer; - m_secondaryVideoLayer = secondaryVideoLayer; [m_videoLayer web_disableAllActions]; - [m_secondaryVideoLayer web_disableAllActions]; m_videoInlineLayer = adoptNS([[WebVideoContainerLayer alloc] init]); #ifndef NDEBUG [m_videoInlineLayer setName:@"WebVideoContainerLayer"]; #endif [m_videoInlineLayer setFrame:CGRectMake(0, 0, contentSize.width(), contentSize.height())]; if (m_videoFullscreenLayer) { - [m_videoLayer removeFromSuperlayer]; - PlatformLayer *activeLayer = secondaryVideoLayer ? secondaryVideoLayer : videoLayer; - [activeLayer setFrame:CGRectMake(0, 0, m_videoFullscreenFrame.width(), m_videoFullscreenFrame.height())]; - [m_videoFullscreenLayer insertSublayer:activeLayer atIndex:0]; + [m_videoLayer setFrame:CGRectMake(0, 0, m_videoFullscreenFrame.width(), m_videoFullscreenFrame.height())]; + [m_videoFullscreenLayer insertSublayer:m_videoLayer.get() atIndex:0]; } else { [m_videoInlineLayer insertSublayer:m_videoLayer.get() atIndex:0]; [m_videoLayer setFrame:m_videoInlineLayer.get().bounds]; - [m_secondaryVideoLayer removeFromSuperlayer]; } } @@ -102,47 +97,26 @@ - (void)setPosition:(CGPoint)position [CATransaction begin]; [CATransaction setDisableActions:YES]; - if (m_secondaryVideoLayer && m_videoLayer) { - if (m_videoFullscreenLayer) { - [m_videoFullscreenLayer insertSublayer:m_secondaryVideoLayer.get() atIndex:0]; - [m_secondaryVideoLayer setFrame:CGRectMake(0, 0, m_videoFullscreenFrame.width(), m_videoFullscreenFrame.height())]; - } else if (m_videoInlineLayer) { - [m_videoLayer setFrame:[m_videoInlineLayer bounds]]; - [m_videoInlineLayer insertSublayer:m_videoLayer.get() atIndex:0]; - } + if (m_videoLayer) { + CAContext *oldContext = [m_videoLayer context]; - RetainPtr fullscreenLayer = m_videoFullscreenLayer; - RetainPtr videoLayer = m_videoLayer; - RetainPtr secondaryVideoLayer = m_secondaryVideoLayer; - - [CATransaction setCompletionBlock:[completionHandler, fullscreenLayer, videoLayer, secondaryVideoLayer] { - [CATransaction begin]; - [CATransaction setDisableActions:YES]; - - if (fullscreenLayer) - [videoLayer removeFromSuperlayer]; - else - [secondaryVideoLayer removeFromSuperlayer]; - - [CATransaction setCompletionBlock:[completionHandler] { - completionHandler(); - }]; - [CATransaction commit]; - }]; - } else if (m_videoLayer) { if (m_videoFullscreenLayer) { [m_videoFullscreenLayer insertSublayer:m_videoLayer.get() atIndex:0]; [m_videoLayer setFrame:CGRectMake(0, 0, m_videoFullscreenFrame.width(), m_videoFullscreenFrame.height())]; } else if (m_videoInlineLayer) { [m_videoLayer setFrame:[m_videoInlineLayer bounds]]; - [m_videoLayer removeFromSuperlayer]; [m_videoInlineLayer insertSublayer:m_videoLayer.get() atIndex:0]; } else [m_videoLayer removeFromSuperlayer]; - CAContext *oldContext = [m_videoFullscreenLayer context]; - CAContext *newContext = [m_videoInlineLayer context]; + CAContext *newContext = [m_videoLayer context]; if (oldContext && newContext && oldContext != newContext) { +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101200 + if ([oldContext respondsToSelector:@selector(setCommitPriority:)]) { + [oldContext setCommitPriority:0]; + [newContext setCommitPriority:1]; + } +#endif mach_port_t fencePort = [oldContext createFencePort]; [newContext setFencePort:fencePort]; mach_port_deallocate(mach_task_self(), fencePort); @@ -166,18 +140,15 @@ - (void)setPosition:(CGPoint)position if (!m_videoFullscreenLayer) return; - PlatformLayer *activeLayer = m_secondaryVideoLayer.get() ? m_secondaryVideoLayer.get() : m_videoLayer.get(); - [activeLayer setFrame:CGRectMake(0, 0, videoFullscreenFrame.width(), videoFullscreenFrame.height())]; + [m_videoLayer setFrame:CGRectMake(0, 0, videoFullscreenFrame.width(), videoFullscreenFrame.height())]; } void VideoFullscreenLayerManager::didDestroyVideoLayer() { [m_videoLayer removeFromSuperlayer]; - [m_secondaryVideoLayer removeFromSuperlayer]; m_videoInlineLayer = nil; m_videoLayer = nil; - m_secondaryVideoLayer = nil; } } diff --git a/Source/WebCore/platform/spi/cocoa/QuartzCoreSPI.h b/Source/WebCore/platform/spi/cocoa/QuartzCoreSPI.h index 124b0e9a1a7c..ce75b4f66760 100644 --- a/Source/WebCore/platform/spi/cocoa/QuartzCoreSPI.h +++ b/Source/WebCore/platform/spi/cocoa/QuartzCoreSPI.h @@ -41,6 +41,12 @@ #import #endif +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101200 +@interface CAContext () +- (void)setCommitPriority:(uint32_t)commitPriority; +@end +#endif + #endif // __OBJC__ #else @@ -59,6 +65,9 @@ - (mach_port_t)createFencePort; - (void)setFencePort:(mach_port_t)port; - (void)setFencePort:(mach_port_t)port commitHandler:(void(^)(void))block; +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101200 +@property uint32_t commitPriority; +#endif #if PLATFORM(MAC) @property BOOL colorMatchUntaggedContent; #endif