RESOLVED FIXED 81175
[chromium] Add overdraw metrics for texture uploads
https://bugs.webkit.org/show_bug.cgi?id=81175
Summary [chromium] Add overdraw metrics for texture uploads
Dana Jansens
Reported 2012-03-14 17:07:50 PDT
[chromium] Add overdraw metrics for texture uploads
Attachments
Patch (54.67 KB, patch)
2012-03-14 21:09 PDT, Dana Jansens
no flags
Patch (21.36 KB, patch)
2012-03-15 09:21 PDT, Dana Jansens
no flags
Patch (21.59 KB, patch)
2012-03-15 17:33 PDT, Dana Jansens
no flags
Patch for landing (21.46 KB, patch)
2012-03-16 07:36 PDT, Dana Jansens
no flags
Dana Jansens
Comment 1 2012-03-14 21:09:26 PDT
Dana Jansens
Comment 2 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.
David Reveman
Comment 3 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.
Dana Jansens
Comment 4 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.
Dana Jansens
Comment 5 2012-03-15 08:19:06 PDT
Splitting this CL up so changes to CCOverdrawMetrics/CCOcclusionTracker happen in 81222. Should make review easier.
David Reveman
Comment 6 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.
Dana Jansens
Comment 7 2012-03-15 09:02:31 PDT
Ok cool. I'll make a bug to add this.
Dana Jansens
Comment 8 2012-03-15 09:21:09 PDT
Dana Jansens
Comment 9 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
Adrienne Walker
Comment 10 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?
Dana Jansens
Comment 11 2012-03-15 17:28:31 PDT
Done. Thanks!
Dana Jansens
Comment 12 2012-03-15 17:33:16 PDT
Dana Jansens
Comment 13 2012-03-16 07:36:32 PDT
Created attachment 132281 [details] Patch for landing
WebKit Review Bot
Comment 14 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>
WebKit Review Bot
Comment 15 2012-03-16 10:14:51 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.