If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Prevent AsyncPanZoomController from repainting Zoom and Overscroll animations until animation has completed

RESOLVED FIXED in Firefox 47

Status

()

Core
Panning and Zooming
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: rbarker, Assigned: rbarker)

Tracking

Trunk
mozilla47
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Comment 1

2 years ago
This code is left over from when C++APZ did not send continuous repaint requests during animations.
(Assignee)

Comment 2

2 years ago
Created attachment 8715022 [details] [diff] [review]
0001-Bug-1245285-AsyncPanZoomController-ZoomToRect-needlessly-sends-repaint-request-for-final-FrameMetric-state-r-16020214-9ae9978.patch
(Assignee)

Updated

2 years ago
Attachment #8715022 - Flags: review?(botond)
(Assignee)

Comment 3

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=98d1e5538b73
(Assignee)

Updated

2 years ago
Assignee: nobody → rbarker
Comment on attachment 8715022 [details] [diff] [review]
0001-Bug-1245285-AsyncPanZoomController-ZoomToRect-needlessly-sends-repaint-request-for-final-FrameMetric-state-r-16020214-9ae9978.patch

Review of attachment 8715022 [details] [diff] [review]:
-----------------------------------------------------------------

We had an IRC discussion about this. The conclusion was that it would be better to avoid per-frame repaints for zoom animations, and keep this ahead-of-time repaint for the final resolution.
Attachment #8715022 - Flags: review?(botond)
(Assignee)

Updated

2 years ago
Summary: AsyncPanZoomController::ZoomToRect needlessly sends repaint request for final FrameMetric state → Prevent AsyncPanZoomController from repainting Zoom and Overscroll animations until animation has completed
(Assignee)

Comment 5

2 years ago
Created attachment 8715470 [details] [diff] [review]
0001-Bug-1245285-part-1-Remove-unused-mRepaintInterval-from-AsyncPanZoomAnimation-r-16020312-3ed97b7.patch
Attachment #8715022 - Attachment is obsolete: true
(Assignee)

Comment 6

2 years ago
Created attachment 8715471 [details] [diff] [review]
0002-Bug-1245285-part-2-Prevent-Zoom-and-Overscroll-animations-from-repainting-durring-animation-r-16020312-9c339003.patch
(Assignee)

Updated

2 years ago
Attachment #8715470 - Flags: review?(botond)
(Assignee)

Updated

2 years ago
Attachment #8715471 - Flags: review?(botond)
Comment on attachment 8715470 [details] [diff] [review]
0001-Bug-1245285-part-1-Remove-unused-mRepaintInterval-from-AsyncPanZoomAnimation-r-16020312-3ed97b7.patch

Review of attachment 8715470 [details] [diff] [review]:
-----------------------------------------------------------------

As a follow-up, please remove the prefs apz.fling_repaint_interval and apz.smooth_scroll_repaint_interval (the declaration in gfxPrefs.h, the description in AsyncPanZoomController.cpp, and lines setting it in modules/libpref/init/all.js).

And since we're cleaning up anyways, might as well do the same for apz.pan_repaint_interval, which is already unused.
Attachment #8715470 - Flags: review?(botond) → review+
Comment on attachment 8715471 [details] [diff] [review]
0002-Bug-1245285-part-2-Prevent-Zoom-and-Overscroll-animations-from-repainting-durring-animation-r-16020312-9c339003.patch

Review of attachment 8715471 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8715471 - Flags: review?(botond) → review+
(Assignee)

Comment 9

2 years ago
Created attachment 8715499 [details] [diff] [review]
0003-Bug-1245285-part-3-Remove-unused-repaint_interval-prefs-r-16020313-8cc03b5.patch
Attachment #8715499 - Flags: review?(botond)

Updated

2 years ago
Attachment #8715499 - Flags: review?(botond) → review+

Comment 10

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/41ea5de656a9
https://hg.mozilla.org/integration/mozilla-inbound/rev/a211643de3b6
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0c8fb70a095
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/a4e1258bc299 - e10s wasn't happy, frequent https://treeherder.mozilla.org/logviewer.html#?job_id=21051744&repo=mozilla-inbound and https://treeherder.mozilla.org/logviewer.html#?job_id=21048046&repo=mozilla-inbound
Those failures are more likely to be from my bug 990916. I'll take a look.
See Also: → bug 1245830
I filed bug 1245830 for the intermittent failure. Personally I think you should be able to reland this but the sheriffs might prefer that I land the fix for the intermittent first (or they might want to back out bug 990916 but as the intermittent seems low-volume I hope not).
Yes, you can reland, sorry for the churn, I was nearly asleep and not doing things right.

Comment 15

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e7eb92b6856
https://hg.mozilla.org/integration/mozilla-inbound/rev/4436ae606459
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9506e817fe1

Comment 16

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7e7eb92b6856
https://hg.mozilla.org/mozilla-central/rev/4436ae606459
https://hg.mozilla.org/mozilla-central/rev/c9506e817fe1
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox47: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.