288369 - [FIX]Don't do DEFERRED view batch when doing pre-paint reflow flush

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

The problem is that this effectively reschedules the point at which the
invalidates will happen -- instead of happening when the reflow event fires,
they will now happen at the (later) point when the invalidate event fires.  This
causes frame dropping on the testcase at
http://www.speich.net/computer/moztesting/3d.htm

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

This also makes sure we only touch mScrollCnt on the root viewmanager, as
documented...

Comment 2

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

Comment on attachment 179112 [details] [diff] [review]
Patch

That's OK, but can't we do the widget resyncing even during scrolling?

Comment 3

[Boris Zbarsky [:bzbarsky]]

We could, but I thought you'd asked me to move it into this block in email?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Specifically, you said:

> If we do this, we should move the widget reconfiguration code
> that I just wrote into the if clause so it only happens when
> mScrollCnt is zero.

Also note that I'll go ahead and fix up the comment in that code before checking
in....

Comment 5

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

I was confused and then expressed myself unclearly. Sorry. Just make the
ProcessPendingUpdates unconditional. If the Begin/EndBatch flushes updates, then
rootVM->mHasPendingUpdates will be false and we won't waste any time resyncing
widgets again (which is what I was vaguely worried about when I made my
incorrect comment).

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch for checkin

Comment 7

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 8

[Simon Fraser [no longer active]]

There are reports of sporadic "scrambling" issues in Camino starting on the 4/1
build. Maybe regression from this fix?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Possible, I guess...  That could effectively cause widget movement and
invalidation and then only paint part of the newly-invalidated area.  Depending
on what else is going on and how well widgets deal, that could lead to issues,
maybe...

Comment 10

[Mike Pinkerton (not reading bugmail)]

that's exactly what appears to be happening. the views/widgets are moving after
a page reflow and the area where they were previously is not being repainted. 

Comment 11

[Boris Zbarsky [:bzbarsky]]

Not being repainted right then, or ever?  What I'd expect is that the widget
moves cause invalidates that cause a later call into painting from widget... 
This does mean that things only work right if the widget clears its "region to
be repainted" member, whatever it is, before calling into painting, not after
(see http://lxr.mozilla.org/seamonkey/source/widget/src/gtk/nsWindow.cpp#886 for
an example).

Could that be the problem here?

Comment 12

[Mike Pinkerton (not reading bugmail)]

not being repainted then. scrolling or resizing the window causes everything to
repaint correctly. 

Comment 13

[Boris Zbarsky [:bzbarsky]]

That sounds like "ever"...  "Then" would be a brief flicker of the wrong thing,
then the right thing being painted.

So that does sound like the problem that could happen per comment 11... Any idea
whether that's what the mac code does?