238493 - Need to (re)implement widget change caching

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

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

We have a lot of flickering going on during reflow due to widgets being
temporarily repositioned. We need to revive the view manager's support for
suppressing widget changes until reflow is done.

Ultimately we want to fix up the geometries only after we have painted all
windows into a back buffer.

Comment 1

[Warner Young]

I'd like to suggest that this, and bug 132337, block Firefox 1.0.  This is a
very highly visible bug on well-trafficked sites like Gamespot or Anandtech that
even the most novice user will notice (in my opinion).

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Reflow refactoring is likely to fix the temporary repositioning.

Comment 3

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

Attachment: work in progress

Even if reflow doesn't have to do trial reflows (and even after your
refactoring it will in some cases, like the two-pass margin collapse &
clearance algorithm), it's still going to be helpful to postpone widget
geometry changes until the last moment. So I'm doing this by suppressing widget
changes during Begin/EndUpdateViewBatch and then repositioning/resizing all
widgets when we re-enable refresh.

This code seems to work --- the flickering is gone --- but may be slowing down
window resizes ... I'm not sure. I will investigate. I also need to port the
paint flashing code to gtk2 so I can check only the right areas are painted.

It includes the patch for bug 262760 because this depends on that.

Comment 4

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

Attachment: fix

I ran into a GTK2 bug that made windows not position correctly if they were
moved via Resize(x,y,w,h) where the width or height is zero. Other than that
the patch is fairly straightforward.

This stops flickering in most of the dependencies and could conceivably give us
some Tp win.

Comment 5

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

Comment on attachment 161548 [details] [diff] [review]
fix

See comments in the bug. BTW I checked Windows and GTK1 and they don't seem to
have that particular GTK2 bug...

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 161548 [details] [diff] [review]
fix

r+sr=bzbarsky

Comment 7

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

Checked in.

Comment 8

[jag (Peter Annema)]

http://axolotl.mozilla.org/graph/query.cgi?testname=pageload&tbox=btek&autoscale=1&days=7&avg=1&showpoint=2004:10:11:14:33:45,823

It looks like this checkin bumped btek from avg. 817 ms to 822 ms.

Comment 9

[Ger Teunis]

Will this patch also land in the aviary1.0?

Comment 10

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

