GNOME Bugzilla – Bug 764898
gnome-shell aborts with nvidia drivers since 3.20 (due to cogl)
Last modified: 2016-06-28 13:36:43 UTC
Created attachment 325731 [details] [review] Patch to fix problem With gnome 3.20, some deprecated cogl functions have been replaced by non-deprecated functions. This breaks systems with nvidia drivers, where cogl cannot create framebuffer objects when gdm/gnome-shell are not the active display. This is an issue that I've only seen with nvidia drivers so far, but as the code tries to handle this case in an invalid way, it should be fixed in gnome-shell. Attached is a patch that describes and fixes the bug in gnome-shell. More information about this bug can be found in https://bugs.archlinux.org/task/48772
Hereby I confirm 1) I'm affected by the issue addressed by the patch 2) this patch makes it go away See https://bugzilla.opensuse.org/show_bug.cgi?id=976871 for details.
Review of attachment 325731 [details] [review]: Thanks for the patch! Try to re-write the commit subject and message so that they fit under ~70 characters wide. The subject should also be prefixed with the subsystem, in this case st: ... ::: src/st/st-theme-node-drawing.c @@ +2247,3 @@ COGL_TEXTURE_NO_SLICING, COGL_PIXEL_FORMAT_ANY); if (buffer != COGL_INVALID_HANDLE) Can you please cleanup this code? If there's no buffer here, return immediately. @@ +2251,3 @@ + CoglError *error = NULL; + + offscreen = cogl_offscreen_new_with_texture (buffer); This may fail so we still need to check the return value. Free the buffer and return. @@ +2271,3 @@ + else + { + cogl_handle_unref (offscreen); Try to coalesce as much of these unrefs as possible, use a goto if needed. ::: src/st/st-theme-node-transition.c @@ +277,3 @@ + cogl_error_free (catch_error); + priv->old_offscreen = COGL_INVALID_HANDLE; + g_return_val_if_fail (priv->old_offscreen != COGL_INVALID_HANDLE, FALSE); These g_return_val_if_fail() were already there, but they're actually used incorrectly. This isn't a programming error, it's a legit runtime error so there's no point in using this (and printing warnings to stderr continuously). Just return FALSE. And while doing it here, do it above too for ->old/new_texture. @@ +283,3 @@ cogl_handle_unref (priv->new_offscreen); priv->new_offscreen = cogl_offscreen_new_with_texture (priv->new_texture); Please be consistent and don't leave an empty line here since you didn't leave one previously either.
Created attachment 328076 [details] [review] Reworked patch #1 As working with a desktop that is crashing every now and then is not funny, I took the go at it. Please find a reworked patch attached for review.
Review of attachment 328076 [details] [review]: looks good, thanks, can you push? ::: src/st/st-theme-node-drawing.c @@ +2271,1 @@ + offscreen = cogl_offscreen_new_with_texture (buffer); I got this one wrong previously: indeed, it can't fail
Pushed, thanks.
Pushing to the sable branch as well: 3bbf681..35cc224 gnome-3-20 -> gnome-3-20
Is the fix really included in Gnome Shell 3.20 ? Because I, and some others, have the problem on Fedora 24 up to date. https://bugzilla.redhat.com/show_bug.cgi?id=1350184 http://forums.fedoraforum.org/showthread.php?p=1764275 http://forums.fedoraforum.org/showthread.php?p=1764805 Thanks !
(In reply to jeremy9856 from comment #7) > Is the fix really included in Gnome Shell 3.20 ? It is on the gnome-3-20 branch, but not in a released 3.20.x tarball. We have a couple of fixes piled up for both mutter and gnome-shell, so I'll do stable releases for those today or tomorrow.
Great ! Thank you very much !