28506 - don't set cliprect when drawing a box's children

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[Chris Waterson]

As per our profiling binge described here:

news://news.mozilla.org/38AC5AAA.8E4390B3%40netscape.com

We discovered this accounted for 10% of the time it take resize the chrome. 
Troy had a suggestion here:

news://news.mozilla.org/38AC6698.D83181AB%40netscape.com

that may be a simpler alternative to the work that you and I described 
(changing box to derive from something other than container).

Comment 1

[Chris Waterson]

Attachment: remove all the 'overflow:hidden' rules from boxes and kin

Comment 2

[Chris Waterson]

The above patch "fixes" the style rules for <window>, <toolbar>, <toolbox>, and 
some degenerate inline style to make sure that the cliprect is set 
inappropriately. (I've verified by running the app with a breakpoint set in 
nsContainerFrame::PaintChildren's call to SetClipRect, and haven't tripped it 
once with these patches.)

The changes *seem* safe; i.e., everything seems to work allright. I'm gonna run 
with them for a day or so to make sure.

Comment 3

[Chris Waterson]

PDT: I think we should consider taking this change. It's a 10% speedup for 
window resize; hard to quantify in terms other than, "yeah, it feels a bit 
faster." The change is straightforward (fixing some style rules), and is easy 
to back out or patch if we find it regresses things. FWIW, normal banging on 
mail and browser don't seem to illustrate any obvious problems.

Comment 4

[Eric Vaughan]

It turns our in boxes there are time when you do need to clip. One example is 
the tool bar. When it is squeezed too much the toolbox will continue to get 
smaller and need to clip the toolbar. So I have actually come up with a fix that 
does not involve style rule. Instead the box if overflow hidden is set will only 
clip if its one or more of its children is outside its bounds. This removes 99% 
of push and pops of clipping.

Comment 5

[leger]

Not ready to PDT + this yet.  Are we taking waterson's changes or expanding on 
the fix (this would make us nervous)?  Also, more people need to be testing 
before check in.

Comment 6

[Chris Waterson]

N.B. that my changes are *incorrect*. Ignore them!

Comment 7

[rickg]

PDT is very anxious to have the performance of the app improve beta, but we're 
also concerned about risk. Can we get more data about the benefits/risk of this 
fix?

Comment 8

[Eric Vaughan]

This fix removes 90% of set and pop clips. Its done by reimplementing the 
PaintChildren method in nsBoxFrame to do one additional thing. That is if it is 
supposed to clip it checks to see whether the child is outside of its bounds. If 
it isn't it does not clip. I have tested it on mac, windows, and linix. It 
provides a 10% speed increase on windows and possibly more on linix because 
clipping is so expensive there. Its quite safe and only affect xul boxes.

Comment 9

[Eric Vaughan]

targeting

Comment 10

[leger]

evaughn, please get approval from jar before ok to check in.

Comment 11

[Eric Vaughan]

Fixed

Comment 12

[Paul MacQuiddy]

marking verified (on pure faith)