Bug 81552 - [chromium] Push culled tiles that are prepainted
Summary: [chromium] Push culled tiles that are prepainted
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dana Jansens
URL:
Keywords:
Depends on: 81529
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-19 13:32 PDT by Dana Jansens
Modified: 2012-03-19 20:11 PDT (History)
7 users (show)

See Also:


Attachments
Patch (5.68 KB, patch)
2012-03-19 13:34 PDT, Dana Jansens
enne: review+
Details | Formatted Diff | Diff
Patch (7.51 KB, patch)
2012-03-19 14:55 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (7.41 KB, patch)
2012-03-19 17:53 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dana Jansens 2012-03-19 13:32:11 PDT
[chromium] Push culled tiles that are prepainted
Comment 1 Dana Jansens 2012-03-19 13:34:08 PDT
Created attachment 132652 [details]
Patch
Comment 2 Dana Jansens 2012-03-19 13:49:05 PDT
To be clear, during an idle paint, we still attempt to VisiblePaint, which would mark the tile culled. Then even though we would actually idle-paint the tile, we would not push it to the impl thread.
Comment 3 Dana Jansens 2012-03-19 14:55:01 PDT
Created attachment 132678 [details]
Patch
Comment 4 Dana Jansens 2012-03-19 14:56:13 PDT
Comment on attachment 132652 [details]
Patch

Fix this up a bit. I want a tile to be marked as "updated" ie not culled if it was updated on any pass through prepareToUpdateTiles. Previous version depended on the fact that PaintVisible culled and PaintIdle did not, this one is more robust.
Comment 5 Adrienne Walker 2012-03-19 17:19:17 PDT
Comment on attachment 132652 [details]
Patch

Looks good to me.
Comment 6 Adrienne Walker 2012-03-19 17:27:40 PDT
Comment on attachment 132678 [details]
Patch

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

I like this one better too.  One small nit:

> Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp:-432
> -                    tile->m_updateCulled = true;

Can you add an assert(!tile->m_updated) here, since this would cause issues if m_updateRect got touched but somehow m_updated was true?
Comment 7 Dana Jansens 2012-03-19 17:42:46 PDT
Comment on attachment 132678 [details]
Patch

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

>> Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp:-432
>> -                    tile->m_updateCulled = true;
> 
> Can you add an assert(!tile->m_updated) here, since this would cause issues if m_updateRect got touched but somehow m_updated was true?

Yeh, great plan. Thanks.
Comment 8 Dana Jansens 2012-03-19 17:53:04 PDT
Created attachment 132737 [details]
Patch
Comment 9 WebKit Review Bot 2012-03-19 20:11:01 PDT
Comment on attachment 132737 [details]
Patch

Clearing flags on attachment: 132737

Committed r111335: <http://trac.webkit.org/changeset/111335>
Comment 10 WebKit Review Bot 2012-03-19 20:11:05 PDT
All reviewed patches have been landed.  Closing bug.