Summary: | REGRESSION (r123837): Full screen transition is broken at apple.com | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | mitz | ||||
Component: | CSS | Assignee: | Jer Noble <jer.noble> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Major | CC: | esprehn+autocc, jer.noble, jonlee, macpherson, menard, mikelawther, ojan.autocc, simon.fraser, webkit-bug-importer, webkit.review.bot | ||||
Priority: | P1 | Keywords: | InRadar, Regression | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
URL: | http://www.apple.com/macbook-pro/#macbookpro | ||||||
Attachments: |
|
Description
mitz
2012-09-01 18:01:42 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. Mike, do you have any thoughts on how to address this regression? Should we just revert r123837? 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? (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. > 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. 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. (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? 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 :( Created attachment 196681 [details]
Patch
Committed r148065: <http://trac.webkit.org/changeset/148065> |