[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