[Webkit-unassigned] [Bug 88482] [Chromium] Compositor should avoid drawing quads when cached textures are available and contents unchanged

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=88482





--- Comment #23 from Adrienne Walker <enne at google.com>  2012-06-13 14:15:37 PST ---
(From update of attachment 147153)
View in context: https://bugs.webkit.org/attachment.cgi?id=147153&action=review

>> Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.cpp:294
>> +    }
> 
> As per offline I'm not sure about this being in CCLayerImpl since it depends on state from calcDrawEtc. But we do have lots of other getters like this, all the drawFoo() ones so maybe my feeling is misplaced.

I don't really like the potential O(n^2) walk through layers.

Could calcDraw do the correct surface property changed propagation (or conversion to recursive layer property changed) as it determines what gets a surface and what doesn't? Or, is there some other way to maybe more explicitly propagate these flags?

> Source/WebCore/platform/graphics/chromium/cc/CCRenderPassDrawQuad.h:47
>      const CCRenderPass* renderPass() const { return m_renderPass; }
> +    CCRenderSurface* targetRenderSurface() const { return m_targetRenderSurface; }
>      bool isReplica() const { return m_isReplica; }
>      unsigned maskTextureId() const { return m_maskTextureId; }
> +    bool contentsChanged() const { return m_contentsChanged; }

I think this is the wrong direction to be going in for CCRenderPassDrawQuad; we should have fewer pointers in quads and not more.  In the future, I'd like to change the CCRenderPass pointer to just be an integer index into the render pass list for the frame.

I don't think either of these members should be added.  If you need the target render surface, get it from the pass.  If you need to know if contents have changed, you can get that from either the pass or the surface.  The quad doesn't need to know if its contents have changed or not in order to draw, that's really a concern of a higher level (CCLayerTreeHostImpl as the owner of the list of the passes).

> Source/WebCore/platform/graphics/chromium/cc/CCRenderSurface.h:183
> +    // FIXME: When the relationships between CCDamageTracker/CCRenderPass/CCRenderSurface/CCRenderPasDrawQuad
> +    // are fixed, this flag may go away. Right now needed to transfer the information from CCDamageTracker
> +    // to CCRenderPassDrawQuad without creating new class dependencies.

Can you explain what the eventual goal is here, possibly with a diagram of who talks to who?

If you remove m_contentsChanged from the quad, then it seems to me that there's no extra dependency, and maybe no need for this extra member?

> Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp:2398
> +TestCase removeRenderPassesCases[] =

Neat!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.