Bug 24436 - With ACCELERATED_COMPOSITING turned on ASSERT in RenderLayer::updateClipRects()
Summary: With ACCELERATED_COMPOSITING turned on ASSERT in RenderLayer::updateClipRects()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-06 15:46 PST by Chris Marrin
Modified: 2009-03-24 14:03 PDT (History)
1 user (show)

See Also:

Attachments
Test case showing bug (3.49 KB, text/html)
2009-03-06 15:47 PST, Chris Marrin 		no flags Details
Patch (2.87 KB, patch)
2009-03-23 17:20 PDT, Simon Fraser (smfr) 		hyatt: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Marrin 2009-03-06 15:46:16 PST 
Turn on ACCELERATED_COMPOSITING, build debug and run the attached test case. You should see an assert at:

    RenderLayer.cpp:2432
Comment 1 Chris Marrin 2009-03-06 15:47:00 PST 
Created attachment 28374 [details]
Test case showing bug
Comment 2 Simon Fraser (smfr) 2009-03-23 17:11:42 PDT 
The problem here is that we use different clip rect roots for painting, and for hit testing. For painting, every composited layer is a clipping root. For hit testing (which happens in software), only elements with transforms are clipping roots.

For now, I think the best solution is to compute temporary clip rects any time we're hit testing in a page with composited content.
Comment 3 Simon Fraser (smfr) 2009-03-23 17:20:26 PDT 
Created attachment 28879 [details]
Patch
Comment 4 Darin Adler 2009-03-23 17:34:25 PDT 
Comment on attachment 28879 [details]
Patch

Is this going to increase the stack size used by hitTestLayer? How big is ClipRects?

I don't think it's good to name a boolean "temporaryClipRects". That sounds like the name of a variable that stores somre rects in it.
Comment 5 Simon Fraser (smfr) 2009-03-23 18:27:53 PDT 
(In reply to comment #4)
> (From update of attachment 28879 [details] [review])
> Is this going to increase the stack size used by hitTestLayer? How big is
> ClipRects?

ClipRects is 3 * sizeof(IntRect) + 4 bytes.

I think it's OK to use more stack space for hit testing transformed elements, since they are uncommon and having deeply nested ones will be very rare.

> I don't think it's good to name a boolean "temporaryClipRects". That sounds
> like the name of a variable that stores somre rects in it.

Agreed, I can fix.
Comment 6 Dave Hyatt 2009-03-24 11:44:34 PDT 
Comment on attachment 28879 [details]
Patch

r=me
Comment 7 Simon Fraser (smfr) 2009-03-24 14:03:47 PDT 
http://trac.webkit.org/changeset/41951