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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 12 14:01:19 PDT 2012


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





--- Comment #18 from zlieber at chromium.org  2012-06-12 14:01:19 PST ---
(In reply to comment #16)
> (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 =)

My point here was that I was adding a documentation to a field, but ended up with a description of an algorithm that is said to belong somewhere else. How can we say the algorithm belongs somewhere else if it follows naturally from the plain description of a field it operates on?

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



More information about the webkit-unassigned mailing list