Bug 132603 - [iOS] Animate AVPlayerLayer into and out of full screen
Summary: [iOS] Animate AVPlayerLayer into and out of full screen
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-06 02:06 PDT by Jer Noble
Modified: 2014-05-06 16:36 PDT (History)
7 users (show)

See Also:


Attachments
Patch (5.04 KB, patch)
2014-05-06 02:10 PDT, Jer Noble
simon.fraser: review+
Details | Formatted Diff | Diff
Patch for landing. (8.30 KB, patch)
2014-05-06 14:16 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>