Created attachment 249411 [details] [review]
remote-display: if llvmpipe was detected, disable animations

If _GNOME_IS_SOFTWARE_RENDERING X Atom is detected on root window,
disable animations.

Created attachment 249412 [details] [review]
check-accelerated: check for llvmpipe

Check if running under llvmpipe and set Atom
_GNOME_IS_SOFTWARE_RENDERING to 1 in that case.

Review of attachment 249412 [details] [review]:

::: tools/gnome-session-check-accelerated-helper.c
@@ +278,3 @@
         if (_is_gl_renderer_blacklisted (renderer))
                 goto out;
+        has_llvmpipe = renderer && strcasestr (renderer, "llvmpipe");

has_llvmpipe is already assigned a default value, so I'd use:
if (renderer && strcasestr (renderer, "llvmpipe"))
  has_llvmpipe = TRUE;
which is more readable.

@@ +447,3 @@
+        if (has_llvmpipe) {
+                _print_error ("llvmpipe detected.");
+                ret = 2;

The ret needs to be defined as a constant in a shared header, I don't like magic numbers.

::: tools/gnome-session-check-accelerated.c
@@ +168,3 @@
                 g_clear_error (&error);
         } else {
+                is_accelerated = (WEXITSTATUS(estatus) == 0) || (WEXITSTATUS(estatus) == 2);

Can you make the WEXITSTATUS() changes separately?

@@ +182,3 @@
 
+        if (is_software_rendering) {
+            XChangeProperty (GDK_DISPLAY_XDISPLAY (display),

Indentation.

Review of attachment 249411 [details] [review]:

Looks good otherwise, pending the gnome-session patch.

::: plugins/remote-display/gsd-remote-display-manager.c
@@ +219,3 @@
 	}
 
+	/* disable animation if running under llvmpipe */

animations.

Reassigning to gnome-session to get reviews done there, as the gnome-settings-daemon part is complete.

Created attachment 249495 [details] [review]
remote-display: if llvmpipe was detected, disable animations

If _GNOME_IS_SOFTWARE_RENDERING X Atom is detected on root window,
disable animations.

Comment on attachment 249495 [details] [review]
remote-display: if llvmpipe was detected, disable animations

fixed comment. Will wait until gnome-session patch is accepted before landing

so, one issue is....  we definitely want animations if a user is using a local guest (say in boxes)

(In reply to comment #8)
> so, one issue is....  we definitely want animations if a user is using a local
> guest (say in boxes)

why ? On physical hardware as well as on some VM with llvmpipe only, my tests shows GNOME 3 is barely usable with animations...

hmm, well on my machine animations are pretty fluid when running with software rendering.  I guess, maybe, the defining characteristic isn't really "uses software rendering" but is "can keep a good framerate" ?

Reviewers tend to use virtual machines when writing reviews, so we really should try to make the stock experience in a virtual machine as close to the non-virtual machine experience as possible.

Though, maybe this kind of patch is fine and the real fix is something like this:

http://airlied.livejournal.com/77553.html

Created attachment 250044 [details] [review]
check-accelerated: move exit code for helper to separate header

Created attachment 250045 [details] [review]
check-accelerated: fix indentation

Created attachment 250046 [details] [review]
check-accelerated: check for llvmpipe

Check if running under llvmpipe and set Atom
_GNOME_IS_SOFTWARE_RENDERING to 1 in that case.

Attachment 250044 [details] pushed as 74c0294 - check-accelerated: move exit code for helper to separate header
Attachment 250045 [details] pushed as 2b5a5b9 - check-accelerated: fix indentation
Attachment 250046 [details] pushed as 529956f - check-accelerated: check for llvmpipe

Comment on attachment 249495 [details] [review]
remote-display: if llvmpipe was detected, disable animations

Attachment 249495 [details] pushed as 14db162 - remote-display: if llvmpipe was detected, disable animations

Well, 3.14.0 is no more testable in virtualbox+nxclient.
I think it's yet another regression.

(In reply to comment #16)
> Well, 3.14.0 is no more testable in virtualbox+nxclient.
> I think it's yet another regression.

I don't think it's relevant to this bug.

The last line of ~/.xsession-errors:0 upon "oh no" reads like this:

gnome-session-is-accelerated: llvmpipe detected.

(In reply to comment #18)
> The last line of ~/.xsession-errors:0 upon "oh no" reads like this:
> 
> gnome-session-is-accelerated: llvmpipe detected.

So? Unless you have a crash report that's directly relevant to this patch, it's unlikely that this patch is causing this.

How do I obtain that if the session bails out "as intended" instead of chugging on through software renderer as it used to do for a few preceding releases?

Note that I'm not a GNOME user for multiple reasons but just someone who builds and releases en.altlinux.org/regular bunch of LiveCDs with various DEs, and gnome3 one that gets built this Wednesday will bring GNOME 3.14 to the users but those lacking accelerated rendering won't be able to have a look at it.

Our packager gave me the link to this bugreport so I chose to at least provide feedback about another corner case that might be found "inappropriate" or whatever.  It is up to you guys.  Good luck, anyways :)