Closed
Bug 190193
Opened 22 years ago
Closed 21 years ago
Appearing scrollbars causing full screen repaints
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: markushuebner, Assigned: roc)
References
()
Details
(Keywords: perf, testcase, Whiteboard: [fix][backed out of 1.4 final])
Attachments
(2 files)
609 bytes,
text/html
|
Details | |
1.65 KB,
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•22 years ago
|
||
The text-entry bugs that smfr has been working on MAY be related. I'll investigate this.
Assignee: other → roc+moz
Assignee | ||
Comment 2•22 years ago
|
||
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.
Assignee | ||
Comment 3•22 years ago
|
||
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.
Assignee | ||
Comment 4•22 years ago
|
||
Assignee | ||
Comment 5•22 years ago
|
||
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•22 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.
Assignee | ||
Comment 8•22 years ago
|
||
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•22 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...
Assignee | ||
Updated•22 years ago
|
Whiteboard: [fix]
Reporter | ||
Updated•21 years ago
|
Keywords: mozilla1.3
Reporter | ||
Comment 10•21 years ago
|
||
Can we get this into 1.3 final? As Roland pointed out this would considerably help major scrollbar & sideeffects perf issues.
Assignee | ||
Comment 11•21 years ago
|
||
No. We don't do performance work for final releases. It's not worth the risk.
Comment 12•21 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... ;-( ).
Assignee | ||
Updated•21 years ago
|
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+
Assignee | ||
Comment 13•21 years ago
|
||
Fix checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 14•21 years ago
|
||
note, this caused a regression, see bug #194638
Comment 15•21 years ago
|
||
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]
You need to log in
before you can comment on or make changes to this bug.
Description
•