Bug 132481 - [iOS] deviceScaleFactor is being double-applied when rendering captions in full screen mode
Summary: [iOS] deviceScaleFactor is being double-applied when rendering captions in fu...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: iPhone / iPad All
: P2 Critical
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on: 132498
Blocks:
  Show dependency treegraph
 
Reported: 2014-05-02 12:32 PDT by Brent Fulgham
Modified: 2014-05-04 14:02 PDT (History)
9 users (show)

See Also:


Attachments
Patch (1.76 KB, patch)
2014-05-02 12:38 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (3.28 KB, patch)
2014-05-02 18:11 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (9.88 KB, patch)
2014-05-02 21:23 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2014-05-02 12:32:58 PDT
When captions are played in full screen mode, they are about half the size we would expect. It looks like we are configuring the text track representation's graphics context to scale by the device scale factor, but later we apply that device scale factor again when generating the final image we pass to the full screen compositing step.
Comment 1 Brent Fulgham 2014-05-02 12:33:16 PDT
<rdar://problem/16507482>
Comment 2 Brent Fulgham 2014-05-02 12:38:22 PDT
Created attachment 230685 [details]
Patch
Comment 3 Jer Noble 2014-05-02 13:43:44 PDT
Comment on attachment 230685 [details]
Patch

r=me
Comment 4 Brent Fulgham 2014-05-02 14:39:55 PDT
Committed r168192: <http://trac.webkit.org/changeset/168192>
Comment 5 WebKit Commit Bot 2014-05-02 15:54:44 PDT
Re-opened since this is blocked by bug 132498
Comment 6 Brent Fulgham 2014-05-02 18:11:41 PDT
Created attachment 230726 [details]
Patch
Comment 7 Jer Noble 2014-05-02 18:28:33 PDT
Comment on attachment 230726 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1605
> +    if (representationLayer == m_textTrackRepresentationLayer) {
> +        if (m_videoFullscreenLayer && m_textTrackRepresentationLayer) {
> +            // Keep our text track layer in sync with the video layer at all times
> +            CGRect textFrame = m_videoLayer ? [m_videoLayer videoRect] : CGRectMake(0, 0, m_videoFullscreenFrame.width(), m_videoFullscreenFrame.height());
> +            [m_textTrackRepresentationLayer setFrame:textFrame];
> +        }

This is good, but it will miss when the video changes sizes during a single cue display. (I.e., changing rotation while a cue is up will not cause a re-layout.)

Brent and I discussed over IRC and alternate possibility for fixing this.
Comment 8 Brent Fulgham 2014-05-02 21:23:16 PDT
Created attachment 230738 [details]
Patch
Comment 9 Jer Noble 2014-05-02 21:36:34 PDT
Comment on attachment 230738 [details]
Patch

Nice! r=me
Comment 10 WebKit Commit Bot 2014-05-04 14:01:52 PDT
Comment on attachment 230738 [details]
Patch

Clearing flags on attachment: 230738

Committed r168245: <http://trac.webkit.org/changeset/168245>
Comment 11 WebKit Commit Bot 2014-05-04 14:02:00 PDT
All reviewed patches have been landed.  Closing bug.