Bug 67221

Summary: [chromium] Fix scissor rects on clipped nested iframes
Product: WebKit Reporter: Adrienne Walker <enne>
Component: New BugsAssignee: Adrienne Walker <enne>
Status: RESOLVED FIXED    
Severity: Normal CC: enne, jamesr, kbr, vangelis
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch jamesr: review+

Description Adrienne Walker 2011-08-30 12:29:39 PDT
[chromium] Fix scissor rects on clipped nested iframes
Comment 1 Adrienne Walker 2011-08-30 12:33:22 PDT
Created attachment 105669 [details]
Patch
Comment 2 Adrienne Walker 2011-08-30 12:42:42 PDT
See: http://code.google.com/p/chromium-os/issues/detail?id=19635
Comment 3 Kenneth Russell 2011-08-30 14:55:57 PDT
Comment on attachment 105669 [details]
Patch

Vangelis, could you unofficially review this?
Comment 4 Vangelis Kokkevis 2011-08-30 22:03:45 PDT
Comment on attachment 105669 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=105669&action=review

Good catch!

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:264
>                  scissor.intersect(layer->scissorRect());

Or you could simply remove the if() and if layer->scissorRect().isEmpty() then the intersection will be empty too.
Comment 5 Vangelis Kokkevis 2011-08-30 22:05:38 PDT
(In reply to comment #4)
> (From update of attachment 105669 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=105669&action=review
> 
> Good catch!
> 
> > Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:264
> >                  scissor.intersect(layer->scissorRect());
> 
> Or you could simply remove the if() and if layer->scissorRect().isEmpty() then the intersection will be empty too.

Hmm, unless of course we're worried about scissor rects that due to the transform applied to them end up with negative sizes.  Maybe the code you have is safer.
Comment 6 Adrienne Walker 2011-08-31 09:26:18 PDT
> (In reply to comment #4)
> > (From update of attachment 105669 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=105669&action=review
> > 
> > Good catch!
> > 
> > > Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:264
> > >                  scissor.intersect(layer->scissorRect());
> > 
> > Or you could simply remove the if() and if layer->scissorRect().isEmpty() then the intersection will be empty too.
> 
> Hmm, unless of course we're worried about scissor rects that due to the transform applied to them end up with negative sizes.  Maybe the code you have is safer.

If we have IntRects with negative sizes, I think we'll be in more trouble than just here.  I agree with your original comment and will simplify this.  :)
Comment 7 Adrienne Walker 2011-08-31 10:59:18 PDT
Created attachment 105794 [details]
Patch
Comment 8 Adrienne Walker 2011-09-01 10:06:25 PDT
(In reply to comment #7)
> Created an attachment (id=105794) [details]
> Patch

Can I get an unofficial (or official) review whenever one of y'all has time? I would love to land this before the m15 branch.  :)
Comment 9 Vangelis Kokkevis 2011-09-01 10:14:40 PDT
Comment on attachment 105794 [details]
Patch

unofficial r+ from me!
Comment 10 Adrienne Walker 2011-09-01 10:57:50 PDT
Committed r94315: <http://trac.webkit.org/changeset/94315>