GNOME Bugzilla – Bug 789961
[patch] crashes in cogl when making a youtube video fullscreen on EGL/GLES/X11 platform
Last modified: 2018-02-05 11:25:49 UTC
Created attachment 363052 [details] [review] cogl-texture: refuse downloading textures in cogl_texture_get_data() on GLES This is a crash we have observed on Endless OS, which uses gnome-shell as its default shell, on an ARM SoC with a Mali 450 GPU, running X11 and using the EGL/GLES libraries provided by ARM for Mali. The crash happens when one tries to make a youtube video full-screen (from the youtube player's controls), while this video is playing inside a *non-maximized* chromium window. The crash apparently originates somewhere in javascript code (therefore making it impossible for me to trace in GDB), but it ends up crashing in Cogl code. The backtrace of gnome-shell shows:
+ Trace 238142
It seems that meta_shaped_texture_get_image in this context is trying to blend two images together, where one of them is in a texture and needs to be downloaded from the texture back on a cairo surface. However, this operation is not directly supported in GLES, so cogl has a fallback in cogl_texture_get_data that draws the texture on a framebuffer object (FBO) and then reads it back from there. This requires an "active" FBO in the context. The crash happens because the "active" FBO is NULL, most likely because no code has set an active FBO earlier. In order to fix this, I created a patch that basically removes this fallback operation and just refuses to download the content of the texture. This is in line with a 2012 patch in cogl-2.0 that does the same thing, because the "active" FBO API is deprecated (and obviously broken, as it is not obvious to the caller that it needs to set an active FBO before this function call **if the backend is GLES** ?!). It works like a charm, without any visible artifacts. The upstream cogl-2.0 patch is here: https://git.gnome.org/browse/cogl/commit/cogl/cogl-texture.c?id=6d6a277b8e9a63a8268046e5258877ba94a1da5b My patch is attached.
Created attachment 363053 [details] [review] cogl-texture: refuse downloading textures in cogl_texture_get_data() on GLES Sorry, forgot to include the comment changes in the patch.
Ping? This patch seems to make sense to my uninformed eyes considering it's basically a backport of the patch in cogl upstream at https://git.gnome.org/browse/cogl/commit/cogl/cogl-texture.c?id=6d6a277b8e9a63a8268046e5258877ba94a1da5b. I'm rebasing mutter 3.26 now for Endless and have no problem to backport it again, but it feels like this one might actually be a good candidate to land upstream, if Jonas/Rui/others agree.
Review of attachment 363053 [details] [review]: The commit message needs some reasoning why this is needed. Also, with gnome-3-26 you should be able to see the Javascript backtrace by adding "backtrace-segfaults" to SHELL_DEBUG. Anyhow, not relying on implicitly sometimes drawing on some fb that happens to be on top of the stack seems like a good idea to me.
Created attachment 366730 [details] [review] cogl-texture: refuse downloading textures in cogl_texture_get_data() on GLES Updated commit message
Ping? @Florian, let me know if you want me to move this one to gitlab and propose a MR there (keeping authorship, of course), thanks
Review of attachment 366730 [details] [review]: lgtm
Thanks for the review. I've just pushed it to master now: https://gitlab.gnome.org/GNOME/mutter/commit/d8f2f583