528852 - seams when scrolling with position:fixed and GTK+2.18

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Karl Tomlinson (:karlt)]

With GTK+-2.18.3, gdk_window_move_region copies the invalid region from the
source but doesn't subtract the invalid region at the destination.  Combine
this with the invalidation of source regions of previous
gdk_window_move_region operations, and moving to cover an adjacent source
region leaves it invalid.  Further, on gdk_window_process_updates removes any
invalid regions from pending moves, and so no move covers any previous source
region.  This leaves seams between each rectangle moved until they are repainted.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: background image for testcase

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: testcase

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: expected results vertical scrolling

This is the expected situation before the invalid areas are painted.
Obtained with GTK+-2.16.6.

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: actual results vertical scrolling

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: actual results vertical scrolling

Actual results with GTK+2.18.3.

These lines only flicker briefly, but you know they are there.

Comment 6

[Ria Klaassen (not reading all bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a1pre) Gecko/20091116 Minefield/3.7a1pre

I see aso seems on Windows. Not sure if it is the same, but possible.
Not reproducible with Firefox 2.

Comment 7

[Karl Tomlinson (:karlt)]

Thanks for checking Windows NT.  I filed bug 529092 for that.  It will need its own fix.

Comment 8

[Karl Tomlinson (:karlt)]

We can fix this by operating on the GdkWindow's update area ourselves.
A variation on some code that roc wrote here would work:
https://bugzilla.mozilla.org/attachment.cgi?id=386202&action=diff#a/widget/src/gtk2/nsWindow.cpp_sec1

Comment 9

[Karl Tomlinson (:karlt)]

Attachment: subtract invalid regions in move destinations

Comment 10

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

+            gdk_region_union(updateChanges, newUpdates);

Shouldn't this be gdk_region_intersect? You want to keep the newUpdates that are in the destination rect and throw away the rest.

I wonder if it's worth hoisting some of this temporary region creation and destruction out of the loop.

Comment 11

[Karl Tomlinson (:karlt)]

(In reply to comment #10)
> +            gdk_region_union(updateChanges, newUpdates);
> 
> Shouldn't this be gdk_region_intersect? You want to keep the newUpdates that
> are in the destination rect and throw away the rest.

At this point, updateChanges is portion of the old invalid area that has moved, and newUpdates are additional updates due to other windows.  We need them both (union).

We don't want to throw away what is outside the destination rect (unless and until it is covered by another move).  This was my attempt at explaining:

+    // The parts of source regions not covered by their destination get marked
+    // invalid (by GDK).  This is necessary (until covered by another blit)
+    // because GDK translates any pending expose events to the destination,
+    // and so doesn't know whether an expose event might have been due on the
+    // source.

> I wonder if it's worth hoisting some of this temporary region creation and
> destruction out of the loop.

Possible, if fairly ugly and less clear.
It might save some slice allocations (haven't checked).
We could create an empty region outside the loop, so that we could create an assignment operator by intersecting with the empty region then unioning with the new region or rect.

The other possibility is to have two paths:

1) GDK 2.16 or earlier - no need for messing with invalid regions.
2) GDK 2.18 or later - rects can be gathered into fewer regions.

However, I wasn't so keen on having to test two paths.

Comment 12

[Karl Tomlinson (:karlt)]

Requesting approval to land this on m-c.
The equivalent bug on NT (Bug 529092) was a blocker.
With GTK it only affects people with the latest branch of GTK, so I don't think this needs to be a blocker.  That group of people does include those on the latest Ubuntu 9.10 ("Karmic Koala"), though.

It's very hard to automatically test this because it's all about interactions with GTK and I don't know a good way to test GTK's response.

I think we'll want this on 1.9.2 at some stage, because GTK+-2.18 will be more widely used, so I think it's worth getting this on m-c ASAP.  (There isn't really a bug in GTK; GTK just isn't doing things as nicely for our situation as it used to.)

Comment 13

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/mozilla-central/rev/bc9963f9a495

Comment 14

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/f520e15d6791