GNOME Bugzilla – Bug 629616
crash in cogl when run firefox from overview
Last modified: 2010-09-24 15:32:09 UTC
to reproduce: 1. open overview 2. type firefox; enter (don't crash if select it from APPLICATIONS menu) 3. crash before crash appear warning: (./cogl-material.c:676):_cogl_material_update_layers_cache: code should not be reached appear after commit 862f1ea18ceee3e652c02303beee35a7c82120f5
(gdb) bt
+ Trace 223712
I can reproduce that here ...
(In reply to comment #2) > I can reproduce that here ... Maxim: are you on nvidia as well? I cannot reproduce on intel ...
(In reply to comment #3) > (In reply to comment #2) > > I can reproduce that here ... > > Maxim: are you on nvidia as well? I cannot reproduce on intel ... I can reproduce it on both intel and nvidia ...
(In reply to comment #3) > Maxim: are you on nvidia as well? I cannot reproduce on intel ... yes. nvidia 195.36.31. Fedora 13 x86_64
*** Bug 630366 has been marked as a duplicate of this bug. ***
This is a Cogl bug. What is basically going on is that if the opacity of a transitioning theme node changes during the transition, we change the primary color of the CoglMaterial, but don't touch the material layer (http://git.gnome.org/browse/gnome-shell/tree/src/st/st-theme-node-transition.c#n325) Cogl then: - Resets the parent of the material to point something different than the template material - Doesn't change material->n_layers So we have a material with 3 layers, with local changes for only 2 layers, and a parent that has no layers. This is wrong, and causes an assertion failure and crash. At rough guess there are two separate Cogl problems here: - If you have a material with complicated layer state, changing the primary color of the material should *not* result in the parent getting reset; it should just record a local difference on the material. - If you are changing the parent, then you have to copy the layer state from the old parent to the material itself. (Maybe there's no case in which you want to change the parent when there are layers being inherited from the parent.) Changing back to the way Florian had it originally (using the layer color rather the primary color) should work around this problem.
Created attachment 170907 [details] [review] StThemeNodeTransition: work around Cogl bug Switch to using the layer combine constant rather than the material primary color for representing the opacity of the material; this avoids triggering a Cogl bug where changing the primary color corrupts the layer state.
Review of attachment 170907 [details] [review]: Code looks good. I cannot reproduce the bug here (x86_64 only?), but Maxim confirmed on IRC that the fix works.
Comment on attachment 170907 [details] [review] StThemeNodeTransition: work around Cogl bug Workaround pushed as 8f0f159960d1db92af35ebc6c144b3d1844f43b6 (From my debugging, I didn't see any particular reason it would be x86_64 specific - probably more specific sequence of operations - it occurs reliably when an app is launched with the keyboard (cal<return>) or whatever. I don't think it has anything at all to do with the app that is launched. It is conceivable that it is some 32/64 bit problem inside Cogl, I guess.)
I've just pushed the following fix to Clutter that I think should fix the root cause of this bug: Thanks to Owen's investigation that lead to the problem. commit f834b8b138f1f405a7afa7e56560e4014f8bb990 Author: Robert Bragg <robert@linux.intel.com> Date: Thu Sep 23 22:18:42 2010 +0100 material: Don't prune ancestry if it owns some layers Each time a material property changes we look to see if any of its ancestry has become redundant and if so we prune that redundant ancestry. There was a problem with the logic that handles this though because we weren't considering that a material which is a layer state authority may still defer to ancestors to define the state of individual layers. For example a material that derives from a parent with 5 layers can become a STATE_LAYERS authority by simply changing it's ->n_layers count to 4 and in that case it can still defer to its ancestors to define the state of those 4 layers. This patch checks first if a material is a layer state authority and if so only tries to prune its ancestry if it also *owns* all the individual layers it depends on. (I.e. if g_list_length (material->layer_differences) != material->n_layers then it's not safe to try pruning its ancestry!) http://bugzilla-attachments.gnome.org/attachment.cgi?id=170907
(In reply to comment #11) > I've just pushed the following fix to Clutter that I think should fix the root > cause of this bug: It indeed does. Owen should we revert the workaround then?
Created attachment 171002 [details] [review] Revert "StThemeNodeTransition: work around Cogl bug" This reverts commit 8f0f159960d1db92af35ebc6c144b3d1844f43b6. Cogl has been fixed, so it is no longer needed.
Review of attachment 171002 [details] [review]: Yes, we should revert the workaround - there's almost no difference, but I liked the original way a little better.
Attachment 171002 [details] pushed as 1413fa6 - Revert "StThemeNodeTransition: work around Cogl bug"