Appearing scrollbars causing full screen repaints

RESOLVED FIXED

Status

()

Core
Layout
--
major
RESOLVED FIXED
15 years ago
12 years ago

People

(Reporter: Markus Hübner, Assigned: roc)

Tracking

(Blocks: 1 bug, {perf, testcase})

Trunk
x86
Windows XP
perf, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fix][backed out of 1.4 final], URL)

Attachments

(2 attachments)

(Reporter)

Description

15 years ago
When viewing the dhtml viewer-demo things are fine as long as the animated 
elements are somewhere in the middle of the window. But when they move to the 
outside of a window and scrollbars appears the animation is very choppy.
I'd say that mozilla is doing a full screen repaint that is causing this 
slowdown.
This is actually happening in Mozilla for quite some time now - talking to roc 
on IRC he said Simon has been working on that already.
(Reporter)

Updated

15 years ago
Blocks: 21762
The text-entry bugs that smfr has been working on MAY be related. I'll
investigate this.
Assignee: other → roc+moz
Created attachment 112364 [details]
Testcase

This testcase shows something interesting if you turn on paint flashing. When
the box moves from left to right, as soon as a horizontal scrollbar appears we
do a full screen repaint every time. (This is presumably a source of the
slowdown.) But when the box moves from top to bottom, when a vertical scrollbar
appears we repaint the whole window once but we don't do any more full repaints
until the box reaches the bottom of its cycle, moves to the top and the
scrollbar disappears. So  top to bottom motion isn't really a problem.
The difference is probably due to the code in
nsContainerFrame::SyncFrameViewAfterReflow, which forces a full repaint when the
scrolled view changes width. It must be time to just whack this for good.
Created attachment 112365 [details] [diff] [review]
obvious patch
So I tried the patch and it does fix the issue.

Also, to my surprise, I didn't find any rendering regressions in the small
amount of testing I did. dhtmlcentral.com, other random browsing, editing some
text in Composer, random XUL, all seem to work.

sfraser: this also fixes *some* of the "typing in a scrolled textarea
invalidates the whole thing" problem. With this patch, when the textarea is
scrolled we don't invalidate the whole thing, we just invalidate the typed line,
the line before, and (unfortunately) any part of the textarea content that is
below the level where the content would end if the textarea was scrolled all the
way to the top.

We should probably consider checking this in when 1.4a opens. It's got to be a
performance win.

Comment 6

15 years ago
This change worries me a bit, though I don't know the code well enough to say
what might regress.

How hard would it be to follow dbaron's suggestion, and move view resizing out
of reflow, doing it in a separate pass?
FWIW, moving view sizing / positioning out of reflow would really only be a
workaround for the fact that we do min/max width computation in the same Reflow
function.
We can't take view sizing and positioning out of reflow unless we want to start
storing the combinedArea in every frame. I don't think it's worth it.

What we probably should do is apply this patch, and then in the future go even
further and make it so that moving and sizing views doesn't invalidate anything
at all, i.e., we force frames to do all invalidation.

sfraser: I'm worried about regressions too, but I think this is a case where the
code is clearly moving in the right direction, any regressions are not obvious
and won't be blockers, and by checking in at the beginning of 1.4a we'll have
the best chance to find any that do occur.

Comment 9

15 years ago
Robert O'Callahan wrote:
> We should probably consider checking this in when 1.4a opens. It's got to be a
> performance win.

What about asking for approval for 1.3beta instead ? Mozilla's scrollbar
handling and rendering (both rendering the scrollbar itself and all the
"sideeffects") s*cks a lot - and this patch seems to cure some of these 
performance issues...
Whiteboard: [fix]
(Reporter)

Updated

14 years ago
Keywords: mozilla1.3
(Reporter)

Comment 10

14 years ago
Can we get this into 1.3 final?
As Roland pointed out this would considerably help major scrollbar & sideeffects
perf issues.
No. We don't do performance work for final releases. It's not worth the risk.

Comment 12

14 years ago
Markus Hübner wrote:
> Can we get this into 1.3 final?

No, I already discussed that with roc+moz on IRC.

> As Roland pointed out this would considerably help major scrollbar & 
> sideeffects perf issues.

... but I doubt this patch kills more than ~~~1/10 of the "Mozilla's scrollbar
rendering is (horribly) inefficient"-issues. For example - for each mouse-move
the whole scrollbar widget redraws itself instead of redrawing the thumb only
(this is bug 175411 ("XUL scrollbar rendering is _extremely_ inefficient") - and
using the OS native scrollbar widgets is not much better due other issues... ;-(
).
Attachment #112365 - Flags: superreview?(dbaron)
Attachment #112365 - Flags: review?(dbaron)
Attachment #112365 - Flags: superreview?(dbaron)
Attachment #112365 - Flags: superreview+
Attachment #112365 - Flags: review?(dbaron)
Attachment #112365 - Flags: review+
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
note, this caused a regression, see bug #194638
Blocks: 194638
fyi, this fix is on the trunk, but not part of 1.4 final.
No longer blocks: 194638
Whiteboard: [fix] → [fix][backed out of 1.4 final]
Blocks: 194638

Updated

12 years ago
No longer blocks: 21762
Blocks: 21762
You need to log in before you can comment on or make changes to this bug.