RESOLVED FIXED 175152
Improve WebKitLegacy video fullscreen animation begin and end rects.
https://bugs.webkit.org/show_bug.cgi?id=175152
Summary Improve WebKitLegacy video fullscreen animation begin and end rects.
Jeremy Jones
Reported 2017-08-03 13:24:37 PDT
Improve WebKitLegacy video fullscreen animation begin and end rects.
Attachments
Patch (4.90 KB, patch)
2017-08-03 13:34 PDT, Jeremy Jones
no flags
Patch for landing. (5.01 KB, patch)
2017-08-03 15:56 PDT, Jeremy Jones
no flags
Jeremy Jones
Comment 1 2017-08-03 13:29:12 PDT
Jeremy Jones
Comment 2 2017-08-03 13:34:40 PDT
Eric Carlson
Comment 3 2017-08-03 13:45:06 PDT
Comment on attachment 317146 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317146&action=review > Source/WebCore/platform/mac/WebVideoFullscreenController.mm:236 > +static NSRect frameExpandedToRatioOfFrame(NSRect frameToExpand, NSRect frame) Nit: NSRect& > Source/WebCore/platform/mac/WebVideoFullscreenController.mm:284 > NSRect frame = [self videoElementRect]; > NSRect endFrame = [screen frame]; > - constrainFrameToRatioOfFrame(&endFrame, &frame); > + frame = frameExpandedToRatioOfFrame(frame, endFrame); Nit: this would be slightly cleaner as something like NSRect frame = frameExpandedToRatioOfFrame([self videoElementRect], endFrame);
Jeremy Jones
Comment 4 2017-08-03 14:25:36 PDT
(In reply to Eric Carlson from comment #3) > Comment on attachment 317146 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=317146&action=review > > > Source/WebCore/platform/mac/WebVideoFullscreenController.mm:236 > > +static NSRect frameExpandedToRatioOfFrame(NSRect frameToExpand, NSRect frame) > > Nit: NSRect& > For small structs the cost of indirection might be higher than the cost of copying. > > Source/WebCore/platform/mac/WebVideoFullscreenController.mm:284 > > NSRect frame = [self videoElementRect]; > > NSRect endFrame = [screen frame]; > > - constrainFrameToRatioOfFrame(&endFrame, &frame); > > + frame = frameExpandedToRatioOfFrame(frame, endFrame); > > Nit: this would be slightly cleaner as something like > > NSRect frame = frameExpandedToRatioOfFrame([self videoElementRect], > endFrame); Done.
Jeremy Jones
Comment 5 2017-08-03 15:56:08 PDT
Created attachment 317174 [details] Patch for landing.
WebKit Commit Bot
Comment 6 2017-08-03 18:58:36 PDT
Comment on attachment 317174 [details] Patch for landing. Clearing flags on attachment: 317174 Committed r220248: <http://trac.webkit.org/changeset/220248>
Note You need to log in before you can comment on or make changes to this bug.