(In reply to comment #8)
> It looks like this checkin bumped btek from avg. 817 ms to 822 ms.

Yes. Some Tps went up, some went down, some didn't move. I think we should keep it.

I suggest that this not land on Aviary.

If someone wants to be helpful they could go through the bugs depending on this
one and identify the ones that are fixed now.

Comment 11

[José Jeria]

Test results on Windows XP (build 20041012):

Bug 39532  No testcase
Bug 47742  NOT FIXED
Bug 132337 FIXED
Bug 224426 WFM, even before this fix
Bug 234673 FIXED
Bug 255899 No way to test, never seen the bug
Bug 263208 FIXED

Comment 12

[Martijn Wargers (dead)]

I've tested the duplicates of bug 132337:
Bug 228059 - Fixed
Bug 221858 - Fixed
Bug 246587 - Fixed
Bug 242230 - Fixed
Bug 239706 - Fixed
Bug 246587 - Fixed
Bug 238714 - No URL or testcase
Bug 262873 - Fixed
Bug 246394 - Fixed
Bug 260053 - Fixed

Bug 255899 is not fixed by the fix for this bug.
Bug 263815 - Fixed

Comment 13

[Stig Nygaard]

I don't know anything how complex this fix is, and how big risk it would be to
land on aviary1.0 branch. But I know that on a slow PC like mine AMD K6-II
400MHz (Win98) bug 132337 (which according to comment #11 are fixed by this) are
VERY visible and annoying on many danish sites. For example some of Denmarks
largest newspapers, especially http://politiken.dk/.

The main column of politiken.dk (in the middle) renders fast, but the left and
right columns renders very slowly. I think I can count it render 3-4 lines pr.
second. And the whole page takes ages to render and it feels like all CPU-power
are eaten by the process.

On the other hand politiken.dk renders fast and I only briefly see a little
flicker, but not much or annoing, when loading the site on my transportable
work-PC with a Pentium M processor (don't know exactly how fast)and WinXP. So
this is problably a bug that which mostly hits users with slower machines.

So if its not to risky, please consider this for aviary1.0. The lightweight
browser thats also for smaller/slower PCs :-)

Comment 14

[Ger Teunis]

(In reply to comment #13)
> So if its not to risky, please consider this for aviary1.0. The lightweight
> browser thats also for smaller/slower PCs :-)

I totally agree on this, this patch will greatly improve page rendering and the
first-time experience for new users for the 1.0 release.

Comment 15

[Frank Wein [:mcsmurf]]

Bug 263992 (menus at the wrong position) has been caused by this bug here.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Uh, guys?  We're not stupid.  We know what the patch fixes.  If we considered it
safe enough for Aviary, we wouldn't have issues with it going into Aviary.  So
if Robert is against it, presumably he thinks it's not safe enough, eh?

Certainly I don't think it's safe enough.

Comment 17

[Martijn Wargers (dead)]

Some bugs I found with the keyword 'flicker' and that are fixed by the fix for
this bug:
Bug 170702 - Fixed (no flicker anymore with the first dhtml testcase, also
performance seems improved by appr. 5%)
Bug 209531 - Not reproducable anymore (but would be fixed by this bug)
Bug 221622 - Fixed
Bug 225836 - Fixed
Bug 228923 - Fixed
Bug 235553 - Fixed
Bug 249978 - Fixed
Bug 255761 - Fixed (although this has caused a regression, I think)
Bug 255922 - Fixed
Bug 260419 - Fixed

Comment 18

[Martijn Wargers (dead)]

Sorry, the statements I made about Bug 170702 in comment 17 are bogus.

Another couple of bugs fixed by the fix for this bug (searched with the keyword
'flash'):
Bug 246138 - Fixed
Bug 245417 - Fixed
Bug 250583 - Fixed
Bug 257902 - Fixed
Bug 259804 - Fixed

Comment 19

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

Thanks Martjin. As usual, you rock my world. After a week or so when regressions
have settled down, it would be great if you can go through the bugs and mark
them fixed. Thank you so much.

Comment 20

[Boris Zbarsky [:bzbarsky]]

This also caused regression bug 264245, looks like.

Comment 21

[Boris Zbarsky [:bzbarsky]]

And likely bug 264235 and bug 264229.

Comment 22

[HJ]

attachment 161548 [details] [diff] [review] also killed <tooltip position="before_start"... 

It worked in Mozilla builds up to 20040507 but is broken since Mozilla build
20040508

Comment 23

[Boris Zbarsky [:bzbarsky]]

Um...  That attachment wasn't checked in till 2004-10-11.  That's 5 months after
2004-05-08. 

Comment 24

[HJ]

(In reply to comment #23)
> Um...  That attachment wasn't checked in till 2004-10-11.  That's 5 months after
> 2004-05-08. 

Ok, so I was wrong, but someone on IRC told me that this was the bug I was
looking for. Me bad, because I didn't check *if* he was right, because I trusted
him. Won't do that again, that's for sure! Again, sorry for this error.

Comment 25

[Asaf Romano (gone)]

is bug 264101 also a regression from this one?

Comment 26

[Martijn Wargers (dead)]

*** Bug 266162 has been marked as a duplicate of this bug. ***

Comment 27

[Jasper]

Camino seems to have some serious issues concerning the drawing of the scroll
bars right after the patch landed on the trunk. See Bug 268320 for more
information. We need somebody to back out the patch and see if that fixes the
issues.

Comment 28

[Chris Priebe]

I have created a testing page that implements the instance of having an iframe
on the page with a float command in the css

http://beta.mytwu.com/firefoxBug/

Comment 29

[Steve Temple]

This is happening on version 1. Did it fail to make it in time for that version? 

Comment 30

[Boris Zbarsky [:bzbarsky]]

Steve, please read the bug before commenting... in particular, read comments 9,
10, 13, 14, 16.  In brief, this fix landed half a year after Firefox 1.0
branched off the development trunk.

Comment 31

[:Gavin Sharp [email: [e-mail]]]

*** Bug 276458 has been marked as a duplicate of this bug. ***

Comment 32

[Bob Clary [:bc] (inactive)]

*** Bug 282417 has been marked as a duplicate of this bug. ***

Comment 33

[José Jeria]

Is Bug 282708 related to this one? Its about plugins flickering in the top left
when loading the webpage.

Comment 34

[Martijn Wargers (dead)]

*** Bug 290487 has been marked as a duplicate of this bug. ***

Comment 35

[chris hofmann]

sounds like bug 303433 might have been fixed by this too... 
https://bugzilla.mozilla.org/show_bug.cgi?id=303433#c45  

Comment 36

[Martijn Wargers (dead)]

*** Bug 39532 has been marked as a duplicate of this bug. ***