you mean solid gray? or the noise texture?

No texture. Very light gray - close to white.

what's the value of

 gsettings get org.gnome.desktop.background primary-color

? How big is your wallpaper image? We changed image loading to be asynchronous. We probably need to introduce a synchronization point at startup.

maybe something like...

Created attachment 237013 [details] [review]
layout: defer startup animation until background is loaded

The user's wallpaper is loaded asynchronously at startup.
If the wallpaper is big then it may take a long time to load.

We don't currently wait for the background to show up before
proceeding with the startup animation.

If the background takes a long time to load, the user will
experience undesirable flicker at startup.

This commit blocks start up until the background is loaded.

Created attachment 237015 [details]
video (mov)

This is what I see immediately after alt+f2 r.

Looked at this Jon today.  This is a regression caused by 


    65303d027aa8615b6aabb16e8d3308e2ee7de446 
    main: defer startup until we know our session mode
    
    commit 92083eaf76fc7a5c2ecdd182896583ab5026ddf0 made
    session mode loading an asynchronous operation.
    
    Aspects of the session mode aren't known immediately at
    start up. For instance, sessionMode.isGreeter returns
    false for greeter sessions until the asynchronous
    operation completes.
    
    This commit defers start up processing until the session
    mode is fully known.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=682429

Now that loading is done asynchronously we iterate the main loop before taking the still frame and so we end up screenshoting a blank stage.

We can fix this by taking the still frames earlier, but we may just want to use the system noise texture instead of doing the still frames at all.  Current designs always show the system noise texture in these transitions anyway.  Then we can get out of the screenshotting business entirely.

Created attachment 237097 [details] [review]
main: don't show stage until still frames are loaded

We sometimes map the stage before we've loaded a background on it
because of a race asynchronously loading the session mode.

This manifests as the startup animating starting over a white
background.

This commit defers showing the stage until after the still frames
are loaded.

Review of attachment 237097 [details] [review]:

