Closed
Bug 1018255
Opened 10 years ago
Closed 9 years ago
Followup tweaks to the overscroll effect physics
Categories
(Core :: Panning and Zooming, defect)
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: milan, Assigned: botond)
References
Details
Attachments
(6 files, 7 obsolete files)
9.30 KB,
patch
|
Details | Diff | Splinter Review | |
1.52 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
1.16 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
8.30 KB,
patch
|
botond
:
review+
|
Details | Diff | Splinter Review |
11.73 KB,
patch
|
botond
:
review+
|
Details | Diff | Splinter Review |
8.78 KB,
patch
|
botond
:
review+
|
Details | Diff | Splinter Review |
We anticipate tweaking the overscroll effect introduced in bug 998025. This bug will collect the requirements as they show up with testing, perhaps turning into a meta bug.
Reporter | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
This is basically straight out of Milan's patch.
Assignee: nobody → botond
Assignee | ||
Comment 3•9 years ago
|
||
Calling ApplyOverscrollEffect when we are not overscrolled should be harmless, but it's needless work so we should avoid it.
Assignee | ||
Comment 4•9 years ago
|
||
Attachment #8433702 -
Flags: review?(bugmail.mozilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8433701 -
Flags: review?(bugmail.mozilla)
Assignee | ||
Comment 5•9 years ago
|
||
This is the "making the Z-effect slighter" change that we discussed.
Assignee | ||
Comment 6•9 years ago
|
||
This patch implements the use of spring physics to model the snap-back animation. (This is different from the changes to the snap-back animation in Milan's patch.) These patches, taken together, implement the physics tweaks that we discussed. I haven't put prefs in place for everything yet, I'd like to give Gordon a chance to pay around with this and see if we have the right models.
Updated•9 years ago
|
Blocks: apz-overscroll
Comment 7•9 years ago
|
||
Comment on attachment 8433702 [details] [diff] [review] Part 3 - Fix a short-circuit evaluation bug in OverscrollSnapBackAnimation::Sample Review of attachment 8433702 [details] [diff] [review]: ----------------------------------------------------------------- You could just use | instead of ||
Attachment #8433702 -
Flags: review?(bugmail.mozilla) → review+
Updated•9 years ago
|
Attachment #8433701 -
Flags: review?(bugmail.mozilla) → review+
Assignee | ||
Comment 8•9 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #7) > You could just use | instead of || It somehow feels wrong to use a bitwise operator for a logical purpose.
Assignee | ||
Comment 9•9 years ago
|
||
Updated to expose relevant prefs.
Attachment #8433700 -
Attachment is obsolete: true
Attachment #8435181 -
Flags: review?(bugmail.mozilla)
Assignee | ||
Comment 10•9 years ago
|
||
Updated to expose relevant prefs.
Attachment #8433703 -
Attachment is obsolete: true
Attachment #8435182 -
Flags: review?(bugmail.mozilla)
Assignee | ||
Comment 11•9 years ago
|
||
Updated to expose relevant prefs.
Attachment #8433707 -
Attachment is obsolete: true
Attachment #8435183 -
Flags: review?(bugmail.mozilla)
Assignee | ||
Comment 12•9 years ago
|
||
Fixed a mistake.
Attachment #8435181 -
Attachment is obsolete: true
Attachment #8435181 -
Flags: review?(bugmail.mozilla)
Attachment #8435187 -
Flags: review?(bugmail.mozilla)
Comment 13•9 years ago
|
||
Comment on attachment 8435187 [details] [diff] [review] Part 1 - Stop fling more quickly when overscrolled Review of attachment 8435187 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/app/b2g.js @@ +955,5 @@ > pref("apz.subframe.enabled", true); > > // Overscroll-related settings > pref("apz.overscroll.enabled", false); > +pref("apz.overscroll.fling_friction", "0.01"); trailing ws ::: gfx/thebes/gfxPrefs.h @@ +121,5 @@ > DECL_GFX_PREF(Once, "apz.max_velocity_queue_size", APZMaxVelocityQueueSize, uint32_t, 5); > DECL_GFX_PREF(Live, "apz.min_skate_speed", APZMinSkateSpeed, float, 1.0f); > DECL_GFX_PREF(Live, "apz.num_paint_duration_samples", APZNumPaintDurationSamples, int32_t, 3); > DECL_GFX_PREF(Live, "apz.overscroll.enabled", APZOverscrollEnabled, bool, false); > + DECL_GFX_PREF(Live, "apz.overscroll.fling_friction", APZOverscrollFlingFriction, float, 0.01f); trailing ws
Attachment #8435187 -
Flags: review?(bugmail.mozilla) → review+
Comment 14•9 years ago
|
||
Comment on attachment 8435182 [details] [diff] [review] Part 4 - Make the translation (in the direction of overscroll) more pronounced than the zoom Review of attachment 8435182 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/apz/src/AsyncPanZoomController.cpp @@ +1867,5 @@ > // composited area, respectively. > > // The maximum proportion of the composition length which can become blank > // space along an axis as we overscroll along that axis. > + const float CLAMPING = gfxPrefs::APZOverscrollClamping(); trailing ws
Attachment #8435182 -
Flags: review?(bugmail.mozilla) → review+
Comment 15•9 years ago
|
||
Comment on attachment 8435183 [details] [diff] [review] Part 5 - Use spring physics for the snap-back animation Review of attachment 8435183 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/apz/src/Axis.cpp @@ +120,5 @@ > float Axis::GetOverscroll() const { > return mOverscroll; > } > > void Axis::StartSnapBack() { If this is empty we might as well remove it and put it back if we need it later. @@ +139,5 @@ > + // b is a constant that provides damping (friction) > + // v is the velocity of the point at the end of the spring > + // See http://gafferongames.com/game-physics/spring-physics/ > + const float kSpringStiffness = gfxPrefs::APZOverscrollSnapBackSpringStiffness(); > + const float kSpringFriction = gfxPrefs::APZOverscrollSnapBackSpringFriction(); In the previous patch you used UPPERCASE for const variables. I don't care whether you do kBlah or UPPERCASE but it would be good to make it consistent one way or the other.
Attachment #8435183 -
Flags: review?(bugmail.mozilla) → review+
Assignee | ||
Comment 16•9 years ago
|
||
Updated to address review comments. Carrying r+.
Attachment #8435187 -
Attachment is obsolete: true
Attachment #8435852 -
Flags: review+
Assignee | ||
Comment 17•9 years ago
|
||
Updated to address review comments. Carrying r+.
Attachment #8435182 -
Attachment is obsolete: true
Attachment #8435854 -
Flags: review+
Assignee | ||
Comment 18•9 years ago
|
||
Updated to address review comments. Carrying r+.
Attachment #8435183 -
Attachment is obsolete: true
Attachment #8435856 -
Flags: review+
Assignee | ||
Comment 19•9 years ago
|
||
green try |
Combined Try push for this bug and bug 1020045: https://tbpl.mozilla.org/?tree=Try&rev=d88bfc86e7f5
Assignee | ||
Comment 20•9 years ago
|
||
landing |
Tweaked some prefs based on the latest feedback from Gordon and Milan, and landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/9315213aa679 https://hg.mozilla.org/integration/mozilla-inbound/rev/e761f600d11a https://hg.mozilla.org/integration/mozilla-inbound/rev/1495e351283e https://hg.mozilla.org/integration/mozilla-inbound/rev/05ce4d3cf94e https://hg.mozilla.org/integration/mozilla-inbound/rev/77d99630aa73
Comment 21•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9315213aa679 https://hg.mozilla.org/mozilla-central/rev/e761f600d11a https://hg.mozilla.org/mozilla-central/rev/1495e351283e https://hg.mozilla.org/mozilla-central/rev/05ce4d3cf94e https://hg.mozilla.org/mozilla-central/rev/77d99630aa73
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•