240274 - nsRenderingContextGTK::PopState calls nsRegionGTK::IsEmpty too much

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Gtk, defect)

Description

[Boris Zbarsky [:bzbarsky]]

PopState() takes a boolean arg that almost all callers ignore.  Setting this arg
requires a call to nsRegionGTK::IsEmpty, typically.  This is about 10% of the
time spent in PopState.

I'd think that this arg should become an (optional) PRBool* arg; then callers
can pass null if they don't care about the clip.

Comment 1

[tor]

SetClipRect() also has an oft-ignored boolean arg that causes IsEmpty()
to be called.

Comment 2

[tor]

I went through the tree and couldn't find any callers that used the
arg returned from PopState() - I'll attach the patch tomorrow.

Comment 3

[tor]

Attachment: remove PopState() argument

Comment 4

[tor]

Attachment: resync PopState() patch to tree

Comment 5

[tor]

PopState() patch checked in.

Comment 6

[tor]

Attachment: remove argument of SetClip{Rect,Region}

Comment 8

[tor]

Attachment: remove unrelated nsGCCache changes from previous patch

Comment 9

[tor]

Checked in.

Comment 10

[R.K.Aa.]

Attachment: png showing regression

After this checkin: When i mouse over items on personal toobar, the black line
under them vanishes. When i scroll windows, black lines draw over images, and
white lines over the fonts. Not so good.

Testing with a current trunk cvs build, Linux.
I backed out the 40 or so files this checkin touched, and the bug vanished.
I rolled the changes back in again, and the bug re-surfaced.

Building with gcc version 3.3.3 and 
--enable-default-toolkit=gtk2 --disable-accessibility --disable-postscript
--disable-ldap --disable-debug --disable-tests --disable-logging
--enable-crypto --disable-bidi --enable-optimize=-O2 --disable-jsd
--disable-mathml

Comment 11

[tor]

Oops, accidentally checked in a one-line perf test change that causes
these regressions.  I've backed that out of the tree.