Bug 95323

Summary: Enable/disable composited scrolling based on overflow
Product: WebKit Reporter: Sami Kyöstilä <skyostil>
Component: New BugsAssignee: Sami Kyöstilä <skyostil>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, jamesr, mitz, simon.fraser, skyostil, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Sami Kyöstilä 2012-08-29 04:17:27 PDT
Enable/disable composited scrolling based on overflow
Comment 1 Sami Kyöstilä 2012-08-29 04:22:59 PDT
Created attachment 161180 [details]
Patch
Comment 2 Gyuyoung Kim 2012-08-29 04:59:15 PDT
Comment on attachment 161180 [details]
Patch

Attachment 161180 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13682161
Comment 3 Sami Kyöstilä 2012-08-29 08:12:18 PDT
Created attachment 161229 [details]
Patch
Comment 4 Sami Kyostila 2012-09-05 04:48:34 PDT
Ping, anyone up for a review? This bug makes layer scrolling not work very reliably because the necessary layers aren't getting created.

Alternatively, we could remove the entire optimization and always respect -webkit-overflow-scrolling:touch (at a memory cost).
Comment 5 Simon Fraser (smfr) 2012-09-05 11:43:02 PDT
Comment on attachment 161229 [details]
Patch

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

> Source/WebCore/rendering/RenderLayer.cpp:2649
> +    if (renderer()->view() && compositor()->updateLayerCompositingState(this))
> +        compositor()->setCompositingLayersNeedRebuild();

I wish we could do partial tree updates :(
Comment 6 WebKit Review Bot 2012-09-05 11:53:06 PDT
Comment on attachment 161229 [details]
Patch

Clearing flags on attachment: 161229

Committed r127620: <http://trac.webkit.org/changeset/127620>
Comment 7 WebKit Review Bot 2012-09-05 11:53:09 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 mitz 2012-09-05 13:55:33 PDT
(In reply to comment #6)
> (From update of attachment 161229 [details])
> Clearing flags on attachment: 161229
> 
> Committed r127620: <http://trac.webkit.org/changeset/127620>

The test added in this revision has been failing on Mountain Lion. See <http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r127620%20(613)/compositing/overflow/overflow-auto-with-touch-toggle-pretty-diff.html>.
Comment 9 mitz 2012-09-05 13:56:18 PDT
(In reply to comment #8)
> (In reply to comment #6)
> > (From update of attachment 161229 [details] [details])
> > Clearing flags on attachment: 161229
> > 
> > Committed r127620: <http://trac.webkit.org/changeset/127620>
> 
> The test added in this revision has been failing on Mountain Lion. See <http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r127620%20(613)/compositing/overflow/overflow-auto-with-touch-toggle-pretty-diff.html>.

The same test is also failing on Lion.
Comment 10 James Robinson 2012-09-05 13:57:24 PDT
From the ChangeLog: "Note that this test will pass only if OVERFLOW_SCROLLING is enabled."  Since those apple mac ports don't set OVERFLOW_SCROLLING, they are expected to fail and should be marked as failing or have failing -expected.txt's checked in or whatever the apple-mac port does these days.
Comment 11 mitz 2012-09-05 14:01:40 PDT
(In reply to comment #8)
> (In reply to comment #6)
> > (From update of attachment 161229 [details] [details])
> > Clearing flags on attachment: 161229
> > 
> > Committed r127620: <http://trac.webkit.org/changeset/127620>
> 
> The test added in this revision has been failing on Mountain Lion. See <http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r127620%20(613)/compositing/overflow/overflow-auto-with-touch-toggle-pretty-diff.html>.

Landed Mac-specific expected results in <http://trac.webkit.org/r127645>.
Comment 12 Simon Fraser (smfr) 2012-09-05 14:09:54 PDT
OVERFLOW_SCROLLING is a terrible name for this switch. We all have overflow scrolling! It should be ACCELERATED_OVERFLOW_SCROLLING or something.