Bug 140897 – Prevent flicker when exiting fullscreen by synchronizing transactions.

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jeremy Jones]

Prevent flicker when exiting fullscreen by synchronizing transactions.

[Jeremy Jones]

Created attachment 245368 [details]
Patch

[Jeremy Jones]

Created attachment 245642 [details]
Patch

[Tim Horton]

Comment on attachment 245642 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=245642&action=review

> Source/WebCore/ChangeLog:8
> +        Synchronize across CAContexts when moving the video layer between layer hierarcies.

spelling "hierarchies"

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1063
> +    for (CAContext* context in [CAContext allContexts]) {

star's on the wrong side

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1064
> +        if (!fencePort)

I'm not sure it's appropriate to fence all CAContexts, is it? We really just want to hit WebKit's and the remote layer's, right?
Also, is the UIProcess' context really accessible from here? That seems vaguely surprising.

> Source/WebCore/platform/spi/cocoa/QuartzCoreSPI.h:60
> ++ (NSArray*)allContexts;

star's on the wrong side

[Tim Horton]

Oh, maybe you're synchronizing between two WebProcess-owned video-hosting CAContexts? In that case, can't you just hit both of them?

[Jeremy Jones]

(In reply to comment #3)
> Comment on attachment 245642 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=245642&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        Synchronize across CAContexts when moving the video layer between layer hierarcies.
> 
> spelling "hierarchies"

Fixed.

> 
> > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1063
> > +    for (CAContext* context in [CAContext allContexts]) {
> 
> star's on the wrong side

Fixed.

> 
> > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1064
> > +        if (!fencePort)
> 
> I'm not sure it's appropriate to fence all CAContexts, is it? We really just
> want to hit WebKit's and the remote layer's, right?
> Also, is the UIProcess' context really accessible from here? That seems
> vaguely surprising.

Now finds the root layer and and compares that against the CAContext's layer to make sure it fences necessary CAContexts.

> 
> > Source/WebCore/platform/spi/cocoa/QuartzCoreSPI.h:60
> > ++ (NSArray*)allContexts;
> 
> star's on the wrong side

Fixed.

[Jeremy Jones]

(In reply to comment #4)
> Oh, maybe you're synchronizing between two WebProcess-owned video-hosting
> CAContexts? In that case, can't you just hit both of them?

Exactly. There isn't a good way to go from CALayer to CAContext, so it now tries to match the root layer against the CAContext's layer.

[Jeremy Jones]

Created attachment 245891 [details]
Patch

[Tim Horton]

Comment on attachment 245891 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=245891&action=review

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1066
> +        newRootLayer = m_videoInlineLayer.get();

It's too bad you don't have the PlatformCALayer(Remote) here and can't acquire the contexts directly from those. Is it possible to make that happen? It would be much prettier than this.

[Jeremy Jones]

(In reply to comment #8)
> Comment on attachment 245891 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=245891&action=review
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1066
> > +        newRootLayer = m_videoInlineLayer.get();
> 
> It's too bad you don't have the PlatformCALayer(Remote) here and can't
> acquire the contexts directly from those. Is it possible to make that
> happen? It would be much prettier than this.

Agree, but I can't think of an elegant way to get the appropriate contexts down to this layer. I'll file another bug to investigate that.

[WebKit Commit Bot]

Comment on attachment 245891 [details]
Patch

Clearing flags on attachment: 245891

Committed r179574: <http://trac.webkit.org/changeset/179574>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.