Bug 67221 - [chromium] Fix scissor rects on clipped nested iframes
Summary: [chromium] Fix scissor rects on clipped nested iframes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adrienne Walker
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-30 12:29 PDT by Adrienne Walker
Modified: 2011-09-01 10:57 PDT (History)
4 users (show)

See Also:


Attachments
Patch (6.08 KB, patch)
2011-08-30 12:33 PDT, Adrienne Walker
no flags Details | Formatted Diff | Diff
Patch (6.05 KB, patch)
2011-08-31 10:59 PDT, Adrienne Walker
jamesr: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>