As I explained in my email, GFX-GTK seems to repaint in response to Invalidate()s without checking to see if the invalidated area is actually visible. We should restrict the area to be repainted to the visible area, to the extent we can. We should also pass the paint region to the view manager instead of breaking it up into rects and passing them one at a time. The view manager is quite capable of painting a region intelligently.

OK, I have a patch. It seems to speed up some DHTML (e.g., Viewer demo #13) hugely. But I can't figure out how to tell anything about whether a window is visible at all, let alone what the visible area is. I've tried XWindowAttributes attrs; XGetWindowAttributes(GDK_DISPLAY(), GDK_WINDOW_XWINDOW(aWindow), &attrs); return attrs.map_state == IsViewable; but it seems to return TRUE even when the window is iconized.

BTW the speedup seems to come from only doing one paint instead of processing several paint events, one per rectangle in the region to be updated.

Hmmm.. As a gross hack we could get the root geometry and window geometry and try to produce an exposed rect from that.... That won't help with other windows being on top of our window, however. :( I'd love to test your patch on those evil "opacity on 200 divs" testcases, btw. It sounds to me like it should reduce painting from being O(N^2) in the number of divs involved (which is what it is right now) to O(N)....

XGetWindowAttributes() requires a server round trip. Yuck! Why don't you use the cached coordinates?

Well, the idea is to find out what part of our window is on-screen and not covered by other windows. If we can do that without a server round-trip, great! (I'm not seeing a way to really do the second part even _with_ server round-trips.)

I just wanted to find out if the window is mapped or not, hoping that virtual desktops and window iconification unmap nonvisible windows. But it doesn't even seem to work. BTW is there any documentation anywhere about which X and GTK calls require server round trips? I haven't been able to find any.

There's no way to tell what parts of a window are mapped and what windows are overlapping it, at least not with the current X API. Besides, that information is useless because as soon as you get it it's out of date and someone might move the windows on you. If you want to know what windows are mapped and what windows aren't, it's pretty easy to tell. We might need to start tracking the GDK_MAP event, but that's about it. Oh, and for functions that require an X server round trip, pretty much any function that includes a "Get" or "Query" require a flush and a roundtrip.

> Besides, that information is useless because as soon as you get it it's out of > date and someone might move the windows on you. ... in which case we'd get expose events and we'd take care of it. It turns out there is some information available in the form of VisibilityChanged events which are tracked by the superwin stuff. It keeps a visibility state GDK_VISIBILITY_FULLY_OBSCURED, GDK_VISIBILITY_PARTIAL, and GDK_VISIBILITY_UNOBSCURED. I can use that to detect when a Mozilla window is completely covered. Unfortunately virtual desktop switching and window iconization give GDK_VISIBILITY_PARTIAL instead of GDK_VISIBILITY_FULLY_OBSCURED. I'm not sure why. It turns out that the XGetWindowAttributes that I had does work in this situation if I query mSuperWin->shell_window. I've tried getting mapped status from GTK some other way, but I can't find my way around the maze of superwins, mozareas, shells, nsWindows, gtkwindows, and gdkwindows. I hope GTK2 is cleaner than this :-). Anyway, I'll attach the patch I have, which is still darn useful.

Here ya go. Extra per-platform work will be required to let other platforms take advantage of whatever paint region info they have. This patch will still work on all platforms though, if there's no region we just use the rect they gave us.

fwiw, gtk2 actually passed in a region for damaged areas. Is that available for use now? (That's open as bug #121264 fwiw.)

Yep! With this patch, you're all set to go.

Hmm.... With this patch in my (opt GTK1) tree I see no particular speedups on http://alladyn.art.pl/gandalf/MozillaTests/dynl-c31.html and I _do_ see a regression -- the "Set opacity 10%" test does not paint the whole set of divs but only points a little rect (about 50px tall by 300w wide?) in the top left of the line of divs.

Oh, I and I had to edit the patch to get it to apply, since nsRegion.h has moved to gfx/public

The old GTK code would coalesce rects into a single "paint bounds rect" if the region had more than 10 rects in it. So really complex invalid regions will behave basically the same as before, especially if the the region mostly fills its bounds rect. I'm pulling a new tree now to check out the regression.

Well, I could reproduce it for a while but then it went away :-(. I suspect a resource exhaustion issue with the blending buffers...

With the patch there is some problems with demo #10 too (resource:///res/samples/test10.html). Images with opacity aren't painted correctly. Propably the same bug as in http://alladyn.art.pl/gandalf/MozillaTests/dynl-c31.html .

Sadly, that test case works for me now.

Something really bad is happening ... it seems under some circumstances, gdk_draw_image in nsDrawingSurfaceGTK::Unlock completely screws up when the clip region in mGC is more than just one rectangle. It's not a problem in the blender because I can replace the blending code with solid paint and the painting is still clipped. Typically a large band is clipped out.

OK, better patch. This inserts a workaround into nsRenderingContextGTK::CreateDrawingSurface that clears the current clip region while the surface is created. We don't need that clip area anyway and this avoids GTK bugs. This also fixes a bug (which the old code had but was somehow masking) where the update region was being cleared after DoPaint(). This was wrong because painting can sometimes trigger further invalidation (e.g., when a native theme fails to draw a widget and we have to paint it over again with the theme disabled).

Comment on attachment 113901 [details] [diff] [review] Bugs fixed... going for r+sr from blizzard ... I'll get kmcclusk to review the view changes.

Ccing kmcclusk for view review ... Kevin, you may want to implement the Windows code for sending the update region back up to the view manager.

The nsWindow changes seem to have some transparent window stuff mixed in? At least the mIsTransparent and mTransparencyBitmap stuff?

Er, yeah :-).

/home/bzbarsky/mozilla/profile/mozilla/widget/src/gtk/nsWindow.cpp: In method `nsresult nsWindow::Update()': /home/bzbarsky/mozilla/profile/mozilla/widget/src/gtk/nsWindow.cpp:866: `kRegionCID' undeclared (first use this function) /home/bzbarsky/mozilla/profile/mozilla/widget/src/gtk/nsWindow.cpp:866: (Each undeclared identifier is reported only once /home/bzbarsky/mozilla/profile/mozilla/widget/src/gtk/nsWindow.cpp:866: for each function it appears in.)

Yeah, that works. ;) No need to apologize; I know it's hard to untangle patches. :( I wouldn't have even bothered you about this except I was trying to see what perf impact it has on typical "dhtml" testcase (not much, since they all fall in the "200 divs" category....). It does help the viewer demo a lot. ;)

