After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 789961 - [patch] crashes in cogl when making a youtube video fullscreen on EGL/GLES/X11 platform
[patch] crashes in cogl when making a youtube video fullscreen on EGL/GLES/X1...
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2017-11-06 11:14 UTC by George Kiagiadakis
Modified: 2018-02-05 11:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
cogl-texture: refuse downloading textures in cogl_texture_get_data() on GLES (1.56 KB, patch)
2017-11-06 11:14 UTC, George Kiagiadakis
none Details | Review
cogl-texture: refuse downloading textures in cogl_texture_get_data() on GLES (1.94 KB, patch)
2017-11-06 11:19 UTC, George Kiagiadakis
none Details | Review
cogl-texture: refuse downloading textures in cogl_texture_get_data() on GLES (2.62 KB, patch)
2018-01-12 14:26 UTC, George Kiagiadakis
accepted-commit_now Details | Review

Description George Kiagiadakis 2017-11-06 11:14:09 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:

  • #0 _cogl_texture_draw_and_read
    at cogl-texture.c line 676
  • #1 cogl_texture_get_data
    at cogl-texture.c line 1165
  • #2 meta_shaped_texture_get_image
    at compositor/meta-shaped-texture.c line 973
  • #3 shell_util_get_content_for_window_actor
    at shell-util.c line 436
  • #4 ffi_call_VFP

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.
Comment 1 George Kiagiadakis 2017-11-06 11:19:24 UTC
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.
Comment 2 Mario Sánchez Prada 2018-01-08 17:57:44 UTC
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.
Comment 3 Jonas Ådahl 2018-01-10 03:07:59 UTC
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.
Comment 4 George Kiagiadakis 2018-01-12 14:26:13 UTC
Created attachment 366730 [details] [review]
cogl-texture: refuse downloading textures in cogl_texture_get_data() on GLES

Updated commit message
Comment 5 Mario Sánchez Prada 2018-02-01 18:01:19 UTC
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
Comment 6 Jonas Ådahl 2018-02-02 03:50:36 UTC
Review of attachment 366730 [details] [review]:

lgtm
Comment 7 Mario Sánchez Prada 2018-02-05 11:25:49 UTC
Thanks for the review. I've just pushed it to master now:
https://gitlab.gnome.org/GNOME/mutter/commit/d8f2f583