| Summary: | Investigate rubberbanding/transform-overhang-[e|se|s].html failures | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | asvitkine | ||||||
| Component: | New Bugs | Assignee: | asvitkine | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | andersca, asvitkine, jamesr, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Mac | ||||||||
| OS: | OS X 10.7 | ||||||||
| Attachments: |
|
||||||||
|
Description
asvitkine
2012-02-07 11:39:22 PST
The code in ScrollView::calculateOverhangAreasForPainting() is not producing any overhang areas because contentsWidth() and contentsHeight() are both 0. Those checks is there to prevent a "full overhang" bug happening when the page is loading. It does make me think these are related to how the layout tests are setup for the composited case, since we're not seeing the same effect for the non-composited versions of these tests. The issue is that in the Composited path, the overhang layer was only getting enabled/disabled in ScrollView::scrollContents(). However, the order of things was such that the scrolling took place before the size had been set, at which time overhang was rightfully calculated to not exist. However, there was no update to this when the size was finally set. The solution is to re-calculate overhang bounds on size change in addition to scrolling. Created attachment 127583 [details]
Patch
James, can you review? Ping. Comment on attachment 127583 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127583&action=review Seems OK, but this weird mix of ScrollView/FrameView/RenderLayerCompositor for overhang areas is really annoying to try to understand and debug. On mac !CHROMIUM, the ScrollbarTheme sets up the overhang area layer to just be a tiled pattern and the shadow layer to be some sort of magic path-based layer. we don't support either of those in our compositor today, but I think we could with not too much effort. I think we'd be much better off using an approach like that rather than manually repainting + reuploading this overhang layer all the time. I suspect you won't want to work on improving this, but would you mind filing some bugs to improve this? I'm not super confident in our ability to maintain the current code. > Source/WebCore/platform/ScrollView.h:285 > + void updateOverhangAreas(); this should be private calculate...() probably should be private as well Created attachment 128228 [details]
Patch
(In reply to comment #6) > (From update of attachment 127583 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=127583&action=review > > Seems OK, but this weird mix of ScrollView/FrameView/RenderLayerCompositor for overhang areas is really annoying to try to understand and debug. > > On mac !CHROMIUM, the ScrollbarTheme sets up the overhang area layer to just be a tiled pattern and the shadow layer to be some sort of magic path-based layer. we don't support either of those in our compositor today, but I think we could with not too much effort. I think we'd be much better off using an approach like that rather than manually repainting + reuploading this overhang layer all the time. I suspect you won't want to work on improving this, but would you mind filing some bugs to improve this? Filed: https://bugs.webkit.org/show_bug.cgi?id=79251 > > Source/WebCore/platform/ScrollView.h:285 > > + void updateOverhangAreas(); > > this should be private > > calculate...() probably should be private as well Done. The commit-queue encountered the following flaky tests while processing attachment 128228 [details]: storage/indexeddb/index-count.html bug 79266 (authors: hans@chromium.org and jsbell@chromium.org) The commit-queue is continuing to process your patch. Comment on attachment 128228 [details] Patch Clearing flags on attachment: 128228 Committed r108536: <http://trac.webkit.org/changeset/108536> All reviewed patches have been landed. Closing bug. |