WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
95650
REGRESSION (
r123837
): Full screen transition is broken at apple.com
https://bugs.webkit.org/show_bug.cgi?id=95650
Summary
REGRESSION (r123837): Full screen transition is broken at apple.com
mitz
Reported
2012-09-01 18:01:42 PDT
To reproduce: navigate to the URL. When the video starts playing, click the full screen arrows in the playback controls. Observe the transition into full-screen playback. Press Esc to exit full screen mode and observe the transition. Rather than animate smoothly, the video content scale changes instantaneously, while remaining clipped to its original bounds. Its position the animates to the center of the screen, without resizing. Then the size changes abruptly. This was caused by <
http://trac.webkit.org/r123837
>, the fix for
bug 92220
.
Attachments
Patch
(1.45 KB, patch)
2013-04-05 14:17 PDT
,
Jer Noble
simon.fraser
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
mitz
Comment 1
2012-09-01 18:02:20 PDT
<
rdar://problem/12222130
>
Mike Lawther
Comment 2
2012-09-03 00:07:43 PDT
I can reproduce this, but only with Safari. I tested with the shipping Safari 6.0 (7536.25) on MacOS 10.7, and with a locally built WebKit synced past
r123837
. The transition to fullscreen is transitioning from a fixed width,height of 848,480 to a a percent based width,height of 100%,100%. I have not dived into the fullscreen or video player code, but I'm assuming 100% in this case is the extent of the monitor size. From what I see, the blend of these is performed correctly - my monitor is 2560x1600 and the values appear correct, eg: eval'd blending of (type Fixed, value 848.000000) to (type Percent, value 100.000000) at 13.0802% = 1071.932983px eval'd blending of (type Fixed, value 480.000000) to (type Percent, value 100.000000) at 13.0802% = 626.498230px [...] eval'd blending of (type Fixed, value 848.000000) to (type Percent, value 100.000000) at 84.5118% = 2294.842529px eval'd blending of (type Fixed, value 480.000000) to (type Percent, value 100.000000) at 84.5118% = 1426.532471px Going back out of fullscreen, the numbers don't look the same, eg: eval'd blending of (type Percent, value 100.000000) to (type Fixed, value 848.000000) at 45.7933% = 848.000000px eval'd blending of (type percent, value 100.000000) to (type Fixed, value 480.000000) at 45.7933% = 480.000000px [...] eval'd blending of (type Percent, value 100.000000) to (type Fixed, value 848.000000) at 78.0345% = 848.000000px eval'd blending of (type Percent, value 100.000000) to (type Fixed, value 480.000000) at 78.0345% = 480.000000px It appears the container has already been resized to 848x480, and so the blended value is always 848,480.
mitz
Comment 3
2012-10-02 10:03:29 PDT
Mike, do you have any thoughts on how to address this regression? Should we just revert
r123837
?
Mike Lawther
Comment 4
2012-10-02 16:41:04 PDT
From what I can see in the digging I did above, the smooth transition itself is working correctly. My gut feel is that there is a workaround in the existing fullscreen code to make the transition smooth. Now that the underlying transition is actually smooth, the assumptions in the workaround are no longer valid, leading to weirdness. @jer.noble - does that sound likely? If not, I suggest we: - revert
r123837
(it's a dead simple patch) - someone helps me write a test for the existing fullscreen transition, and we land this - re-land the smooth transition patch, making sure fullscreen doesn't break How does that sound?
Jer Noble
Comment 5
2012-10-02 17:21:53 PDT
(In reply to
comment #4
)
> From what I can see in the digging I did above, the smooth transition itself is working correctly. > > My gut feel is that there is a workaround in the existing fullscreen code to make the transition smooth. Now that the underlying transition is actually smooth, the assumptions in the workaround are no longer valid, leading to weirdness. > > @jer.noble - does that sound likely?
The blending is happening at the window server level. We resize the view to take up the entire screen, then ask for the element's screen rect. The window server then interpolates from the original screen rect to the final one. Before
r123837
, the full screen element's renderer's contentsBox() returned the full screen rect. After
r123837
, the full screen element's renderer's contentsBox() returns the original box.
> If not, I suggest we: > > - revert
r123837
(it's a dead simple patch) > - someone helps me write a test for the existing fullscreen transition, and we land this
There are some helper functions in Internals that let you do processing between when full screen starts and ends. That would be a good time to check the bounds of the full screen element.
Mike Lawther
Comment 6
2012-10-02 18:25:55 PDT
> The blending is happening at the window server level. We resize the view to take up the entire screen, then ask for the element's screen rect. The window server then interpolates from the original screen rect to the final one. > > Before
r123837
, the full screen element's renderer's contentsBox() returned the full screen rect. > After
r123837
, the full screen element's renderer's contentsBox() returns the original box.
This sounds like the assumption that has been violated. Prior to
r123837
, the transition between a fixed size and a percent would 'snap' instantly to the final state. Is the fix here is to simply remove the CSS transition upon the view resize? It sounds like you actually want it to be instantaneous.
Mike Lawther
Comment 7
2012-11-01 16:21:19 PDT
Hey Dan, Jer - did you try removing the CSS transition on the view resize? It really feels like this is the right solution here, as Jer points out in this case the blending is happening at the window server level.
Jer Noble
Comment 8
2012-11-01 16:30:52 PDT
(In reply to
comment #7
)
> Hey Dan, Jer - did you try removing the CSS transition on the view resize? It really feels like this is the right solution here, as Jer points out in this case the blending is happening at the window server level.
I'm confused. Is there a CSS transition which is being added to the page through a style sheet?
Mike Lawther
Comment 9
2012-11-01 17:16:52 PDT
There is definitely a CSS transition happening there, evidenced both by what you describe, and the measurements I took in
comment #2
. It's a transition from a fixed size (eg a pixel size of 848x480) to a percentage size (100%x100%). This is the assumption that I reckon has been violated - prior to
r123837
, a mixed unit transition would instantly 'snap' from begin to end, with no interpolation. And so the window server code provides the interpolation instead (I'm guessing animateFromRect in Source/WebCore/platform/mac/WebVideoFullscreenController.mm). Now, the CSS transition actually interpolates as well, and there are too many cooks in the animation kitchen. I'm afraid I don't know where the CSS transition is coming from - I had a quick dig around in likely sounding culprits such as Source/WebCore/css/fullscreenQuickTime.css. My Mac dev machine is currently hosed, so I can't check any of this stuff :(
Jer Noble
Comment 10
2013-04-05 14:17:27 PDT
Created
attachment 196681
[details]
Patch
Jer Noble
Comment 11
2013-04-09 16:56:17 PDT
Committed
r148065
: <
http://trac.webkit.org/changeset/148065
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug