Closed Bug 605692 Opened 9 years ago Closed 9 years ago

Add a minimum offset change check to prevent content flickering when fingers are not (really) moving

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: vingtetun, Assigned: vingtetun)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached patch Patch (obsolete) — Splinter Review
While doing a pinch zoom I often do some pause to look at the content while my fingers are still on screen.

In Fennec the content flickered making strings unreadable while it does not move in other browser.

The patch add a threshold above which we're not taking gesture change into account.
Attachment #484589 - Flags: review?(mbrubeck)
Comment on attachment 484589 [details] [diff] [review]
Patch

Testing this patch, it seems to make zoom slow and choppy when making small or slow pinch movements.

Rather than throw away small changes, I think it might be better to accumulate them.  The original code in attachment 449580 [details] [diff] [review] looked like it was designed to do this (search that patch for "Accumulate pinch delta") but the capability was never actually used.  Do you think that approach could work?

You can use the aEvent.delta value directly to determine a threshold to ignore, rather than comparing rects.
Attachment #484589 - Flags: review?(mbrubeck) → review-
(In reply to comment #1)
> Comment on attachment 484589 [details] [diff] [review]
> Patch
> 
> Testing this patch, it seems to make zoom slow and choppy when making small or
> slow pinch movements.

I guess I've been too aggressive, my initial plan was to using the scale factor to have the effect practically invisible for small zoom levels. But I think you're right, I should will probably do something more sophisticated.

> Rather than throw away small changes, I think it might be better to accumulate
> them.  The original code in attachment 449580 [details] [diff] [review] looked like it was designed to do
> this (search that patch for "Accumulate pinch delta") but the capability was
> never actually used.  Do you think that approach could work?

The problem I have with this approach is if we accumulate small changes until we reach a threshold this mean we will still moving the content even if the user does nothing (== just keep his fingers on the screen) because there is always very small fingers moves.
Attached patch Patch v0.2Splinter Review
Finally accumulating delta works quite well probably because of the real randomness creating positive/negative values.
Attachment #484589 - Attachment is obsolete: true
Attachment #484701 - Flags: review?(mbrubeck)
Attachment #484701 - Flags: review?(mbrubeck) → review+
http://hg.mozilla.org/mobile-browser/rev/b1d918601c22
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Bug 606391 reported

verified:
Mozilla/5.0 (Android; Linux armv71; rv2.0b8pre) Gecko/20101021 Firefox/4.0b8pre Fennec/4.0b2pre
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101021 Firefox/4.0b8pre Fennec/4.0b2pre
Status: RESOLVED → VERIFIED
Attached patch followupSplinter Review
Using today's nightly I have realized that I done a mistake when pushing. (Sigh)
Attachment #485315 - Flags: review?(mbrubeck)
Attachment #485315 - Flags: review?(mbrubeck) → review+
Depends on: 608183
bugspam
Assignee: nobody → 21
You need to log in before you can comment on or make changes to this bug.