Bug 6388

Summary:

REGRESSION: Incomplete repaint when dragging the map on Google maps

Product:

WebKit

Reporter:

mitz

Component:

Layout and Rendering

Assignee:

Nobody <webkit-unassigned>

Status:

VERIFIED FIXED

Severity:

Major

CC:

alice.barraclough

Priority:

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

URL:

http://maps.google.com/

Attachments:

Description	Flags
Reduced testcase	none
Possible fix	darin: review-
Updated patch	darin: review+

mitz

Reported 2006-01-05 14:06:14 PST

If you drag the map slowly straight down (or straight to the right), it "smears".

Attachments
Reduced testcase (536 bytes, text/html) 2006-01-05 16:10 PST, mitz	no flags	Details
Possible fix (2.23 KB, patch) 2006-01-24 15:55 PST, mitz	darin: review-	Details Formatted Diff Diff
Updated patch (4.61 KB, patch) 2006-01-25 08:27 PST, mitz	darin: review+	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

mitz

Comment 1 2006-01-05 14:17:51 PST

The regression happened between Nov 6 and Nov 8 so I'm guessing it's from fixing bug 5633.

mitz

Comment 2 2006-01-05 16:10:35 PST

Created attachment 5506 [details] Reduced testcase The optimized repaint code thinks that the image only resized, rather than moved.

Alice Liu

Comment 3 2006-01-11 13:25:43 PST

<rdar://problem/4405688>

mitz

Comment 4 2006-01-24 15:55:40 PST

Created attachment 5931 [details] Possible fix This fixes the Google maps site and the testcase. I think the bug (mistaking a move for a resize due to clipping) can only happen to objects that have a layer, since otherwise repaintDuringLayoutIfMoved handles it. If the patch is good, please r- and I'll resubmit with a ChangeLog entry and a manual test (unfortunately repaint issues can't be tested automatically).

Darin Adler

Comment 5 2006-01-24 19:29:21 PST

Comment on attachment 5931 [details] Possible fix Looks good. I think you can move on to the next step on this one.

Dave Hyatt

Comment 6 2006-01-24 20:19:28 PST

Could you explain this further? I don't see why these extra fields are necessary.

Dave Hyatt

Comment 7 2006-01-24 20:23:25 PST

This is marked as a regression. Was it caused by the checkin I did a month or two ago to optimize repainting when a block resized but didn't move?

mitz

Comment 8 2006-01-24 22:15:24 PST

(In reply to comment #7) > This is marked as a regression. Was it caused by the checkin I did a month or > two ago to optimize repainting when a block resized but didn't move? Yes. There are several of those, some fixed, some still not. (In reply to comment #6) > Could you explain this further? I don't see why these extra fields are > necessary. Using absolute repaint rects, all repaintAfterLayoutIfNeeded sees is a resize, due to clipping. This extra info is what's needed to break the move/resize ambiguity. There is at least one other case where repaintAfterLayoutIfNeeded lacks enough information to determine what really changed (see bug 6770), so perhaps repaintAfterLayoutIfNeeded will need to take extra arguments (e.g. the object's unclipped rect in absolute coordinates), in which case layers will need to cache that info.

Dave Hyatt

Comment 9 2006-01-24 22:50:55 PST

Ok, yeah I see now. I think I might rename the member variables m_repaintX and m_repaintY or something like that, since they are really just cached values for the repaint check step.

mitz

Comment 10 2006-01-25 08:27:36 PST

Created attachment 5946 [details] Updated patch Renamed member variables and added ChangeLog and manual test.

Darin Adler

Comment 11 2006-01-25 09:38:47 PST

Comment on attachment 5946 [details] Updated patch Nice. Lets go with this. r=me Onward to the other repaint problems. :-)

mitz

Comment 12 2006-01-27 01:02:13 PST

Verified in r12402 nightly

Eric Seidel (no email)

Comment 13 2006-01-31 21:20:54 PST

Removing Regression keyword from bugs already fixed.

Note You need to log in before you can comment on or make changes to this bug.