make splash screen faster

RESOLVED FIXED

Status

()

Core
Widget: Android
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: blassey, Assigned: blassey)

Tracking

Trunk
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(fennec2.0b4+)

Details

Attachments

(3 attachments, 3 obsolete attachments)

Created attachment 498043 [details] [diff] [review]
patch

we're taking a Ts hit from the current splash screen dialog
Attachment #498043 - Flags: review?(mwu)
(Assignee)

Updated

7 years ago
tracking-fennec: --- → ?
(Assignee)

Updated

7 years ago
OS: Linux → Android
Hardware: x86_64 → ARM
Screenshot for Madhava?
tracking-fennec: ? → 2.0b4+
Created attachment 498045 [details]
screen shot
Assignee: nobody → madhava
Created attachment 498230 [details]
screenshot
Assignee: madhava → blassey.bugs
Attachment #498045 - Attachment is obsolete: true
Attachment #498230 - Flags: ui-review?(madhava)
Created attachment 498231 [details] [diff] [review]
patch
Attachment #498043 - Attachment is obsolete: true
Attachment #498231 - Flags: review?(mwu)
Attachment #498043 - Flags: review?(mwu)
Created attachment 498233 [details] [diff] [review]
m-b resource patch

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

Comment 6

7 years ago
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 7

7 years ago
Comment on attachment 498231 [details] [diff] [review]
patch

r+ with review comments addressed.
Attachment #498231 - Flags: review+
Attachment #498233 - Flags: review+
(Assignee)

Updated

7 years ago
Depends on: 621484
Created attachment 499804 [details] [diff] [review]
patch

I reworked this a bit, so wanted to get another review
Attachment #498231 - Attachment is obsolete: true
Attachment #499804 - Flags: review?(mwu)

Updated

7 years ago
Attachment #499804 - Flags: review?(mwu) → review+
pushed http://hg.mozilla.org/mozilla-central/rev/2a25c4d1ed99
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Blocks: 627557
(Assignee)

Updated

7 years ago
Attachment #498230 - Flags: ui-review?(madhava)
You need to log in before you can comment on or make changes to this bug.