Bug 81175 - [chromium] Add overdraw metrics for texture uploads
Summary: [chromium] Add overdraw metrics for texture uploads
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: 81156 81181 81222
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-14 17:07 PDT by Dana Jansens
Modified: 2012-03-16 10:14 PDT (History)
7 users (show)

See Also:


Attachments
Patch (54.67 KB, patch)
2012-03-14 21:09 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (21.36 KB, patch)
2012-03-15 09:21 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (21.59 KB, patch)
2012-03-15 17:33 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch for landing (21.46 KB, patch)
2012-03-16 07:36 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-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.