Last Comment Bug 704784 - Only show scrollbars while panning (finger down), fade away on finger up
: Only show scrollbars while panning (finger down), fade away on finger up
Status: VERIFIED FIXED
: uiwanted
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: P2 normal (vote)
: ---
Assigned To: away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com)
:
: Sebastian Kaspari (:sebastian)
Mentors:
Depends on: 701351
Blocks: 709560
  Show dependency treegraph
 
Reported: 2011-11-23 02:54 PST by Steffen Wilberg
Modified: 2016-07-29 14:20 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
11+


Attachments
(1/2) make the scrollbars look nicer (7.67 KB, patch)
2011-12-09 08:26 PST, away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com)
pwalton: review+
Details | Diff | Splinter Review
(2/2) fade scrollbars away (7.67 KB, patch)
2011-12-09 08:27 PST, away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com)
pwalton: review-
Details | Diff | Splinter Review
(2/2) fade scrollbars away (12.54 KB, patch)
2011-12-09 12:05 PST, away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com)
pwalton: review+
Details | Diff | Splinter Review
(1/2) make the scrollbars look nicer (8.67 KB, patch)
2011-12-09 13:57 PST, away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com)
bugmail: review+
Details | Diff | Splinter Review
(2/2) fade scrollbars away (12.80 KB, patch)
2011-12-09 14:01 PST, away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com)
bugmail: review+
Details | Diff | Splinter Review

Description Steffen Wilberg 2011-11-23 02:54:24 PST
Mark Finkle (:mfinkle) said in bug 701351 comment 12:
> We need the scrollbars to fade away after panning is finished.Finger down == scrollbars visible, Finger up == scrollbars fade away.
Comment 1 Steffen Wilberg 2011-11-23 03:23:37 PST
(That didn't wrap nicely)

We need the scrollbars to fade away after panning is finished.
Finger down == scrollbars visible, Finger up == scrollbars fade away.
Comment 2 Madhava Enros [:madhava] 2011-11-30 08:35:20 PST
We need to establish the look here; I think we just want to emulate some generation (gingerbread) of native ones?
Comment 3 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2011-11-30 08:44:45 PST
wesj pointed me to http://developer.android.com/reference/android/view/View.html#awakenScrollBars%28%29 which might let us just use the built-in scrollbars if we can get them to size correctly. I totally missed that when I looked for android scrollbars previously, so we might not need graphics after all.
Comment 4 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2011-12-06 11:23:57 PST
I tried a bunch to get the built-in scrollbars working, but I was unsuccessful. The main reason is that we are using a GLSurfaceView, and instead of drawing itself, it delegates drawing to a GLSurfaceRenderer, so the code flow is completely different. The draw code in the view itself is never used and so the scrollbars are never drawn, even after I enabled them and got awakenScrollbars() to return true.

I'm going to have to fall back to our scrollbar layers, but make them look nicer.
Comment 5 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2011-12-09 08:26:19 PST
Created attachment 580421 [details] [diff] [review]
(1/2) make the scrollbars look nicer
Comment 6 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2011-12-09 08:27:56 PST
Created attachment 580423 [details] [diff] [review]
(2/2) fade scrollbars away
Comment 7 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2011-12-09 08:31:27 PST
I chose viewport/page size changes as the trigger rather than touch events so the scrollbars stay up during flings and if the page does a scrollTo, as per discussionn with Madhava yesterday.
Comment 8 Patrick Walton (:pcwalton) 2011-12-09 11:49:44 PST
Comment on attachment 580421 [details] [diff] [review]
(1/2) make the scrollbars look nicer

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

r=me with changes

::: mobile/android/base/gfx/ScrollbarLayer.java
@@ +87,5 @@
>          return new ScrollbarLayer(image, vertical);
>      }
>  
> +    /* Returns the power of two that is greater than or equal to value */
> +    private int getPowerOfTwo(int value) {

A faster way:

http://acius2.blogspot.com/2007/11/calculating-next-power-of-2.html

Cwiiis needed this function too. Might want to factor this out into a protected static method of Layer.

@@ +114,5 @@
>  
> +            float viewHeight = context.viewport.height();
> +
> +            // for the body of the scrollbar, we take a 1x1 pixel from the center of the image ...
> +            int[] cropRect = new int[] { CAP_RADIUS, CAP_RADIUS, 1, 1 };

Could you factor these out into private static finals, since they're constant? Would save a few allocations and lower GC pressure a bit.
Comment 9 Patrick Walton (:pcwalton) 2011-12-09 11:50:46 PST
Comment on attachment 580423 [details] [diff] [review]
(2/2) fade scrollbars away

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

This is the same as the first patch -- did you upload the wrong one?
Comment 10 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2011-12-09 12:05:24 PST
Created attachment 580495 [details] [diff] [review]
(2/2) fade scrollbars away

Upload correct patch.
Comment 11 Patrick Walton (:pcwalton) 2011-12-09 12:24:31 PST
Comment on attachment 580495 [details] [diff] [review]
(2/2) fade scrollbars away

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

r=me with changes

::: mobile/android/base/gfx/LayerRenderer.java
@@ +128,5 @@
>          LayerController controller = mView.getController();
>          Layer rootLayer = controller.getRoot();
>          RenderContext screenContext = createScreenContext(), pageContext = createPageContext();
>  
> +        if (! pageContext.fuzzyEquals(mLastPageContext)) {

nit: if (!pageContext.fuzzyEquals(mLastPageContext))

@@ +277,5 @@
>          }
>      }
> +
> +    class FadeRunnable implements Runnable {
> +        private boolean mStartScheduled;

Maybe "mStarted" would be clearer?

@@ +282,5 @@
> +        private long mRunAt;
> +
> +        void scheduleStartFade(long delay) {
> +            mRunAt = SystemClock.elapsedRealtime() + delay;
> +            if (! mStartScheduled) {

nit: if (!mStartScheduled)

::: mobile/android/base/gfx/ScrollbarLayer.java
@@ +100,5 @@
>          }
>          return value * 2;
>      }
>  
> +    public boolean fade() {

Please document what the return value of this means.
Comment 12 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2011-12-09 13:57:15 PST
Created attachment 580537 [details] [diff] [review]
(1/2) make the scrollbars look nicer

Updated as per comments, carrying forward r+
Comment 13 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2011-12-09 14:01:45 PST
Created attachment 580541 [details] [diff] [review]
(2/2) fade scrollbars away

Updated as per comments, carrying forward r+
Comment 14 away[Nov24,Dec5) Kartikaya Gupta (email:kats@mozilla.com) 2011-12-09 14:02:25 PST
Landed on inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/daeda7b6d249
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8dad67cd332
Comment 16 Camelia Urian 2011-12-12 00:30:03 PST
Build ID: Mozilla/5.0 (Android; Linux armv7l; rv:11.0a1) Gecko/20111211 Firefox/11.0a1 Fennec/11.0a1
Device: Samsung Nexus S
OS: Android 2.3

Scrollbars are displayed while panning (finger down), scrollbars fade away on finger up.
Marking bug as verified.

Note You need to log in before you can comment on or make changes to this bug.