Bug 81175

Summary: [chromium] Add overdraw metrics for texture uploads
Product: WebKit Reporter: Dana Jansens <danakj>
Component: New BugsAssignee: Dana Jansens <danakj>
Status: RESOLVED FIXED    
Severity: Normal CC: backer, enne, jamesr, nduca, piman, reveman, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 81156, 81181, 81222    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for landing none

Description Dana Jansens 2012-03-14 17:07:50 PDT
[chromium] Add overdraw metrics for texture uploads
Comment 1 Dana Jansens 2012-03-14 21:09:26 PDT
Created attachment 131982 [details]
Patch
Comment 2 Dana Jansens 2012-03-14 21:12:17 PDT
I decided to track texture upload metrics instead of "painted pixels" or something.

It measures the number of bytes of textures uploaded to texture memory, and the amount of bytes avoided via paint culling.
Comment 3 David Reveman 2012-03-15 06:48:52 PDT
So tracking uploads instead of painted pixels means we can't see the effect of per-tile painting in these metrics, right? I think that's good but it would be nice to have some additional metrics to track that. Maybe "InvalidatedPixels" and "PaintedPixels". "PaintedPixels" would be higher than "InvalidatedPixels" when per-tile painting is not used.
Comment 4 Dana Jansens 2012-03-15 08:12:07 PDT
(In reply to comment #3)
> So tracking uploads instead of painted pixels means we can't see the effect of per-tile painting in these metrics, right? I think that's good but it would be nice to have some additional metrics to track that. Maybe "InvalidatedPixels" and "PaintedPixels". "PaintedPixels" would be higher than "InvalidatedPixels" when per-tile painting is not used.

Is the number of painted pixels something that we are going to optimize beyond turning on per-tile-painting? I had the impression that we are always going to paint all dirty tiles that are visible at all.
Comment 5 Dana Jansens 2012-03-15 08:19:06 PDT
Splitting this CL up so changes to CCOverdrawMetrics/CCOcclusionTracker happen in 81222. Should make review easier.
Comment 6 David Reveman 2012-03-15 08:47:04 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > So tracking uploads instead of painted pixels means we can't see the effect of per-tile painting in these metrics, right? I think that's good but it would be nice to have some additional metrics to track that. Maybe "InvalidatedPixels" and "PaintedPixels". "PaintedPixels" would be higher than "InvalidatedPixels" when per-tile painting is not used.
> 
> Is the number of painted pixels something that we are going to optimize beyond turning on per-tile-painting? I had the impression that we are always going to paint all dirty tiles that are visible at all.

These metrics would help determine how much per-tile painting is buying us. I'm not aware of any worthwhile optimization beyond that.
Comment 7 Dana Jansens 2012-03-15 09:02:31 PDT
Ok cool. I'll make a bug to add this.
Comment 8 Dana Jansens 2012-03-15 09:21:09 PDT
Created attachment 132061 [details]
Patch
Comment 9 Dana Jansens 2012-03-15 09:22:36 PDT
This changes the symantics of UpdatableTile::m_updateRect slightly.

To tell if a tile was updated
before: !m_updateRect.isEmpty()
now: !m_updateRect.isEmpty() && !m_updateCulled
Comment 10 Adrienne Walker 2012-03-15 17:11:52 PDT
Comment on attachment 132061 [details]
Patch

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

Looks good.  R=me.

> Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp:66
> +    bool isDirtyForCurrentFrame() { return !m_dirtyRect.isEmpty() && (m_updateRect.isEmpty() || m_updateCulled); }

nit: can you add a comment to explain this expression?
Comment 11 Dana Jansens 2012-03-15 17:28:31 PDT
Done. Thanks!
Comment 12 Dana Jansens 2012-03-15 17:33:16 PDT
Created attachment 132160 [details]
Patch
Comment 13 Dana Jansens 2012-03-16 07:36:32 PDT
Created attachment 132281 [details]
Patch for landing
Comment 14 WebKit Review Bot 2012-03-16 10:14:46 PDT
Comment on attachment 132281 [details]
Patch for landing

Clearing flags on attachment: 132281

Committed r111023: <http://trac.webkit.org/changeset/111023>
Comment 15 WebKit Review Bot 2012-03-16 10:14:51 PDT
All reviewed patches have been landed.  Closing bug.