[webkit-reviews] review denied: [Bug 39027] http://philip.html5.org/tests/canvas/suite/tests/2d.composite.uncovered.fill.source-in.html fails : [Attachment 92671] Updated patch with tests passing

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri May 13 17:05:38 PDT 2011


James Robinson <jamesr at chromium.org> has denied Julien Chaffraix
<jchaffraix at webkit.org>'s request for review:
Bug 39027:
http://philip.html5.org/tests/canvas/suite/tests/2d.composite.uncovered.fill.so
urce-in.html fails
https://bugs.webkit.org/show_bug.cgi?id=39027

Attachment 92671: Updated patch with tests passing
https://bugs.webkit.org/attachment.cgi?id=92671&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=92671&action=review

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:945
> +	   // FIXME: CompositeSourceAtop should follow the same behavior
however I am not sure
> +	   // our test coverage is enough.

i'm not sure exactly what this FIXME means.  should this reference a webkit
bug?

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:947
> +	   if (state().m_globalComposite == CompositeSourceIn
> +	       || state().m_globalComposite == CompositeSourceOut) {

this should be on one line

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1068
> +    // FIXME: CompositeSourceAtop should follow the same behavior however I
am not sure
> +    // our test coverage is enough.
> +    if (state().m_globalComposite == CompositeSourceIn
> +	   || state().m_globalComposite == CompositeSourceOut) {

same comments as above

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1077
> +	   FloatRect canvasRect(0, 0, canvas()->width(), canvas()->height());
> +	   canvasRect = state().m_transform.inverse().mapRect(canvasRect);
> +
> +	   c->save();
> +	   c->clipOut(enclosingIntRect(rect));
> +	   c->setCompositeOperation(CompositeClear);
> +	   c->fillRect(canvasRect);
> +	   c->restore();

this smells like bad copy-pasta.  can you refactor this into a helper?


More information about the webkit-reviews mailing list