Bug 88924 - [chromium] Add an allocation step for CCRenderer before drawing a frame
Summary: [chromium] Add an allocation step for CCRenderer before drawing a frame
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:
Blocks: 89474
  Show dependency tree / graph
 
Reported: 2012-06-12 14:47 PDT by Dana Jansens
Modified: 2012-06-19 15:19 PDT (History)
7 users (show)

See Also:

Attachments
Patch (6.57 KB, patch)
2012-06-12 14:47 PDT, Dana Jansens 		no flags Details | Formatted Diff | Diff
Patch (24.79 KB, patch)
2012-06-14 17:23 PDT, Dana Jansens 		no flags Details | Formatted Diff | Diff
Patch (35.63 KB, patch)
2012-06-18 12:37 PDT, Dana Jansens 		no flags Details | Formatted Diff | Diff
Patch (5.95 KB, patch)
2012-06-19 12:40 PDT, Dana Jansens 		no flags Details | Formatted Diff | Diff
Patch (5.92 KB, patch)
2012-06-19 13:17 PDT, Dana Jansens 		no flags Details | Formatted Diff | Diff
Patch for landing (27.75 KB, patch)
2012-06-19 14:21 PDT, Dana Jansens 		no flags Details | Formatted Diff | Diff
Patch for landing (6.05 KB, patch)
2012-06-19 14:22 PDT, Dana Jansens 		no flags Details | Formatted Diff | Diff
Patch for landing (5.75 KB, patch)
2012-06-19 14:23 PDT, Dana Jansens 		no flags Details | Formatted Diff | Diff
Show Obsolete (7) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Dana Jansens 2012-06-12 14:47:20 PDT 
[chromium] Add an allocation step for CCRenderer before drawing a frame
Comment 1 Dana Jansens 2012-06-12 14:47:49 PDT 
Created attachment 147168 [details]
Patch
Comment 2 Dana Jansens 2012-06-13 08:35:16 PDT 
Not for review yet (misclick on EWS) just putting up steps as I work on this RenderSurface textures in LRC code. (Longing for github :))
Comment 3 Dana Jansens 2012-06-14 17:23:37 PDT 
Created attachment 147686 [details]
Patch
Comment 4 Dana Jansens 2012-06-18 12:37:33 PDT 
Created attachment 148150 [details]
Patch

another peek before i hook this up to LRC.
Comment 5 Dana Jansens 2012-06-18 12:40:55 PDT 
I'm planning to do something like this for allocation of RenderPass textures (this is always in the root/host compositor).

I'm going to start hooking this up to LRC and see how it fares, I had done this earlier without the CCScopedTexture class and it was a bit ugly, so I've cleaned it up some before attempting again.

If anyone wants to look at it and tell me they like/dislike the design ideas here that would be very nice as I'm still not overly committed to it.

Basic idea:

Add a step before drawing to decide what passes will get allocated, and which saved textures from the previous frame will not be clobbered and will be available when they are needed in the current frame.

Then when drawing we already know which passes we draw, and can allocate textures as needed on the fly.
Comment 6 Dana Jansens 2012-06-19 12:40:56 PDT 
Created attachment 148391 [details]
Patch

This is just the piece to add the allocation step to the draw-a-frame process.
Comment 7 Dana Jansens 2012-06-19 13:17:23 PDT 
Created attachment 148402 [details]
Patch

Decide allocations/Set the memory limit before reserving/testing cached surface textures.
Comment 8 Adrienne Walker 2012-06-19 14:17:16 PDT 
Comment on attachment 148402 [details]
Patch

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

R=me.

> Source/WebCore/platform/graphics/chromium/cc/CCRenderer.h:71
> +    // This method should be a no-op in a nested compositor that does not render its passes directly.

Not sure this comment adds anything.  Other functions will probably be a no-op too.  We can document that in the derived interface.
Comment 9 Dana Jansens 2012-06-19 14:20:16 PDT 
Comment on attachment 148402 [details]
Patch

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

thanks!

>> Source/WebCore/platform/graphics/chromium/cc/CCRenderer.h:71
>> +    // This method should be a no-op in a nested compositor that does not render its passes directly.
> 
> Not sure this comment adds anything.  Other functions will probably be a no-op too.  We can document that in the derived interface.

Ya.. good call.
Comment 10 Dana Jansens 2012-06-19 14:21:04 PDT 
Created attachment 148419 [details]
Patch for landing
Comment 11 Dana Jansens 2012-06-19 14:22:09 PDT 
Created attachment 148420 [details]
Patch for landing
Comment 12 Dana Jansens 2012-06-19 14:23:49 PDT 
Created attachment 148422 [details]
Patch for landing
Comment 13 Dana Jansens 2012-06-19 14:23:55 PDT 
Learning about webkit-patch land-safely :|
Comment 14 WebKit Review Bot 2012-06-19 15:19:04 PDT 
Comment on attachment 148422 [details]
Patch for landing

Clearing flags on attachment: 148422

Committed r120759: <http://trac.webkit.org/changeset/120759>
Comment 15 WebKit Review Bot 2012-06-19 15:19:09 PDT 
All reviewed patches have been landed.  Closing bug.