Bug 89474 - [chromium] Use CCRenderPassTextureAllocator in LayerRendererChromium
Summary: [chromium] Use CCRenderPassTextureAllocator in LayerRendererChromium
Status: RESOLVED INVALID
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: 88924 89485
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-19 08:36 PDT by Dana Jansens
Modified: 2012-06-27 13:55 PDT (History)
6 users (show)

See Also:


Attachments
Patch (23.21 KB, patch)
2012-06-19 08:40 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (49.85 KB, patch)
2012-06-20 15:26 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-06-19 08:36:35 PDT
[chromium] Use CCRenderPassTextureAllocator in LayerRendererChromium
Comment 1 Dana Jansens 2012-06-19 08:40:47 PDT
Created attachment 148343 [details]
Patch
Comment 2 Dana Jansens 2012-06-19 08:41:07 PDT
This is what it looks like when hooking up the RenderPassTextureAllocator
Comment 3 Dana Jansens 2012-06-20 08:55:50 PDT
Comment on attachment 148343 [details]
Patch

misclick ews :< the button is so big!
Comment 4 WebKit Review Bot 2012-06-20 09:43:28 PDT
Comment on attachment 148343 [details]
Patch

Attachment 148343 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13003243
Comment 5 Dana Jansens 2012-06-20 15:26:27 PDT
Created attachment 148666 [details]
Patch
Comment 6 Adrienne Walker 2012-06-21 12:43:45 PDT
Comment on attachment 148666 [details]
Patch

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

> Source/WebCore/platform/graphics/chromium/cc/CCRenderPassTextureAllocator.cpp:125
> +void CCRenderPassTextureAllocator::decideAllocationsForFrame(const CCRenderPassList& renderPassesInDrawOrder, const IntSize& deviceViewportSize, size_t memoryLimitBytes)

How does this function interact with removing cached frames and damage?

(1) Considering some surfaces "in use" when they're not actually going to get drawn will cause us to overestimate our high water mark of texture usage and toss out more textures than we actually need to. For example, assume you have a super deep dependency subtree of render passes, but the one at the top has no damage.  The removing cached passes function occurs after this and would remove all but the top.  However, the only textures that you'd need to save would be the top one and all of its direct dependencies.  This may be way less than the amount of in use textures.

(2) Without considering damage, are you possibly considering saving textures that we need to completely redraw? (I suppose we could consider doing partial surface updates.)

(3) Without considering damage, are you possibly considering saving textures that aren't going to get drawn, because we've saved some ancestor texture that depends on it?

PS Would it make sense to just have render passes know explicitly about their dependent render passes, rather than having to paw through the quad list?
Comment 7 Dana Jansens 2012-06-21 14:45:54 PDT
(In reply to comment #6)
> (From update of attachment 148666 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148666&action=review
> 
> > Source/WebCore/platform/graphics/chromium/cc/CCRenderPassTextureAllocator.cpp:125
> > +void CCRenderPassTextureAllocator::decideAllocationsForFrame(const CCRenderPassList& renderPassesInDrawOrder, const IntSize& deviceViewportSize, size_t memoryLimitBytes)
> 
> How does this function interact with removing cached frames and damage?

There's no notion of damage in the RenderPass list right now so it is ignoring the whole idea. I'm planning to add the contentsDamaged() flag in there somewhere so it can drop textures from the previous frame that are damaged immediately and they don't cost budget.

> (1) Considering some surfaces "in use" when they're not actually going to get drawn will cause us to overestimate our high water mark of texture usage and toss out more textures than we actually need to. For example, assume you have a super deep dependency subtree of render passes, but the one at the top has no damage.  The removing cached passes function occurs after this and would remove all but the top.  However, the only textures that you'd need to save would be the top one and all of its direct dependencies.  This may be way less than the amount of in use textures.

Yes. I think we would like to save all the textures if we can, as it allows us to recover optimally from any damage, but they should not count as being needed, and should be dropped more easily.

> (2) Without considering damage, are you possibly considering saving textures that we need to completely redraw? (I suppose we could consider doing partial surface updates.)

I am thinking about this, and perhaps reusing textures more aggressively for different surfaces to avoid texture delete/create.

> (3) Without considering damage, are you possibly considering saving textures that aren't going to get drawn, because we've saved some ancestor texture that depends on it?

Not /because/ of it, but I think we should save as many textures, overall, as fit within our budget.

> PS Would it make sense to just have render passes know explicitly about their dependent render passes, rather than having to paw through the quad list?

Given the size of the quad lists, I'm not sure..