[webkit-reviews] review denied: [Bug 80871] Reuse buffer allocation if canvas size does not change : [Attachment 132258] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Mar 16 13:02:40 PDT 2012


Jeff Timanus <twiz at chromium.org> has denied Sami Kyostila
<skyostil at google.com>'s request for review:
Bug 80871: Reuse buffer allocation if canvas size does not change
https://bugs.webkit.org/show_bug.cgi?id=80871

Attachment 132258: Patch
https://bugs.webkit.org/attachment.cgi?id=132258&action=review

------- Additional Comments from Jeff Timanus <twiz at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=132258&action=review


I'm not extremely familiar with the interaction between the various
notifications.

The changes look good in general.  Just an observation about where the
selective clearing logic should be placed.

> Source/WebCore/html/HTMLCanvasElement.cpp:258
> +    if (hadImageBuffer && oldSize == IntSize(w, h) && (!m_context ||
m_context->is2d())) {

Under which circumstances will m_context not be assigned?

Is it possible to put all of this logic in HTMLCanvasElement::setSurfaceSize? 
The notifications below could be skipped if old-size == new-size.

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:138
> +	       while (stackSize--)

Was this a previously glaring error?  Good catch!


More information about the webkit-reviews mailing list