Closed Bug 517802 Opened 13 years ago Closed 13 years ago

Scrolling repaints too much if the view is dirty

Categories

(Core :: Widget: Cocoa, defect)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: mstange, Assigned: mstange)

References

Details

(Keywords: perf)

Attachments

(1 file)

Attached patch v1Splinter Review
STR:
 1. Open this page in Firefox.
 2. Start Quartz Debug and mark "No delay after flash", "Flash screen updates"
    and "Autoflush drawing".
 3. Focus Firefox and scroll this page using the mouse wheel. Observe that even
    though the whole view flashes, only the newly-revealed part is actually
    repainted.
 4. Scroll the page by dragging the scrollbar.

Expected results:
Only repaint where necessary.

Actual results:
The whole view is repainted at every scroll.

The attached patch fixes this by using a 10.5+-only API to scroll dirty rects.
Attachment #401756 - Flags: review?(joshmoz)
Comment on attachment 401756 [details] [diff] [review]
v1

This looks fine but "translateRectsNeedingDisplayInRect:by:" documentation says it actually draws:

"Draws the resultant rectangles in the view."

If this is true, you probably don't want that. If this is false, please file a bug with Apple about the documentation.
In addition to filing that bug, please make an opt build with this patch on the try server and sanity check perf numbers. Also, take the resulting optimized build and sanity check it yourself by scrolling espn.com and a long text document without quartz debug. If you don't see any problems say so here and I'll r+.
(In reply to comment #1)
> (From update of attachment 401756 [details] [diff] [review])
> This looks fine but "translateRectsNeedingDisplayInRect:by:" documentation says
> it actually draws:
> 
> "Draws the resultant rectangles in the view."

This is false. I've filed rdar://problem/7241955 .

I also did the other things you asked me to and didn't find any problems.
Attachment #401756 - Flags: review?(joshmoz) → review+
http://hg.mozilla.org/mozilla-central/rev/ccdf3c2791bd
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
This problem doesn't occur in Firefox 3.0 and 3.5. What changed so that we need translateRectsNeedingDisplayInRect now?
My first guess would be bug 459319, which turned off synchronous repaints. When we were repainting synchronously, maybe we scrolled the view before the scrollbar rect had a chance to get marked as dirty, so viewWasDirty was false in those cases.
But even if we didn't have the problem with those STR I gave in comment 0, the general problem was present in 3.0, too.
You need to log in before you can comment on or make changes to this bug.