Comment on attachment 113915 [details] [diff] [review] terribly, terribly sorry >+ mTranMatrix->TransformCoord(&trect.x, &trect.y, >+ &trect.width, &trect.height); Can you fix the indenting here while you're at it? >+ // Don't paint anything if our window isn't visible. >+ if (!mSuperWin || mSuperWin->visibility == GDK_VISIBILITY_FULLY_OBSCURED) >+ // || !WindowIsVisible(mSuperWin->shell_window)) >+ return; Since you're not using WindowIsVisible you can remove this line and the WindowIsVisible function, right? > paintEvent.rect = &aRect; >+ paintEvent.region = nsnull; More indenting? (There are lots of other places like this in the patch.) Other that those little nits, looks good to me.

> Since you're not using WindowIsVisible you can remove this line and the > WindowIsVisible function, right? Will do. I'll fix the indenting before checking in. Thanks.

Checked in.

Thanks for fixing this. I used to get a weird transient rectangular copy-shift-paste blitting effect before. Gone now. And seems a little faster too. Red Hat 8.0, gcc 3.2, cvs build (up to date as of 22/03/2003) Configure arguments --enable-svg --enable-crypto --enable-default-toolkit=gtk2 --disable-toolkit-gtk --disable-toolkit-qt --enable-xft --enable-freetype2 --enable-cpp-rtti --enable-cpp-exceptions --enable-extensions --with-system-jpeg --with-system-zlib --with-system-png --with-system-mng

Can you clue me in as to how/if we might do the same thing for Mac?

You don't have to do anything. If you want to take advantage of this, all you have to do is to fill in the 'region' field of the paint event with the region that needs to be painted. Then the 'rect' field will be ignored.

For the record, this bug caused the regression in bug 199159.