Bug 103470

Summary: Test compositing and slow scrolling behavior of fixed position elements under transformed elements
Product: WebKit Reporter: Xianzhu Wang <wangxianzhu>
Component: Layout and RenderingAssignee: Xianzhu Wang <wangxianzhu>
Status: RESOLVED FIXED    
Severity: Normal CC: 7raivis, jamesr, klobag, shawnsingh, simon.fraser, skyostil, tomhudson, vangelis, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 102543    
Bug Blocks:    
Attachments:
Description Flags
Patch (can't build on EWS, depends on the patch of bug 102543)
none
Patch (can't build on EWS, depends on the patch of bug 102543)
none
Patch
none
Patch none

Description Xianzhu Wang 2012-11-27 17:01:27 PST
Any visible fixed position element that is not composited can cause slow scrolling on Chromium (ScrollingCoordinator::mainThreadScrollingReasons -> ScrollingCoordinator::hasVisibleSlowRepaintFixedObjects()).

Fixed position element under transformed element is not composited because of the following code in RenderLayerCompositor:

    // Don't promote fixed position elements that are descendants of transformed elements.
    // They will stay fixed wrt the transformed element rather than the enclosing frame.
    if (container != m_renderView)
        return false;
Comment 1 James Robinson 2012-11-27 18:15:44 PST
http://dev.w3.org/csswg/css3-transforms/#transform-rendering says:

For elements whose layout is governed by the CSS box model, any value other than ‘none’ for the transform results in the creation of both a stacking context and a containing block. The object acts as a containing block for fixed positioned descendants.

NOTE: Is this effect on position:fixed necessary? If so, need to go into more detail here about why fixed positioned objects should do this, i.e., that it's much harder to implement otherwise. See Bug 16328.

Related bug: https://www.w3.org/Bugs/Public/show_bug.cgi?id=16328
Comment 2 Sami Kyöstilä 2012-11-28 08:30:01 PST
At least for Chromium that restriction in RenderLayerCompositor seems unnecessary because we already mark transformed layers as being containers for child fixed position layers and the compositor knows how to deal with that.
Comment 3 Xianzhu Wang 2012-11-29 16:12:05 PST
Created attachment 176844 [details]
Patch (can't build on EWS, depends on the patch of bug 102543)
Comment 4 Xianzhu Wang 2012-11-29 16:18:46 PST
Created attachment 176846 [details]
Patch (can't build on EWS, depends on the patch of bug 102543)

The last patch was a wrong version. Updated.
Comment 5 Sami Kyöstilä 2012-12-06 04:09:32 PST
Comment on attachment 176846 [details]
Patch (can't build on EWS, depends on the patch of bug 102543)

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

> Source/WebCore/page/scrolling/ScrollingCoordinator.cpp:345
> +            && viewportConstrainedObject->container() == viewportConstrainedObject->view())

Would it be better to avoid adding these types of fixed positioned elements to viewportConstrainedObjects() in the first place since they aren't really viewport constrained? This would make this function return the correct result in non-accelerated compositing builds too.
Comment 6 Xianzhu Wang 2012-12-06 09:36:13 PST
(In reply to comment #5)
> (From update of attachment 176846 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=176846&action=review
> 
> > Source/WebCore/page/scrolling/ScrollingCoordinator.cpp:345
> > +            && viewportConstrainedObject->container() == viewportConstrainedObject->view())
> 
> Would it be better to avoid adding these types of fixed positioned elements to viewportConstrainedObjects() in the first place since they aren't really viewport constrained? This would make this function return the correct result in non-accelerated compositing builds too.

I have thought of this but for now the elements are added/removed in RenderLayerModelObject::styleDidChange() which seems not to cover all the cases that an element becomes viewport constrained or not viewport constrained. For example, a viewport-constrained fixed position element may become non-viewport-constrained without style change of itself.

Seems we can add/remove the viewport-constrained objects in RenderLayerCompositor::requiresCompositingForPosition()?
Comment 7 Xianzhu Wang 2012-12-11 21:47:03 PST
Created attachment 178962 [details]
Patch
Comment 8 Simon Fraser (smfr) 2013-01-02 21:28:23 PST
Has this ever been observed on a web page in the wild?
Comment 9 Xianzhu Wang 2013-01-04 11:20:44 PST
(In reply to comment #8)
> Has this ever been observed on a web page in the wild?

Not really. Noticed this when we were looking for all possibilities causing slow scrolling on Chromium.
Comment 10 Simon Fraser (smfr) 2013-01-04 11:22:47 PST
Comment on attachment 178962 [details]
Patch

This this is just a new test, you should retitle the bug accordingly.
Comment 11 Xianzhu Wang 2013-01-04 11:44:16 PST
Created attachment 181349 [details]
Patch
Comment 12 WebKit Review Bot 2013-01-04 13:06:11 PST
Comment on attachment 181349 [details]
Patch

Clearing flags on attachment: 181349

Committed r138842: <http://trac.webkit.org/changeset/138842>
Comment 13 WebKit Review Bot 2013-01-04 13:06:16 PST
All reviewed patches have been landed.  Closing bug.