Closed Bug 701586 Opened 8 years ago Closed 8 years ago

Axis lock during panning

Categories

(Firefox for Android :: General, defect, P3)

x86
macOS
defect

Tracking

()

VERIFIED FIXED
Tracking Status
firefox11 --- verified
fennec 11+ ---

People

(Reporter: pcwalton, Assigned: kats)

References

Details

Attachments

(1 file)

birch-pan-zoom needs an axis lock during panning.
Priority: -- → P3
Assignee: nobody → kgupta
I think this needs more work, but I need feedback from people who try it out first.
Attachment #575503 - Flags: review?(pwalton)
Comment on attachment 575503 [details] [diff] [review]
Add axis lock (WIP)

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

Looks good. Though I'm just going to trust the trigonometry is correct.

::: embedding/android/ui/PanZoomController.java
@@ +177,2 @@
>          case PANNING_HOLD:
> +            // fall through

nit: I've seen it written as "// FALL THROUGH" right before the next "case" in Mozilla code.

@@ +255,5 @@
> +            } else if (Math.abs(angle - (Math.PI / 2)) < AXIS_LOCK_ANGLE) {
> +                // lock to y-axis
> +                x = mX.firstTouchPos;
> +            } else {
> +                // break axis lock but log the angle so we can fine-tune this when people complain

:)

@@ +274,5 @@
> +                mState = PanZoomState.PANNING_HOLD;
> +            } else if (mState == PanZoomState.PANNING_LOCKED) {
> +                mState = PanZoomState.PANNING_HOLD_LOCKED;
> +            } else {
> +                // should never happen, but handle anyway for robustness

Not relevant to this patch, but we really should have some kind of non-fatal-in-production, fatal-in-debug assertion setup...
Attachment #575503 - Flags: review?(pwalton) → review+
> nit: I've seen it written as "// FALL THROUGH" right before the next "case"
> in Mozilla code.

Fixed; I was juggling some lines around and missed that.

> 
> Not relevant to this patch, but we really should have some kind of
> non-fatal-in-production, fatal-in-debug assertion setup...

Yeah, agreed. Usually the java assert statement satisfies this criteria, and it seems like we can do this on android - assertions can be enabled by "adb shell setprop dalvik.vm.enableassertions all". I'll test it out and add a note on the java idioms page.
http://hg.mozilla.org/projects/birch/rev/ec82d9d943f9

I'll leave the bug open for a bit to see if there's any complaints on the behaviour.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Depends on: 703814
tracking-fennec: --- → 11+
Verified fixed on:
Nightly 13.0a1 (2012-02-21)
Beta 11.0 (20120221151724)
Device: Samsung Galaxy SII (Android 2.3.4)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.