Created attachment 332444 [details] [review]
nautilus-desktop: draw background when screen is not composited

At least GNOME Flashback session with Metacity still can be used
without compositing manager. Since transparency is not available
nautilus currently draw black background.

Fix it by checking if compositing manager is running and if it is
not then get background surface from root window and draw it on
nautilus desktop window.

Hey,

Took me some time to find out since the comment was in an unrelated bug report, but Cosimo already answered you in https://bugzilla.gnome.org/show_bug.cgi?id=765159#c22

I added the explanation in the first bug report requesting this. Let's mark this one as duplicate.

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

(In reply to Carlos Soriano from comment #2)
> Took me some time to find out since the comment was in an unrelated bug
> report, but Cosimo already answered you in
> https://bugzilla.gnome.org/show_bug.cgi?id=765159#c22

That was answer to question about restoring background drawing if --background command line option is passed. This patch is about only "drawing" background if compositing manager is not available...

(In reply to Cosimo Cecchi from comment #22)
> Longer answer: it's up to the GNOME desktop shell to render the background.
> If you are not running GNOME shell (e.g. the old fallback mode scenario), it
> would be the responsibility of gnome-settings-daemon's old background plugin
> to render it. If you also are not running gnome-settings-daemon, then you

gnome-settings-daemon does not draw background these days. But I think it does not matter if background is rendered from gnome-settings-daemon or gnome-flashback or any other app.

The problem is that when compositing manager is not available nautilus desktop window is no longer transparent - it is impossible to see rendered background unless desktop window is closed. You see icons on black window...

> most likely are also not using gnome-control-center to set the desktop
> background (e.g. you use another DE), so there's no point in having nautilus
> read those settings to draw the background.

We do use gnome-settings-daemon and gnome-control-center. :) And patch attached to this bug report does not touch any settings.

My patch adds code to check if composting manager is available. If it is not then it reads background from root window and renders it on nautilus desktop window.

Carlos and/or Cosimo, can you please answer to my last comment?

Review of attachment 332444 [details] [review]:

Hey, sorry I forgot about this now that we are close to the UI freeze and work piles up :)

I'm fine with it since the code for the desktop is split, but I would like to hear from Cosimo.

Review of attachment 332444 [details] [review]:

Yeah, I'm not at all opposed to something like this, especially now that the desktop is a separate program.
Some minor comments on the patch, but looks good otherwise.

::: nautilus-desktop/nautilus-desktop-window.c
@@ -1,1 @@
-

Looks like a spurious change

@@ +61,3 @@
+	GdkScreen *screen = gdk_screen_get_default ();
+
+	if (window->details->surface) {

Use g_clear_pointer()

@@ +95,3 @@
+{
+	NautilusDesktopWindow *window = NAUTILUS_DESKTOP_WINDOW (widget);
+	GdkAtom gdkatom;

You can fetch the screen from the widget

@@ +165,3 @@
 
+	if (window->details->composited == FALSE) {
+		GdkScreen *screen = gdk_screen_get_default ();

You could fetch the GdkScreen from the widget instance directly.

@@ +166,3 @@
+	if (window->details->composited == FALSE) {
+		GdkScreen *screen = gdk_screen_get_default ();
+		GdkWindow *root = gdk_screen_get_root_window (screen);

Nautilus code style usually mandates variable declarations at the beginning of the functions instead of nested inside other blocks (multiple occurrences throughout the patch).

@@ +171,3 @@
+	}
+
+	if (window->details->surface) {

Use g_clear_pointer()

@@ +326,3 @@
 				     "nautilus-desktop-window");
 
+	window->details->composited = TRUE;

This should not be necessary. Or if it is, fetch the real value from the GdkScreen

Created attachment 333375 [details] [review]
nautilus-desktop: draw background when screen is not  composited

Review of attachment 333375 [details] [review]:

LGTM!

(In reply to Cosimo Cecchi from comment #6)
> Review of attachment 332444 [details] [review] [review]:
> @@ +326,3 @@
>  				     "nautilus-desktop-window");
>  
> +	window->details->composited = TRUE;
> 
> This should not be necessary. Or if it is, fetch the real value from the
> GdkScreen

Now I remember why it was set to TRUE...

The idea was:
- if compositor is available then we do nothing
- if compositor is not available then we setup background.

By using FALSE and/or value from GdkScreen background will not be setup, because of `if (window->details->composited == composited) return;`

Will prepare patch to change back it to TRUE.

Created attachment 333618 [details] [review]
nautilus-desktop: initialize composited to TRUE

Review of attachment 333618 [details] [review]:

Makes sense, feel free to push!