779572 - Zooming is not taken into account for min pan distance

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Comment 1

[Doug Sherk (:drs) (inactive)]

Attachment: Factor zoom into min pan distance before moving the viewport

When we're zoomed out really far, we have to move our finger across almost the entire screen just to start moving it. This fixes that.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 648022 [details] [diff] [review]
Factor zoom into min pan distance before moving the viewport

>diff --git a/gfx/layers/ipc/AsyncPanZoomController.cpp b/gfx/layers/ipc/AsyncPanZoomController.cpp

>+      MonitorAutoLock monitor(mMonitor);
>+      if (PanDistance() / mFrameMetrics.mResolution.width < panThreshold) {
>         return nsEventStatus_eIgnore;
>       }
>+

Let's keep the locking scoped to this computation so that we don't
hold it in StartPanning().

And hm, do we need to be locking in StartPanning()?

Comment 3

[Doug Sherk (:drs) (inactive)]

Attachment: Factor zoom into min pan distance before moving the viewport

Comment 4

[Doug Sherk (:drs) (inactive)]

Attachment: Factor zoom into min pan distance before moving the viewport

herp derp

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 648971 [details] [diff] [review]
Factor zoom into min pan distance before moving the viewport

Document that StartPanning() needs the state mutex and assert that it's held in the function.

Comment 6

[Doug Sherk (:drs) (inactive)]

Ok, so it turns out that StartPanning should not be protected except for its change to |mState|, which was not being properly protected anywhere else. I have removed the monitor holding from that and will be re-adding it back in a followup patch which will protect all changes to |mState| properly.

Comment 7

[Doug Sherk (:drs) (inactive)]

Attachment: Factor zoom into min pan distance before moving the viewport

Comment 8

[Doug Sherk (:drs) (inactive)]

Attachment: Properly protect state in AsyncPanZoomController with a monitor

Comment 9

[Doug Sherk (:drs) (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4d52cfac7c37
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce9014442e13

Comment 10

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/d8878c0bd67c - something in that push made Linux oddly unhappy.

Comment 11

[Doug Sherk (:drs) (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7f25c1478701
https://hg.mozilla.org/integration/mozilla-inbound/rev/b468eccf7261

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7f25c1478701
https://hg.mozilla.org/mozilla-central/rev/b468eccf7261