::: js/ui/main.js
@@ +198,3 @@
     // completed IO tasks
     GLib.idle_add(GLib.PRIORITY_LOW, function() {
+        global.stage.show();

Where do you really want the stage to be shown? Here or in startupAnimation?

Created attachment 237101 [details] [review]
main: don't show stage until still frames are loaded

We sometimes map the stage before we've loaded a background on it
because of a race asynchronously loading the session mode.

This manifests as the startup animating starting over a white
background.

This commit defers showing the stage until after the still frames
are loaded.

(In reply to comment #9)
> Review of attachment 237097 [details] [review]:
> 
> ::: js/ui/main.js
> @@ +198,3 @@
>      // completed IO tasks
>      GLib.idle_add(GLib.PRIORITY_LOW, function() {
> +        global.stage.show();
> 
> Where do you really want the stage to be shown? Here or in startupAnimation?

Doesn't matter though, both was obviously wrong.

Review of attachment 237101 [details] [review]:

Both were wrong, so you... chose one of them?

Review of attachment 237101 [details] [review]:

Oh, I misunderstood your comment. OK.

I guess we'll hold off on the first patch for now, unless it actually ends up being a problem in practice.

Attachment 237101 [details] pushed as c562245 - main: don't show stage until still frames are loaded

It does not correct the problem:
http://git.gnome.org/browse/gnome-shell/commit/?id=c562245c167cb7b0d027dc5ff3902db4345a12ca

Yosef, you're seeing the same issue as the video in comment 6 ?

The patch definitely fixed Jon's issue, we tried it on his machine.

Yes.
Before Animitziit entrance I see a white background.

and you definitely ran "make install" after updating?

Can you open up the installed main.js and see if the global.stage.hide() is in place?

Definitely.
I use PKGBUILD (Arch Linux), and each update is called 'git pull origin'.

Of course, all the changes were made...

(reopening then)

I found a machine that reproduces, will post patches soon.

Created attachment 237923 [details] [review]
compositor: don't show stage right away

We don't want the stage shown until gnome-shell is ready for it.
This commit ensures the stage isn't shown implicitly.

Created attachment 237924 [details] [review]
compositor: map overlay window before redirecting windows

When windows get redirected off screen, all that gets left behind
is black. We don't want to flicker black at startup, though.

This commit maps the overlay window early, before redirecting
toplevels, so they end up getting snapshotted onto the background
pixmap of the overlay window when the overlay window is mapped.

Created attachment 237925 [details] [review]
main: don't explicitly hide the stage

It's hidden by default, so there's no point in hiding it explicitly.

Created attachment 237926 [details] [review]
main: defer session update until after startup animation

We currently call the session updated handler as soon as
the session modes are read.  This handler sets up overview
keybindings (if a user session) and shows the login dialog
(if a gdm session).

We can't do the latter until the stage is mapped because it
takes a grab, but really we shouldn't do either until the
startup animation is finished.

This commit defers processing session updates until the
start up animation is finished.

It fixes a race condition at login screen startup now that
we don't show the stage right away.

Review of attachment 237926 [details] [review]:

::: js/ui/main.js
@@ +197,3 @@
     layoutManager.connect('startup-prepared',
                           Lang.bind(this, function() {
                               layoutManager.startupAnimation();

Despite me saying in the commit message that this defers until the animation finishes, i'm actually just deferring until the animation starts. That's fine for this bug, but wrong for bug 694837 which I was hoping to fix as a side-effect.

Created attachment 237977 [details] [review]
main: defer session update until startup animation

We currently call the session updated handler as soon as
the session modes are read.  This handler sets up keybindings
for leaving the overview (if a user session) and shows the
login dialog (if a gdm session).

We can't do the latter until the stage is mapped because it
takes a grab, and we don't need to do the former until the
user goes into the overview.

This commit defers processing session updates until the
the layout manager says start up is prepared.

It fixes a race condition at login screen startup now that
we don't show the stage right away.

we'll have fix bug 694837 separately. My initial impression of _sessionUpdated was wrong, so i reattached the patch above with a more accurate commit message.

Created attachment 238035 [details] [review]
main: Merge two different session-mode-updated handlers

Created attachment 238036 [details] [review]
Defer initializing UI until after the global session has loaded

Review of attachment 238035 [details] [review]:

sure.

::: js/ui/main.js
@@ +86,3 @@
         screenShield.showDialog();
+
+    _loadDefaultStylesheet();

So you start to fix this in the next patch, but a whole bunch of stuff that was getting called before screenShield.showDialog isn't getting called until after screenShield.showDialog now.

It's probably okay since at the end of the set you maintain behavior from before the set, but I'd either squash the two patches together or make a note that things are temporarily broken and will get corrected by the next commit.

@@ +113,3 @@
 function startSession() {
+    sessionMode.connect('updated', _sessionUpdated);
+    _sessionUpdated();

If you do go with the two-separate-commits route, i probably wouldn't move these lines in this commit. You're just going to move them again in the next commit, and it isn't right to call them this early given what sessionUpdated does, and moving them makes the git log -p output harder to read.

Review of attachment 238036 [details] [review]:

a pretty nice clean up.

::: js/ui/main.js
@@ +84,3 @@
                                   sessionMode.hasRunDialog ? openRunDialog : null);
     if (sessionMode.isGreeter)
         screenShield.showDialog();

So we're still going to call screenShield.showDialog before we're ready. If we land this then we need to come up with a solution for that.

feel free to push these if you want, but one of us will need to update attachment 237977 [details] [review] afterward.

Review of attachment 237923 [details] [review]:

OK.

Review of attachment 237924 [details] [review]:

OK.

Review of attachment 237925 [details] [review]:

OK.

Review of attachment 237977 [details] [review]:

If we do this we'd have to revert http://git.gnome.org/browse/gnome-shell/commit/?id=f644bee83195f7d094ed75e8d8e2b414be39cc96 otherwise the problem it fixed will be back because we don't have a stage at this point again.

Attachment 237923 [details] pushed as 160150d - compositor: don't show stage right away
Attachment 237924 [details] pushed as e15bc37 - compositor: map overlay window before redirecting windows

(In reply to comment #37)
> Review of attachment 237977 [details] [review]:
> 
> If we do this we'd have to revert
> http://git.gnome.org/browse/gnome-shell/commit/?id=f644bee83195f7d094ed75e8d8e2b414be39cc96
> otherwise the problem it fixed will be back because we don't have a stage at
> this point again.

talked with drago01 on irc about this.  The stage is getting realized by mutter in meta_compositor_manage_screen so I don't think we need to revert that commit. the function only sets some window properties on the stage window.

I did some dnd testing by dragging a url from firefox to the overview and the overview opened as expected.

(leaving bug open since Jasper still has those clean ups in mind)

Attachment 237925 [details] pushed as 5e0ff7f - main: don't explicitly hide the stage
Attachment 237977 [details] pushed as 260c082 - main: defer session update until startup animation

Created attachment 238084 [details] [review]
main: Merge two different session-mode-updated handlers

Created attachment 238085 [details] [review]
Defer initializing UI until after the global session has loaded

Review of attachment 238084 [details] [review]:

sure

Review of attachment 238085 [details] [review]:

looks fine except for one nit

::: js/ui/main.js
@@ +116,3 @@
+    // Make sure the default stylesheet is loaded when we
+    // construct the UI.
+    _loadDefaultStylesheet();

This is already called a few lines lower in the function

Comment on attachment 238084 [details] [review]
main: Merge two different session-mode-updated handlers

Attachment 238084 [details] pushed as 0ba1f29 - main: Merge two different session-mode-updated handlers

Created attachment 238235 [details] [review]
Defer initializing UI until after the global session has loaded

Review of attachment 238235 [details] [review]:

++

Attachment 238235 [details] pushed as 0bef925 - Defer initializing UI until after the global session has loaded

The commit 0bef925b5146794220a552f3422b0c0616466425 (https://git.gnome.org/browse/gnome-shell/commit/?id=0bef925b5146794220a552f3422b0c0616466425) causes gdm crash.

Created attachment 238260 [details] [review]
main: Remove leftover hunk connection to sessionUpdated

We shouldn't connect to sessionUpdated twice.

Created attachment 238261 [details] [review]
layout: Move showing the stage and the startup-prepared signal

Waiting until we're idle means nothing if we're constructing
complex actors.

Created attachment 238262 [details] [review]
main: Show the greeter dialog when we're prepared

If we don't wait until the stage is mapped, pushing a modal
will fail with an X error.

Review of attachment 238261 [details] [review]:

seems okay.

::: js/ui/layout.js
@@ +604,3 @@
     },
 
+    _startupAnimation: function() {

I like that you're making this private, but for symmetry maybe prepareStartupAnimation should be private too.

Review of attachment 238262 [details] [review]:

yup, this should do the trick i think.

Attachment 238260 [details] pushed as de2f2d7 - main: Remove leftover hunk connection to sessionUpdated
Attachment 238261 [details] pushed as e2463cb - layout: Move showing the stage and the startup-prepared signal
Attachment 238262 [details] pushed as 65067c2 - main: Show the greeter dialog when we're prepared

... and more breakage, reopening.

Created attachment 238398 [details] [review]
main: Do not export DBus interfaces before initializing the UI

Since commit 7cdb75e7ce76, initializing UI is deferred until the session
mode has been loaded. However DBus is still initialized immediately,
which means that for DBus methods that access properties in Main, there
is now a window between the method being exposed on the bus and the
method being ready to be called. At least g-s-d grabbing global keybindings
is likely to fall in this window on session startup, and almost guaranteed
when regrabbing bindings after a shell restart.
To fix, defer initializing DBus as well.

DBus on startup is still broken -- we grab the bus names, but don't export methods on them.

Yes, and g-s-d has a workaround for that. It's unfortunate, but not an excuse for breaking it even more.

But the reason to grab the bus names early ceased to exists: gnome-session doesn't watch on bus names, it uses the XDCMP registration and direct PID watching to monitor us. So can't we move the ShellGlobal code to register names later in the startup sequence (or move it to JS entirely)?

(In reply to comment #60)
> So can't we move the ShellGlobal code to register names later in the 
> startup sequence (or move it to JS entirely)?

Probably, but it's really a secondary question here - even if we only grab the name when initializing ShellDBus, any method that uses stuff from Main will still be broken until _initializeUI() has been called.

Alternatively we could revert all patches following comment #40.

Yeah, of course, I didn't mean to replace your last patch, but to augment it, by moving the bus name grabbing to happen after we register objects.
Btw, if we only ever get the name after startup, we can init ShellDBus at any time, nobody will call it anyway.

Ping? This needs to be fixed before 3.7.92 ...

Review of attachment 238398 [details] [review]:

OK.

Attachment 238398 [details] pushed as ec014a7 - main: Do not export DBus interfaces before initializing the UI