Bug 66382 - LayerTilerChromium: Grow layers with tile granularity
Summary: LayerTilerChromium: Grow layers with tile granularity
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-17 08:45 PDT by Sami Kyostila
Modified: 2011-08-19 04:49 PDT (History)
4 users (show)

See Also:


Attachments
Patch (2.65 KB, patch)
2011-08-17 08:49 PDT, Sami Kyostila
jamesr: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sami Kyostila 2011-08-17 08:45:45 PDT
LayerTilerChromium: Grow layers with tile granularity
Comment 1 Sami Kyostila 2011-08-17 08:49:44 PDT
Created attachment 104177 [details]
Patch
Comment 2 Adrienne Walker 2011-08-17 11:49:07 PDT
I'm a little confused why this is necessary.

It's my expectation that initially and on resize there's an invalidation for the entire layer size.  If that's the case, then the only time you would have partial tiles filled up would be on the edges.  Because you have to repaint on resize anyway, this shouldn't be a problem.  Is this not true?

If this really is a problem, then I think the right fix here is to just change the tiler to have an explicit setLayerSize() function, rather than attempting to implicitly calculate it and grow to be the right size.

It's my fault it's that way in the first place, but I think growLayerToContain is a backwards way of handling the layer size.  It'd be nice to not go even further down that path.
Comment 3 Adrienne Walker 2011-08-17 11:53:27 PDT
Comment on attachment 104177 [details]
Patch

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

> Source/WebCore/ChangeLog:13
> +        viewport. This also improves the texture cache hit rate, since all
> +        tiles are the same size.

Also, all tile textures should already be m_tileSize in size.
Comment 4 Sami Kyostila 2011-08-17 12:04:43 PDT
I'm seeing partial tiles (and tile textures) along the viewport edges, i.e., not layer edges like you meant?. This is a problem because when a tile is invalidated or resized, it needs to be completely redrawn. This means that when scrolling slowly, all tiles along the leading viewport edge are continually being refreshed from scratch, causing lots of redundant WebKit rendering.

What my patch does is opportunistically grows the layer one tile beyond the viewport, so that a new row of tiles only needs to be generated for every 'tilesize' pixels scrolled.
Comment 5 James Robinson 2011-08-17 12:10:16 PDT
(In reply to comment #4)
> I'm seeing partial tiles (and tile textures) along the viewport edges, i.e., not layer edges like you meant?. This is a problem because when a tile is invalidated or resized, it needs to be completely redrawn. This means that when scrolling slowly, all tiles along the leading viewport edge are continually being refreshed from scratch, causing lots of redundant WebKit rendering.

I don't see this behavior locally.  Can you describe exactly what you're doing to see this and on what page?


> 
> What my patch does is opportunistically grows the layer one tile beyond the viewport, so that a new row of tiles only needs to be generated for every 'tilesize' pixels scrolled.
Comment 6 Sami Kyostila 2011-08-17 13:00:21 PDT
I added some logging to WebCore::LayerTilerChromium::prepareToUpdate to track paint events. Navigating to http://bbc.co.uk with a 1280x664 window I see both large paints events covering the whole viewport (x:0, y:0, w:1280, h:664) as well as smaller rects inside the viewport as the page elements load.

Scrolling the page down slowly, I then see these kinds of events:

 - (0, 0, 1280, 664)
 - (0, 2, 1280, 666)
 - (0, 4, 1280, 668)
 - (0, 6, 1280, 670)
 - etc.

These rects get passed to growLayerToContain(), so the layer grows with pixel granularity rather than tile granularity.
Comment 7 James Robinson 2011-08-17 13:05:01 PDT
(In reply to comment #6)
> I added some logging to WebCore::LayerTilerChromium::prepareToUpdate to track paint events. Navigating to http://bbc.co.uk with a 1280x664 window I see both large paints events covering the whole viewport (x:0, y:0, w:1280, h:664) as well as smaller rects inside the viewport as the page elements load.
> 
> Scrolling the page down slowly, I then see these kinds of events:
> 
>  - (0, 0, 1280, 664)
>  - (0, 2, 1280, 666)
>  - (0, 4, 1280, 668)
>  - (0, 6, 1280, 670)
>  - etc.
> 
> These rects get passed to growLayerToContain(), so the layer grows with pixel granularity rather than tile granularity.

This is with --force-compositing-mode, right?  I don't see this behavior either on trunk or the m14 branch.  bbc.co.uk is also a pretty busy page with animations going on so it's not the best page to test on.
Comment 8 James Robinson 2011-08-17 13:05:28 PDT
Also, what _exact_ revision are you testing with? It sounds like you might be hitting an old bug that cause fullscreen repaints when scrolling.
Comment 9 James Robinson 2011-08-18 16:29:27 PDT
Comment on attachment 104177 [details]
Patch

I don't think this patch is right, and we can't reproduce the problem is it trying to solve.
Comment 10 Sami Kyostila 2011-08-19 04:49:42 PDT
I was not able to reproduce this with WebKit HEAD, so I did some digging to see what actually caused the difference. Turns out I was missing r92255 ("Use edge-distance method for layer anti-aliasing") in my WebKit branch. A side effect of that change is that it makes all the layer tiles the same size, which fixes the issue I was seeing.

I'll close this bug as invalid.