After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 682428 - plymouth => gdm transition doesn't look very good
plymouth => gdm transition doesn't look very good
Status: RESOLVED DUPLICATE of bug 682429
Product: gnome-shell
Classification: Core
Component: login-screen
3.4.x
Other Linux
: Normal normal
: ---
Assigned To: Ray Strode [halfline]
gnome-shell-maint
: 682148 (view as bug list)
Depends on: 682429 694062
Blocks:
 
 
Reported: 2012-08-22 06:43 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2013-02-18 04:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
layout: Add a fake root pixmap actor at startup, and fade it out (3.68 KB, patch)
2012-08-22 07:42 UTC, Jasper St. Pierre (not reading bugmail)
none Details | Review
gdm: Don't fade in the log in dialog (2.22 KB, patch)
2012-08-22 07:43 UTC, Jasper St. Pierre (not reading bugmail)
none Details | Review
layout: Add a fake root pixmap actor at startup, and fade it out (3.76 KB, patch)
2012-08-22 09:11 UTC, Jasper St. Pierre (not reading bugmail)
needs-work Details | Review
gdm: Don't fade in the log in dialog (2.32 KB, patch)
2012-08-22 09:11 UTC, Jasper St. Pierre (not reading bugmail)
none Details | Review
layout: Add a fake root pixmap actor at startup, and fade it out (3.56 KB, patch)
2012-09-13 18:42 UTC, Jasper St. Pierre (not reading bugmail)
reviewed Details | Review
gdm: Don't fade in the log in dialog (2.32 KB, patch)
2012-09-13 18:42 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
gdm: Don't fade in the log in dialog (18.00 KB, patch)
2012-09-14 18:20 UTC, Ray Strode [halfline]
none Details | Review
layout: Add a fake root pixmap actor at startup, and fade it out (4.87 KB, patch)
2012-09-19 07:10 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
layout: Use a MetaBackgroundActor, not a custom ClutterX11TexturePixmap (4.93 KB, patch)
2012-10-26 15:16 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2012-08-22 06:43:55 UTC
After plymouth exits and Xorg/gdm starts, you see the standard configured window background just appear out of nowhere. After that, we fade to the system noise texture background. On F18, this is very visible: dark gray, to light blue stripes, to dark gray again.

After plymouth exits, it's supposed to leave the contents on the screen. gdm used to (probably still does) save a giant pixmap in _XROOTPMAP_ID for the root window. We should be able to create a TFP texture for that, and fade in our contents on top of that.
Comment 1 Jasper St. Pierre (not reading bugmail) 2012-08-22 07:42:54 UTC
Created attachment 222108 [details] [review]
layout: Add a fake root pixmap actor at startup, and fade it out

This provides us with a smooth transition between plymouth and gdm.

Completely untested, but this is the sort of thing I was talking about.
Comment 2 Jasper St. Pierre (not reading bugmail) 2012-08-22 07:43:01 UTC
Created attachment 222109 [details] [review]
gdm: Don't fade in the log in dialog

Instead, directly show it.

This too.
Comment 3 Jasper St. Pierre (not reading bugmail) 2012-08-22 09:11:12 UTC
Created attachment 222115 [details] [review]
layout: Add a fake root pixmap actor at startup, and fade it out

This provides us with a smooth transition between plymouth and gdm.



Still untested, but doesn't crash the shell anymore. I'd appreciate testing, but I understand that you guys have better things to do at this point.
Comment 4 Jasper St. Pierre (not reading bugmail) 2012-08-22 09:11:28 UTC
Created attachment 222116 [details] [review]
gdm: Don't fade in the log in dialog

Instead, directly show it.
Comment 5 Ray Strode [halfline] 2012-08-22 14:47:03 UTC
we should turn off g-s-d's crossfade behavior as well I suppose.
Comment 6 Elad Alfassa 2012-08-30 14:50:13 UTC
*** Bug 682148 has been marked as a duplicate of this bug. ***
Comment 7 drago01 2012-09-13 12:58:03 UTC
Review of attachment 222115 [details] [review]:

Can't test it either but generally looks fine some comments below.

