Bug 42852 – REGRESSION: box shadows on WebGL canvas elements repainted every frame

• Component: WebGL
• Status: RESOLVED FIXED

[James Robinson]

Try adding a large box shadow to the WebGL spinning box example.  The framerate drops because the shadow is re-drawn every frame.  Only the WebGL's contents should be invalidated, not anything on the canvas itself.

[Simon Fraser (smfr)]

Regression from bug 34719.

[Simon Fraser (smfr)]

The HTMLCanvasElement::willDraw() call from WebGLRenderingContext::markContextChanged() is causing the canvas's renderer to get redrawn whenever the WebGL does stuff, which is very bad for performance.

[Simon Fraser (smfr)]

I don't understand the need for willDraw(), and the "Make sure the canvas's image buffer is allocated." comment. The canvas image buffer should only be created on demand.

[James Robinson]

Created attachment 62351 [details]
Patch

[James Robinson]

This patch appears to fix the regression and still passes fast/canvas/webgl/canvas-test.html.  I'm not sure why we have to call willDraw() at all before doing the paintRenderResultsToCanvas() though so I haven't marked the patch review?.

[Chris Marrin]

The code in WebGLRenderingContext::markContextChanged() looks wrong. In the accelerated compositing case the image buffer should never be allocated and the canvas should not be marked dirty (in the willDraw() call). This could be as simple as adding a #else clause to the HW_COMP test to do this only when not hardware accelerated.

[James Robinson]

Created attachment 62354 [details]
Patch

[James Robinson]

Created attachment 62361 [details]
Patch

[James Robinson]

This patch reverts WebGLRenderingContext::markContextChanged() mostly to its previous behavior.  The changes relative to the pre-r63502 version are:

- unconditionally mark m_markedCanvasDirty so if we later need to do a readback we'll know the canvas is dirty
- remove the unnecessary call to HTMLCanvasElement::buffer() and incorrect comment

The call to ::willDraw() is still needed for the software rendering path.

The other change is to add a call to ImageBuffer::clearImage right before calling GraphicsContext3D::paintRenderingResultsToCanvas in the readback path.  I don't think this should be needed, but without this change canvas-test.html fails.  I think it's better to leave this change in with a FIXME for now and try to figure out what is really going on.

I've verified that this passes fast/canvas/webgl/* and renders WebGL content correctly in Safari and Chrome on a Snow Leopard machine.

[Kenneth Russell]

Comment on attachment 62361 [details]
Patch

Looks good to me. Thanks for fixing this.

[James Robinson]

Comment on attachment 62361 [details]
Patch

Clearing flags on attachment: 62361

Committed r63923: <http://trac.webkit.org/changeset/63923>

[James Robinson]

All reviewed patches have been landed.  Closing bug.