[WK2][Mac] Fullscreen animations with mismatched aspect ratios are "squished".

[Jer Noble]

[WK2][Mac] Use CALayer animations rather than NSWindow animations for the Fullscreen transition

[Jer Noble]

Created attachment 247605 [details]
Patch

[Tim Horton]

Comment on attachment 247605 [details]
Patch

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

> Source/WebKit2/ChangeLog:3
> +        [WK2][Mac] Use CALayer animations rather than NSWindow animations for the Fullscreen transition

The title is not great. Why not explain what you're fixing instead of how?

> Source/WebKit2/UIProcess/mac/WKFullScreenWindowController.mm:97
> +    NSView* contentView = [window contentView];

star's on the wrong side.

> Source/WebKit2/UIProcess/mac/WKFullScreenWindowController.mm:378
> +    NSView* contentView = [[self window] contentView];

star's on the wrong side

> Source/WebKit2/UIProcess/mac/WKFullScreenWindowController.mm:441
>  - (NSArray *)customWindowsToEnterFullScreenForWindow:(NSWindow *)window

Do we still need this customWindowsToEnterFullScreenForWindow: mechanism at all?

> Source/WebKit2/UIProcess/mac/WKFullScreenWindowController.mm:562
> +        fadeAnimation.toValue = (id)CGColorGetConstantColor(kCGColorBlack);

Could always use [NSColor blackColor] here (either way!).

[Jer Noble]

(In reply to comment #2)
> Comment on attachment 247605 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=247605&action=review
> 
> > Source/WebKit2/ChangeLog:3
> > +        [WK2][Mac] Use CALayer animations rather than NSWindow animations for the Fullscreen transition
> 
> The title is not great. Why not explain what you're fixing instead of how?

Sure thing; I'll update the title.

> > Source/WebKit2/UIProcess/mac/WKFullScreenWindowController.mm:97
> > +    NSView* contentView = [window contentView];
> 
> star's on the wrong side.

Fixed.

> > Source/WebKit2/UIProcess/mac/WKFullScreenWindowController.mm:378
> > +    NSView* contentView = [[self window] contentView];
> 
> star's on the wrong side

Ditto.

> > Source/WebKit2/UIProcess/mac/WKFullScreenWindowController.mm:441
> >  - (NSArray *)customWindowsToEnterFullScreenForWindow:(NSWindow *)window
> 
> Do we still need this customWindowsToEnterFullScreenForWindow: mechanism at
> all?

Yes. You won't get the custom start callback unless you implement this delegate method.

> > Source/WebKit2/UIProcess/mac/WKFullScreenWindowController.mm:562
> > +        fadeAnimation.toValue = (id)CGColorGetConstantColor(kCGColorBlack);
> 
> Could always use [NSColor blackColor] here (either way!).

Or! WebKit::cachedCGColor(WebKit::Color::black, WebKit::ColorSpaceDeviceRGB)!

(I'll just leave it as is.) :)

[Jer Noble]

Created attachment 247908 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 247908 [details]
Patch for landing

Clearing flags on attachment: 247908

Committed r181770: <http://trac.webkit.org/changeset/181770>

[Jessie Berlin]

I attempted to fix the 32-bit build breakage in http://trac.webkit.org/changeset/181776

[Jer Noble]

Thanks Jessie!