Bug 33361

Summary: WebGL canvas paints background color twice
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Layout and RenderingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarrin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
Testcase
none
Patch mitz: review+

Description Simon Fraser (smfr) 2010-01-07 20:38:02 PST
A WebGL canvas with a CSS background color with alpha will render that background color twice. If the canvas has rounded corners, one layer of background color will show through at the corners.
Comment 1 Simon Fraser (smfr) 2010-01-08 13:48:46 PST
Created attachment 46157 [details]
Testcase
Comment 2 Simon Fraser (smfr) 2010-01-08 13:50:46 PST
Reopening, since this will be an issue once the patch in bug 33361 is landed.

The issue is that, for a directly composited WebGL layer, HTMLCanvasElement::paint() may never be called, so the beginPaint()/reshape()/endPaint() calls will never be called on the WebGLRenderingContext.

Now, some style change can happen that requires that the canvas's layer gets backing store and paints (say, a border is added). Then this method will call WebGLRenderingContext::reshape(), which causes the canvas to be cleared.
Comment 3 Simon Fraser (smfr) 2010-01-08 13:51:41 PST
That last comment was meant for bug 33392.
Comment 4 Simon Fraser (smfr) 2010-01-08 17:56:49 PST
Created attachment 46185 [details]
Patch
Comment 5 mitz 2010-01-08 18:35:49 PST
Comment on attachment 46185 [details]
Patch

> +        * rendering/RenderLayerBacking.h:
> +        * rendering/RenderLayerBacking.cpp:
> +        (WebCore::is3DCanvas):
> +        (WebCore::RenderLayerBacking::RenderLayerBacking):
> +        (WebCore::RenderLayerBacking::updateGraphicsLayerConfiguration):
> +        (WebCore::RenderLayerBacking::updateGraphicsLayerGeometry):
> +        (WebCore::hasBoxDecorationsOrBackgroundImage):
> +        (WebCore::RenderLayerBacking::isSimpleContainerCompositingLayer):
> +        (WebCore::RenderLayerBacking::hasNonCompositingContent):
> +        (WebCore::RenderLayerBacking::hasPaintedContent):
> +        (WebCore::RenderLayerBacking::isDirectlyCompositedImage):
> +        (WebCore::RenderLayerBacking::rendererContentChanged):

Per-function change log comments are encouraged.

>  static bool hasBoxDecorations(const RenderStyle*);
> -static bool hasBoxDecorationsWithBackgroundImage(const RenderStyle*);
> +static bool hasBoxDecorationsOrBackgroundImage(const RenderStyle*);

The naming here is really unfortunate, because it gives “box decoration” conflicting meanings. In hasBoxDecorations(), a background color counts as “box decoration”, but in hasBoxDecorationsOrBackgroundImage() it doesn’t. Your rename doesn’t make things better, just different.

> +// An image can be directly compositing if it's the sole content fo the layer, and has no box decorations

Typo: “fo”

> +    // Returns true if this layer has content that needs to be rendered by painting into the backing store.
> +    bool hasPaintedContent() const;

Not a fan of this name (sounds like this function tells you whether the layer has painted its content already), but I don’t have an alternative suggestion.

r=me
Comment 6 Simon Fraser (smfr) 2010-01-08 20:08:21 PST
Landed in http://trac.webkit.org/changeset/53034 with naming cleanup.