[webkit-reviews] review granted: [Bug 91260] [chromium] Remove incorrect debug assertion in LayerRendererChromium.cpp : [Attachment 152284] Patch

[bugzilla-daemon at webkit.org]

Adrienne Walker <enne at google.com> has granted Shawn Singh
<shawnsingh at chromium.org>'s request for review:
Bug 91260: [chromium] Remove incorrect debug assertion in
LayerRendererChromium.cpp
https://bugs.webkit.org/show_bug.cgi?id=91260

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

------- Additional Comments from Adrienne Walker <enne at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=152284&action=review


R=me, with comment nits.

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:703
> +    // (Note, however, that it may still have perspective components in the
2d parts of the matrix.)

I don't know that it's important to add this superfluous comment about
perspective here.  Somebody modifying this code in the future doesn't need to
be made aware of this extra detail just because this comment was wrong in the
past.

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:884
> +	   // (Note, however, that it may still have perspective components in
the 2d parts of the matrix.)

Same with this perspective comment.

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:-885
> -	   ASSERT(!clipped);

I do think you need a comment about why it's ok that this quad is clipped,
since we assert on clipped pretty much everywhere else.