Bug 132603

Summary: [iOS] Animate AVPlayerLayer into and out of full screen
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, glenn, jeremyj-wk, philipj, sergio, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
simon.fraser: review+
Patch for landing. none

Description Jer Noble 2014-05-06 02:06:21 PDT
[iOS] Animate AVPlayerLayer into and out of full screen
Comment 1 Jer Noble 2014-05-06 02:10:34 PDT
Created attachment 230893 [details]
Patch
Comment 2 Jer Noble 2014-05-06 02:11:14 PDT
<rdar://problem/16778157>
Comment 3 Simon Fraser (smfr) 2014-05-06 12:39:46 PDT
Comment on attachment 230893 [details]
Patch

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

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:354
> +- (id)init

instancetype?

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:359
> +        self.masksToBounds = YES;
> +        self.videoLayerGravity = AVVideoLayerGravityResizeAspect;

Shouldn't use property syntax in the init method.

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:380
> +static FloatRect largestRectWithAspectRatioInsideRect(float aspectRatio, FloatRect rect)
> +{
> +    if (aspectRatio > rect.size().aspectRatio()) {
> +        float dy = rect.width() / aspectRatio - rect.height();
> +        rect.inflateY(dy / 2);
> +    } else {
> +        float dx = rect.height() * aspectRatio - rect.width();
> +        rect.inflateX(dx / 2);
> +    }
> +    return rect;
> +}

Pretty sure we have code elsewhere that does aspect-ratio sizing.

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:392
> +    FloatRect rootBounds;
> +    CALayer *parent = self;
> +    while ((parent = [parent superlayer]))
> +        rootBounds = [parent bounds];

This is weird. How do you know how far up you're walking? Which rect do you actually want?
Comment 4 Jer Noble 2014-05-06 13:21:32 PDT
(In reply to comment #3)
> (From update of attachment 230893 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=230893&action=review
> 
> > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:354
> > +- (id)init
> 
> instancetype?

Changed.

> > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:359
> > +        self.masksToBounds = YES;
> > +        self.videoLayerGravity = AVVideoLayerGravityResizeAspect;
> 
> Shouldn't use property syntax in the init method.

Changed.

> > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:380
> > +static FloatRect largestRectWithAspectRatioInsideRect(float aspectRatio, FloatRect rect)
> > +{
> > +    if (aspectRatio > rect.size().aspectRatio()) {
> > +        float dy = rect.width() / aspectRatio - rect.height();
> > +        rect.inflateY(dy / 2);
> > +    } else {
> > +        float dx = rect.height() * aspectRatio - rect.width();
> > +        rect.inflateX(dx / 2);
> > +    }
> > +    return rect;
> > +}
> 
> Pretty sure we have code elsewhere that does aspect-ratio sizing.

We do (in RenderReplaced), but it's not generic.  I'll add this to GeometryUtils.h.

> > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:392
> > +    FloatRect rootBounds;
> > +    CALayer *parent = self;
> > +    while ((parent = [parent superlayer]))
> > +        rootBounds = [parent bounds];
> 
> This is weird. How do you know how far up you're walking? Which rect do you actually want?

We're looking for our window's root UIViewController's view's bounds.  I'll change this to pass in the view controller, and query it directly.
Comment 5 Jer Noble 2014-05-06 14:16:52 PDT
Created attachment 230931 [details]
Patch for landing.
Comment 6 Jer Noble 2014-05-06 16:36:49 PDT
Committed r168392: <http://trac.webkit.org/changeset/168392>