619626 - make splash screen faster

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Android, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

we're taking a Ts hit from the current splash screen dialog

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Screenshot for Madhava?

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: screen shot

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: screenshot

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: m-b resource patch

this adds the resource stuff, but I assume we want to include final images here

Comment 6

[Michael Wu [:mwu]]

Comment on attachment 498231 [details] [diff] [review]
patch

>@@ -69,7 +71,7 @@ abstract public class GeckoApp
>     public static GeckoSurfaceView surfaceView;
>     public static GeckoApp mAppContext;
>     public static boolean mFullscreen = false;
>-    ProgressDialog mProgressDialog;
>+    boolean mShowingSplashScreen;

This doesn't work as you'd expect. This must be static, otherwise if the user rotates the phone while the splash screen is showing, the object that your surface holder callback looks at for mShowingSplashScreen will never see mShowingSplashScreen == false; Consequently, this means the first GeckoApp object is leaked and a flash of splash screen occurs every time the user changes orientation. It's a good way to insert subliminal messages though.

>@@ -114,12 +116,50 @@ abstract public class GeckoApp
>                                }).show();
>     }
> 
>+    void showSplashScreen(SurfaceHolder holder, int width, int height) {

I recommend naming this drawSplashScreen as the (show|hide)SplashScreen pair suggests that that some state is being set in this pair of functions, whereas hideSplashScreen is just a method without a counterpart.
Also don't see an advantage in using hideSplashScreen() has over mShowingSplashScreen = false; so we should be able to get rid of that.

>+        Canvas c = holder.lockCanvas();
>+        if (c == null)
>+            return;
>+        Resources res = getResources();
>+        c.drawColor(res.getColor(R.color.splash_background));
>+        Drawable drawable = res.getDrawable(R.drawable.splash);
>+        int w = drawable.getIntrinsicWidth();
>+        int h = drawable.getIntrinsicHeight();
>+        int x = (width - w)/2;
>+        int y = (height - h)/2;
>+        drawable.setBounds(x, y, x + w, y + h);
>+        drawable.draw(c);
>+        Paint p = new Paint();
>+        p.setTextAlign(Paint.Align.CENTER);
>+        p.setTextSize(32f);
>+        p.setColor(res.getColor(R.color.splash_font));
>+        c.drawText(getString(R.string.splash_screen_label), width/2, y + h + 32, p);
>+        holder.unlockCanvasAndPost(c);
>+    }
>+
>+    void hideSplashScreen()
>+    {
>+        mShowingSplashScreen = false;
>+    }
>+
>     // Returns true when the intent is going to be handled by gecko launch
>     boolean launch(Intent i)
>     {
>         if (!checkAndSetLaunchState(LaunchState.Launching, LaunchState.Launched))
>             return false;
> 
>+        mShowingSplashScreen = true;
>+        showSplashScreen(surfaceView.getHolder(), surfaceView.mWidth, surfaceView.mHeight);
>+        surfaceView.getHolder().addCallback(new SurfaceHolder.Callback() {
>+            public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {
>+                if (mShowingSplashScreen)
>+                    showSplashScreen(holder, width, height);
>+                
>+            }
>+            public void surfaceCreated(SurfaceHolder holder) {}
>+            public void surfaceDestroyed(SurfaceHolder holder) {}
>+        });
>+

How about moving the check for mShowingSplashScreen to GeckoSurfaceView?

Comment 7

[Michael Wu [:mwu]]

Comment on attachment 498231 [details] [diff] [review]
patch

r+ with review comments addressed.

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

I reworked this a bit, so wanted to get another review

Comment 9

[Brad Lassey [:blassey] (use needinfo?)]

pushed http://hg.mozilla.org/mozilla-central/rev/2a25c4d1ed99