::: src/shell-global.c
@@ +1758,3 @@
+ * supporting raw xlib types.
+ *
+ * Returns: (transfer none): A #ClutterActor that represents the _XROOTPMAP_ID

Why none?

@@ +1775,3 @@
+
+  if (XGetWindowProperty (global->xdisplay,
+                          xroot_window,

Can just use DefaultRootWindow (global->xdisplay) instead.

@@ +1791,3 @@
+  if (root_pixmap_id != None)
+    {
+      g_print ("%ld\n", root_pixmap_id);

Remove.
Comment 8 Jasper St. Pierre (not reading bugmail) 2012-09-13 18:42:44 UTC
Created attachment 224255 [details] [review]
layout: Add a fake root pixmap actor at startup, and fade it out

This provides us with a smooth transition between plymouth and gdm.
Comment 9 Jasper St. Pierre (not reading bugmail) 2012-09-13 18:42:52 UTC
Created attachment 224256 [details] [review]
gdm: Don't fade in the log in dialog

Instead, directly show it.
Comment 10 drago01 2012-09-13 20:59:52 UTC
Review of attachment 224255 [details] [review]:

Looks good still can't test fine to commit if it works.
Comment 11 drago01 2012-09-13 21:01:00 UTC
Review of attachment 224256 [details] [review]:

OK.
Comment 12 Ray Strode [halfline] 2012-09-14 18:20:24 UTC
Created attachment 224357 [details] [review]
gdm: Don't fade in the log in dialog

Instead, directly show it.
Comment 13 Ray Strode [halfline] 2012-09-14 18:22:31 UTC
Comment on attachment 224256 [details] [review]
gdm: Don't fade in the log in dialog

(ignore my churn on this bug)
Comment 14 Jasper St. Pierre (not reading bugmail) 2012-09-19 04:41:45 UTC
Comment on attachment 224256 [details] [review]
gdm: Don't fade in the log in dialog

Attachment 224256 [details] pushed as cc67440 - gdm: Don't fade in the log in dialog


Review on the plymouth patch would be appreciated.
Comment 15 Jasper St. Pierre (not reading bugmail) 2012-09-19 07:08:40 UTC
(In reply to comment #7)
> Review of attachment 222115 [details] [review]:
> 
> Can't test it either but generally looks fine some comments below.
> 
> ::: src/shell-global.c
> @@ +1758,3 @@
> + * supporting raw xlib types.
> + *
> + * Returns: (transfer none): A #ClutterActor that represents the _XROOTPMAP_ID
> 
> Why none?

Because it's the correct annotation. There's an alias, (transfer floating), that might clear things up. Changed.

Also, I had a chance to test this. It works, but I'm doing things slightly differently now.
Comment 16 Jasper St. Pierre (not reading bugmail) 2012-09-19 07:10:27 UTC
Created attachment 224703 [details] [review]
layout: Add a fake root pixmap actor at startup, and fade it out

This provides us with a smooth transition between plymouth and gdm.
Comment 17 Jasper St. Pierre (not reading bugmail) 2012-09-19 07:11:56 UTC
To clarify, I just ran the transition when restarting the shell, and it worked. I have not tested with real Plymouth, nor have I tested with multiple monitor support.
Comment 18 drago01 2012-09-19 08:10:56 UTC
(In reply to comment #15)
> (In reply to comment #7)
> > Review of attachment 222115 [details] [review] [details]:
> > 
> > Can't test it either but generally looks fine some comments below.
> > 
> > ::: src/shell-global.c
> > @@ +1758,3 @@
> > + * supporting raw xlib types.
> > + *
> > + * Returns: (transfer none): A #ClutterActor that represents the _XROOTPMAP_ID
> > 
> > Why none?
> 
> Because it's the correct annotation. There's an alias, (transfer floating),
> that might clear things up. Changed.

clutter_x11_texture_pixmap_new_with_pixmap does:

-----------
  ClutterActor *actor;

  actor = g_object_new (CLUTTER_X11_TYPE_TEXTURE_PIXMAP,
			"pixmap", pixmap,
			NULL);

  return actor;
-----------

And you are returning that so it ought to be (transfer full) no?
Comment 19 Jasper St. Pierre (not reading bugmail) 2012-09-19 08:15:10 UTC
ClutterActor is a GInitiallyUnowned.

(transfer full) is defined as "transferring the reference", but since the object is floating, there's no reference to transfer.
Comment 20 drago01 2012-09-19 08:42:08 UTC
Review of attachment 224703 [details] [review]:

OK fine then.
Comment 21 Florian Müllner 2012-09-19 10:06:34 UTC
Attachment 224703 [details] pushed as 5e12e5f - layout: Add a fake root pixmap actor at startup, and fade it out
Comment 22 Elad Alfassa 2012-10-09 20:00:00 UTC
I can still see the issue described in the bug description using
gnome-shell-3.6.0-1.fc18.x86_64
gdm-3.6.0-1.fc18.x86_64
Comment 23 Elad Alfassa 2012-10-25 20:57:31 UTC
*** Bug 682148 has been marked as a duplicate of this bug. ***
Comment 24 Elad Alfassa 2012-10-25 20:59:01 UTC
Still an issue, please re-open.

See video:
http://doom.co.il/f18-vm-boot.webm

from bug #682148:
>the problem is gdm itself (or gnome-settings-deamon?)
draws the default background on the screen before gdm draws the gray
background.
Comment 25 Jasper St. Pierre (not reading bugmail) 2012-10-26 15:02:00 UTC
Ouch. Can we disable the background plugin in gnome-settings-daemon in the gdm user? The only issue with that is the lack of proper wallpaper in the shield. I'm not sure how to act with that.
Comment 26 Jasper St. Pierre (not reading bugmail) 2012-10-26 15:16:15 UTC
Created attachment 227368 [details] [review]
layout: Use a MetaBackgroundActor, not a custom ClutterX11TexturePixmap

While looking at how the plymouth implementation was built, I was so
short-sighted and focused on the string "_XROOTPMAP_ID" that I didn't
realize it was the name of the standard background on the root window.
Remove our own implementation, and switch to using a standard mutter
MetaBackgroundActor.



---

This doesn't fix anything, it's just a minor cleanup.
Comment 27 drago01 2012-10-26 15:35:43 UTC
Review of attachment 227368 [details] [review]:

Makes sense.
Comment 28 Jasper St. Pierre (not reading bugmail) 2012-10-26 15:54:48 UTC
Comment on attachment 227368 [details] [review]
layout: Use a MetaBackgroundActor, not a custom ClutterX11TexturePixmap

Attachment 227368 [details] pushed as 85728f0 - layout: Use a MetaBackgroundActor, not a custom ClutterX11TexturePixmap
Comment 29 Ray Strode [halfline] 2012-10-26 17:39:46 UTC
well we shouldn't need the background plugin to draw in non-fallback cases at all really right?  It's not whether or not we're in the gdm session, it's whether or not we're using gnome-shell.
Comment 30 Florian Müllner 2012-10-26 17:46:22 UTC
We still need it for the screen shield, don't we?
Comment 31 Jasper St. Pierre (not reading bugmail) 2012-10-26 18:02:41 UTC
(In reply to comment #29)
> well we shouldn't need the background plugin to draw in non-fallback cases at
> all really right?  It's not whether or not we're in the gdm session, it's
> whether or not we're using gnome-shell.

Unless we want to implement cross-fade and everything with GnomeBg in the shell session, we still need it.
Comment 32 Ray Strode [halfline] 2012-10-26 18:21:39 UTC
right, we definitely want to do that
Comment 33 Ray Strode [halfline] 2012-10-26 18:51:16 UTC
My point is right now, background handling is very rube goldberg, and we should probably just do it directly in shell for the non fallback case.
Comment 34 Giovanni Campagna 2012-12-24 15:09:17 UTC
Ok, it's time to consolidate this.
gnome-settings-daemon's background is (will be) obsoleted by bug 682427, and the plymouth=>gdm animation can be covered in bug 682429.

*** This bug has been marked as a duplicate of bug 682429 ***