[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 #7 from zlieber at chromium.org  2012-06-08 20:13:22 PST ---
Thanks for comments; some responses below.

> 
> > 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?

It sure could, but this answers a pretty generic question using only class's own data, so seemed natural to be on the layer. The question is how generic that question is - maybe a different name will make this better? layerTargetSurfaceRequiresRedrawing/ContentsChanged/PixelsChanged?

> 
> > 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" ?

Sounds good, also need to add something about layers that don't have surfaces.

> 
> > 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" ?

Same here.

> 
> > 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.

Yeah I really did want to put this in, but couldn't see how the surface can look at this rect without talking to damage tracker directly. Is there a way? 

If not then this flag can go after we fix the quad creation and relationship between these classes and occlusion tracker, so the flag can be set on the quad externally.

> 
> > 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?

removePassesWithCachedTextures?

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