[Webkit-unassigned] [Bug 88482] [Chromium] RenderSurface for opacity with descendentDrawsContent is slow when large

[bugzilla-daemon at webkit.org]

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





--- Comment #6 from Dana Jansens <danakj at chromium.org>  2012-06-08 14:31:12 PST ---
View in context: https://bugs.webkit.org/attachment.cgi?id=146612&action=review

Some preliminary comments. I see you uploaded another patch already, so I thought I'd publish these as I'm distracted with my machine for a bit.

> Source/WebCore/platform/graphics/chromium/cc/CCDamageTracker.cpp:249
> +    // This method assumes that the layer either does not own a surface at all, or owns
> +    // our target surface. In all other cases, extendDamageForRenderSurface must be called instead.

I guess this is a long way of saying don't call this for layers that own surfaces that contribute to the target surface.

> Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.cpp:278
> +bool CCLayerImpl::layerTargetSurfaceDirty() const

Since this is only called from damage tracker, can it be done there? Maybe file-static method if you like? Can the name be better so we don't need the verbose comment below?

> Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.h:317
> +    // affect the pixels on its target surface, but would require redrawing

"on its target surface" -> "on its own surface" ?

> Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.h:318
> +    // of the target surface of this layer's target surface

"redrawing the contents of the layer's surface in its target" ?

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:231
> +        renderSurface->setContentsChanged(!renderSurface->damageTracker()->currentDamageRect().isEmpty());

I'm not sure why we need this bool on the RenderSurface. It can already see the rect is empty as it looks up here.

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:422
> +void CCLayerTreeHostImpl::removeRedundantPasses(CCRenderPassList& passes)

I think this could have a better name. "Redundant" doesn't explain what this function is doing really - why are they redundant?

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