Closed Bug 619626 Opened 9 years ago Closed 9 years ago

make splash screen faster

Categories

(Core :: Widget: Android, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
fennec 2.0b4+ ---

People

(Reporter: blassey, Assigned: blassey)

References

Details

Attachments

(3 files, 3 obsolete files)

Attached patch patch (obsolete) — Splinter Review
we're taking a Ts hit from the current splash screen dialog
Attachment #498043 - Flags: review?(mwu)
tracking-fennec: --- → ?
OS: Linux → Android
Hardware: x86_64 → ARM
Screenshot for Madhava?
tracking-fennec: ? → 2.0b4+
Attached image screen shot (obsolete) —
Assignee: nobody → madhava
Attached image screenshot
Assignee: madhava → blassey.bugs
Attachment #498045 - Attachment is obsolete: true
Attachment #498230 - Flags: ui-review?(madhava)
Attached patch patch (obsolete) — Splinter Review
Attachment #498043 - Attachment is obsolete: true
Attachment #498231 - Flags: review?(mwu)
Attachment #498043 - Flags: review?(mwu)
this adds the resource stuff, but I assume we want to include final images here
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?
Attachment #498231 - Flags: review?(mwu)
Comment on attachment 498231 [details] [diff] [review]
patch

r+ with review comments addressed.
Attachment #498231 - Flags: review+
Attachment #498233 - Flags: review+
Depends on: 621484
Attached patch patchSplinter Review
I reworked this a bit, so wanted to get another review
Attachment #498231 - Attachment is obsolete: true
Attachment #499804 - Flags: review?(mwu)
Attachment #499804 - Flags: review?(mwu) → review+
pushed http://hg.mozilla.org/mozilla-central/rev/2a25c4d1ed99
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Blocks: 627557
Attachment #498230 - Flags: ui-review?(madhava)
You need to log in before you can comment on or make changes to this bug.