Bug 91260

Summary: [chromium] Remove incorrect debug assertion in LayerRendererChromium.cpp
Product: WebKit Reporter: Shawn Singh <shawnsingh>
Component: Layout and RenderingAssignee: Shawn Singh <shawnsingh>
Status: RESOLVED FIXED    
Severity: Normal CC: cc-bugs, enne, jamesr, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Shawn Singh 2012-07-13 10:00:06 PDT
Bug originally reported in http://code.google.com/p/chromium/issues/detail?id=136017 ... The assertion is being triggered due to anti-aliasing inflation that causes clipping.  To our knowledge, this scenario of clipping is actually tolerable, and has not yet caused any known bugs.

In addition to fixing the assert, I updated comments that needed to be worded better.

Patch coming in a moment
Comment 1 Shawn Singh 2012-07-13 10:05:38 PDT
Created attachment 152284 [details]
Patch
Comment 2 Adrienne Walker 2012-07-13 10:13:33 PDT
Comment on attachment 152284 [details]
Patch

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.
Comment 3 Shawn Singh 2012-07-13 10:20:52 PDT
Created attachment 152288 [details]
Patch for landing
Comment 4 Shawn Singh 2012-07-13 10:22:15 PDT
Thanks for the review!  I'm happy to tweak the comments more if they're not good enough yet, please feel free to cq- it if needed.
Comment 5 WebKit Review Bot 2012-07-13 12:40:46 PDT
Comment on attachment 152288 [details]
Patch for landing

Clearing flags on attachment: 152288

Committed r122619: <http://trac.webkit.org/changeset/122619>
Comment 6 WebKit Review Bot 2012-07-13 12:40:51 PDT
All reviewed patches have been landed.  Closing bug.