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

VERIFIED FIXED in M14

Status

()

Core
XUL
P3
major
VERIFIED FIXED
19 years ago
19 years ago

People

(Reporter: Chris Waterson, Assigned: Eric Vaughan)

Tracking

({perf})

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [PDT+]have fix)

Attachments

(1 attachment)

(Reporter)

Description

19 years ago
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).
(Reporter)

Comment 1

19 years ago
Created attachment 5674 [details] [diff] [review]
remove all the 'overflow:hidden' rules from boxes and kin
(Reporter)

Comment 2

19 years ago
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.
(Reporter)

Updated

19 years ago
Keywords: perf
(Reporter)

Comment 3

19 years ago
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.
Keywords: beta1
Whiteboard: fix ready
(Assignee)

Comment 4

19 years ago
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.
Status: NEW → ASSIGNED

Comment 5

19 years ago
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.
Whiteboard: fix ready → [NEED INFO]fix ready
(Reporter)

Comment 6

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

Comment 7

19 years ago
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?
(Assignee)

Comment 8

19 years ago
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.
Whiteboard: [NEED INFO]fix ready → have fix
(Assignee)

Comment 9

19 years ago
targeting
Target Milestone: M14

Comment 10

19 years ago
evaughn, please get approval from jar before ok to check in.
Whiteboard: have fix → [PDT+]have fix
(Assignee)

Comment 11

19 years ago
Fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED

Comment 12

19 years ago
marking verified (on pure faith)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.