121264 - gtk2 should use the region for invalidation, not the rect

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Christopher Blizzard (:blizzard)]

gtk2 should be using the region for invalidation, not the rect supplied.  This
could make things a good bit faster.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Are you talking about invalidation or painting?

You can already pass a region to be painted as a parameter to an NS_PAINT event.
I once had nsViewManager patches to use it instead of the rect if a non-null
region was provided; it wasn't hard to do. Unfortunately the patches couldn't be
just be applied because certain platforms (*cough* gtk *cough*) left garbage in
the region field of the event, so I couldn't reliably tell if it had been
initialized. But we can audit all the gfx implementations and fix them up if
necessary.

Comment 2

[Christopher Blizzard (:blizzard)]

I'm talking about using the region field of the NS_PAINT event.  It is reliably
used in gtk2.  It's also relatively trivial to fix it if it's busted with gtk1.

Comment 3

[Christopher Blizzard (:blizzard)]

roc, is there a bug open on the region code in the view manager that I can link
to with this bug?

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

No. We should have separate bugs for "make toolkit X set region correctly" (for
each X) and "use paint region in view manager".

Comment 5

[Brian Ryner (not reading)]

Attachment: patch

Comment 6

[Christopher Blizzard (:blizzard)]

Comment on attachment 159624 [details] [diff] [review]
patch

Woo!

Comment 7

[Brian Ryner (not reading)]

Attachment: patch without global update region

dbaron and I talked about this some and we're not sure that having a cached
update region is really a win here.  You do avoid a call to CreateInstance, but
there's additional overhead from managing the cached region, and it has to
delete the underlying GdkRegion in that region every time as well.  I think we
should probably go with the simpler approach.

Comment 8

[Brian Ryner (not reading)]

checked in

Comment 9

[Christopher Blizzard (:blizzard)]

I'm pretty sure this just broke my build.  Using testgtkembed.

Comment 10

[Christopher Blizzard (:blizzard)]

Attachment: stack trace

Looks like there's some hard-coded knowledge on the rect element.

Comment 11

[Brian Ryner (not reading)]

Attachment: fix crash

Comment 12

[Brian Ryner (not reading)]

follow-up patch checked in.