Summary: | REGRESSION: box shadows on WebGL canvas elements repainted every frame | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | James Robinson <jamesr> | ||||||||
Component: | WebGL | Assignee: | Zhenyao Mo <zmo> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cmarrin, kbr, simon.fraser, zmo | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
James Robinson
2010-07-22 15:03:52 PDT
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. 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. Created attachment 62351 [details]
Patch
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?. 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. Created attachment 62354 [details]
Patch
Created attachment 62361 [details]
Patch
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. Comment on attachment 62361 [details]
Patch
Looks good to me. Thanks for fixing this.
Comment on attachment 62361 [details] Patch Clearing flags on attachment: 62361 Committed r63923: <http://trac.webkit.org/changeset/63923> All reviewed patches have been landed. Closing bug. |