Closed Bug 288369 Opened 19 years ago Closed 19 years ago

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


(Core :: Web Painting, defect, P1)






(Reporter: bzbarsky, Assigned: bzbarsky)




(1 file, 1 obsolete file)

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
Attached patch Patch (obsolete) — Splinter Review
This also makes sure we only touch mScrollCnt on the root viewmanager, as
Attachment #179112 - Flags: superreview?(roc)
Attachment #179112 - Flags: review?(roc)
Blocks: 229391
Priority: -- → P1
Summary: Don't do DEFERRED view batch when doing pre-paint reflow flush → [FIX]Don't do DEFERRED view batch when doing pre-paint reflow flush
Target Milestone: --- → mozilla1.8beta2
Comment on attachment 179112 [details] [diff] [review]

That's OK, but can't we do the widget resyncing even during scrolling?
Attachment #179112 - Flags: superreview?(roc)
Attachment #179112 - Flags: superreview+
Attachment #179112 - Flags: review?(roc)
Attachment #179112 - Flags: review+
We could, but I thought you'd asked me to move it into this block in email?
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
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).
Attachment #179112 - Attachment is obsolete: true
Closed: 19 years ago
Resolution: --- → FIXED
There are reports of sporadic "scrambling" issues in Camino starting on the 4/1
build. Maybe regression from this fix?
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,
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. 
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 for
an example).

Could that be the problem here?
not being repainted then. scrolling or resizing the window causes everything to
repaint correctly. 
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?
Depends on: 294415
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.