[Webkit-unassigned] [Bug 67750] Create a delegate class to help cleanly isolate the chromium compositor API

[bugzilla-daemon at webkit.org]

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





--- Comment #20 from James Robinson <jamesr at chromium.org>  2011-09-22 19:29:08 PST ---
(In reply to comment #19)
> Well, today several layers share the same delegate, e.g. the transform layer and the content layer of a given GraphicsLayerChromium. Problem is, you don't want to assume the transform layer (which is a LayerChromium) draws anything.
> I've been trying to factor out the delegate from LayerChromium altogether, and move it into a CCContentLayerDelegate, only applicable to ContentLayerChromium, but it's more complicated than it appears, because GLC doesn't know anything about the types of layers (hence no safe down casting), yet wants to set itself as the delegate.

GraphicsLayerChromium does know about ContentLayerChromium, and provides itself as the delegate here:
http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp#L77

I like the idea of having a separate delegate type for providing content than for general CCLayer delegation.  notifySyncRequired() applies to all layer types, and once we hook up animation stuff we'll need a set of callbacks on the layer delegate for that, but paintContents() is definitely something that is only relevant for content layers.  This could be a subclass of CCLayerDelegate or an orthogonal type that's passed in to ContentLayerChromium::create().  Image/Video/WebGL/etc layers wouldn't need this.



> 
> But in general, I agree with the premise that drawsContent should have nothing to do with LayerChromium in general.

At the GraphicsLayer interface drawsContent only applies to content layers.  In our implementation we use it in a bunch of other cases, though, that are internal to the compositor implementation - for example if you try to make an mask layer that's crazy sized, when we internally decide that drawsContent() is false for that layer.  That shouldn't be part of the external interface but it is something that it handy for our internals.

Maybe it's time to formalize the split between stuff that is on LayerChromium that is public interface and the stuff that is implementation detail we're using in the compositor.  We use friend declarations / protected for some things, like createCCLayerImpl(), but we aren't nearly as careful as we should be.  LayerChromium::pushPropertiesTo() and LayerChromium::id() for instance are 100% internal and shouldn't be in the public header at all.

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