Closed Bug 238493 Opened 20 years ago Closed 20 years ago

Need to (re)implement widget change caching

Categories

(Core :: Web Painting, defect)

Product:
Core
Component:
Web Painting
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: roc, Assigned: roc)

References

Details

Attachments

(1 file, 1 obsolete file)

fix
27.31 KB, patch
bzbarsky
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review 
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.
Blocks: 39532
Blocks: 239706
Blocks: 47742
Blocks: 255899
Blocks: 132337
No longer blocks: 239706
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).
Reflow refactoring is likely to fix the temporary repositioning.
Attached patch work in progress (obsolete) — — Splinter Review 
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.
Attached patch fix — — Splinter Review 
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.
Attachment #160986 - Attachment is obsolete: true
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...
Attachment #161548 - Flags: superreview?(bzbarsky)
Attachment #161548 - Flags: review?(bzbarsky)
Comment on attachment 161548 [details] [diff] [review]
fix

r+sr=bzbarsky
Attachment #161548 - Flags: superreview?(bzbarsky)
Attachment #161548 - Flags: superreview+
Attachment #161548 - Flags: review?(bzbarsky)
Attachment #161548 - Flags: review+
Checked in.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Will this patch also land in the aviary1.0?
(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.
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
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
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 :-)
(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.


Bug 263992 (menus at the wrong position) has been caused by this bug here.
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.
Depends on: 263992
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
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
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.
Depends on: 264245
This also caused regression bug 264245, looks like.
And likely bug 264235 and bug 264229.
Depends on: 264229, 264235
Depends on: 264352
Blocks: 263380
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
Um...  That attachment wasn't checked in till 2004-10-11.  That's 5 months after
2004-05-08. 
(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.
Blocks: 263642
is bug 264101 also a regression from this one?
Blocks: 265875
*** Bug 266162 has been marked as a duplicate of this bug. ***
Blocks: 266059
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.
Blocks: 268652
Blocks: 269096
Depends on: 267422
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/



This is happening on version 1. Did it fail to make it in time for that version? 
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.
Blocks: 273596
Blocks: 275167
Blocks: 274761
Blocks: 276458
*** Bug 276458 has been marked as a duplicate of this bug. ***
*** Bug 282417 has been marked as a duplicate of this bug. ***
Blocks: 287993
Is Bug 282708 related to this one? Its about plugins flickering in the top left
when loading the webpage.
*** Bug 290487 has been marked as a duplicate of this bug. ***
sounds like bug 303433 might have been fixed by this too... 
https://bugzilla.mozilla.org/show_bug.cgi?id=303433#c45
Depends on: 315186
*** Bug 39532 has been marked as a duplicate of this bug. ***
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: