[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 #16 from Shawn Singh <shawnsingh at chromium.org>  2012-06-12 13:55:51 PST ---
(In reply to comment #15)
> (In reply to comment #10)
> > 
> > > Source/WebCore/platform/graphics/chromium/cc/CCDamageTracker.cpp:261
> > > +    if (layerIsNew || layer->layerTargetSurfaceDirty()) {
> > 
> > I think Dana mentioned that it might be worth moving layerTargetSurfaceDirty() logic to the damage tracker as a static helper function.   I agree with that, but for more reasons - in my opinion this functionality is not meaningful outside of damage tracking, and it could be error-prone/confusing/mis-used if we make it available as an accessor on layers.
> 
> > 
> > > Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.h:318
> > > +    // of the target surface of this layer's target surface
> > 
> > "but would require redrawing the targetSurface onto its ancestor targetSurface"
> > Also, the comment should probably also clarify what happens with this flag if the layer does not own a renderSurface.
> 
> Well... the answer to this is "for layers that do not own a surface this flag acts as layerPropertyChanged". But this is exactly the documentation of a function which you & Dana asked me to move to DamageTracker because it doesn't belong in the layer.
> 
> So my question is - how come the algorithm doesn't belong in the layer, but its description does?
> 
> For this patch I already move the function, but we really, really need to consider this strategy.

I'm a little confused... I don't think Dana and I were asking to move this m_layerSurfacePropertyChanged flag to the damage tracker?   so this comment and the explanation of what it represents would still remain here?

We were proposing to move the layerChangesWillAffectTargetSurfaceContents() computation.  I think its reasonable to see it both ways - you see it as a minor translation between existing layer properties which fits on the layer itself, but personally I see it as code that answers a question that only the damage tracker should be asking.

As for reconsidering the design strategy of this code, we should include a larger group of teammates in that discussion.  I suspect any ideal solution will be quite disruptive, and it might be necessary to incrementally evolve towards a better design than to fix it all at once -- which is what we have been doing =)

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