Bug 91124 – [chromium] The root layer should not try create a second RenderSurface for itself

• Component: New Bugs
• Status: RESOLVED FIXED

[Dana Jansens]

[chromium] The root layer should not try create a second RenderSurface for itself

[Dana Jansens]

Created attachment 152009 [details]
Patch

[Adrienne Walker]

Comment on attachment 152009 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=152009&action=review

R=me.

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:264
> +    // The root layer always has a surface already.

It's not that it has a surface already, it's that its surface is special (has a pre-set size, doesn't get a texture, is already in the surface list) so it doesn't go down the "separate surface" path.

[Dana Jansens]

(In reply to comment #2)
> (From update of attachment 152009 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=152009&action=review
> 
> R=me.
> 
> > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:264
> > +    // The root layer always has a surface already.
> 
> It's not that it has a surface already, it's that its surface is special (has a pre-set size, doesn't get a texture, is already in the surface list) so it doesn't go down the "separate surface" path.

Hm, kinda disagree. It has a RenderSurface object on it already, but agree I can word this better.

[Dana Jansens]

Created attachment 152016 [details]
Patch for landing

[Shawn Singh]

I agree more with Enne's original wording... to me, that wording pointed out the idiosyncrasies in the current code that motivate putting this if-statement here.  It might be true that the renderSurface existed already, but that's something we can (and may want to) change down the road.

Actually on the latest patch for landing, I like the comment wording even less.  It seems a bit misleading in my opinion.  I personally think the right thing to say here is to explicitly acknowledge that its a deviation from the intended clean concept.

something like: "even though technically the root layer's subtree does render to a separate surface, we want to force the root layer to go down the non-surface code path because it shares more common code with that case since the root surface is already expected to be initialized by outside code. This kink could eventually be fixed by reducing the special-ness of the root layer and its surface."

I know its wordy... but that's the spirit of the comment I think is appropriate =)

[Dana Jansens]

This is why we shouldn't have comments :P

[Dana Jansens]

Created attachment 152067 [details]
Patch

[Dana Jansens]

Committed r122511: <http://trac.webkit.org/changeset/122511>