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 791809 - Recording the last frame in g-s crashes mutter if not happening during paint
Recording the last frame in g-s crashes mutter if not happening during paint
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
3.27.x
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2017-12-20 00:46 UTC by Marco Trevisan (Treviño)
Modified: 2017-12-20 11:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
stage: Push framebuffer before setting up viewport (1.45 KB, patch)
2017-12-20 00:51 UTC, Marco Trevisan (Treviño)
committed Details | Review
stage: Push framebuffer before setting up viewport (1.45 KB, patch)
2017-12-20 10:57 UTC, Marco Trevisan (Treviño)
committed Details | Review

Description Marco Trevisan (Treviño) 2017-12-20 00:46:51 UTC
When using the EasyScreencast (this doesn't happen when calling the Stop method from d-feet for example) to record a screencast, once I hit "Stop" g-s crashes, here there are the JS and C traces:

Dec 20 00:57:45 ubuntu org.gnome.Shell.desktop[121748]: == Stack trace for context 0x563fbe3de000 ==
Dec 20 00:57:45 ubuntu org.gnome.Shell.desktop[121748]: #0 0x563fbe520080 i   resource:///org/gnome/shell/ui/screencast.js:87 (0x7f040453ac48 @ 91)
Dec 20 00:57:45 ubuntu org.gnome.Shell.desktop[121748]: #1 0x7ffdabb501f0 I   resource:///org/gnome/gjs/modules/_legacy.js:82 (0x7f04240b4de0 @ 71)
Dec 20 00:57:45 ubuntu org.gnome.Shell.desktop[121748]: #2 0x563fbe51ffd0 i   resource:///org/gnome/shell/ui/screencast.js:167 (0x7f040453ae68 @ 36)
Dec 20 00:57:45 ubuntu org.gnome.Shell.desktop[121748]: #3 0x7ffdabb50da0 I   resource:///org/gnome/gjs/modules/_legacy.js:82 (0x7f04240b4de0 @ 71)
Dec 20 00:57:45 ubuntu org.gnome.Shell.desktop[121748]: #4 0x563fbe51ff08 i   resource:///org/gnome/gjs/modules/overrides/Gio.js:300 (0x7f04240dff78 @ 614)
Dec 20 00:57:45 ubuntu org.gnome.Shell.desktop[121748]: #5 0x563fbe51fe60 i   resource:///org/gnome/gjs/modules/overrides/Gio.js:331 (0x7f04240e3230 @ 34)

  • #1 cogl_set_viewport
    at /media/M2/GNOME/mutter/cogl/cogl/cogl.c line 182
  • #2 _clutter_stage_maybe_setup_viewport
    at /media/M2/GNOME/mutter/clutter/clutter/clutter-stage.c line 3640
  • #3 capture_view
    at /media/M2/GNOME/mutter/clutter/clutter/clutter-stage.c line 4729
  • #4 clutter_stage_capture
    at /media/M2/GNOME/mutter/clutter/clutter/clutter-stage.c line 4802
  • #5 recorder_record_frame
    at ../../gnome-shell/src/shell-recorder.c line 435
  • #6 shell_recorder_close
    at ../../gnome-shell/src/shell-recorder.c line 1589



This seems similar to this downstream bug https://bugzilla.redhat.com/show_bug.cgi?format=multiple&id=1504062
Comment 1 Marco Trevisan (Treviño) 2017-12-20 00:51:02 UTC
Created attachment 365776 [details] [review]
stage: Push framebuffer before setting up viewport

When capture_view* functions are called with the paint flag set
to TRUE, we need to setup the framebuffer, however this was
happening after setting up the viewport, while the viewport
needs the framebuffer to be valid when calling cogl_set_viewport.
Comment 2 Jonas Ådahl 2017-12-20 03:25:28 UTC
Review of attachment 365776 [details] [review]:

Ugh, make sense. I'd really like for us to stop using all of the old deprecated cogl_* functions that rely on framebuffers pushed on a stack.

An alternative is to have maybe_setup_viewport() to call cogl_framebuffer_set_viewport(), but that I guess will just be "for good measure" since the viewport actually used here and there by clutter is the one actually pushed.
Comment 3 Marco Trevisan (Treviño) 2017-12-20 10:57:28 UTC
The following fix has been pushed:
commit 2b60fb0 stage: Push framebuffer before setting up viewport
Comment 4 Marco Trevisan (Treviño) 2017-12-20 10:57:39 UTC
Created attachment 365790 [details] [review]
stage: Push framebuffer before setting up viewport

When capture_view* functions are called with the paint flag set
to TRUE, we need to setup the framebuffer, however this was
happening after setting up the viewport, while the viewport
needs the framebuffer to be valid when calling cogl_set_viewport.
Comment 5 Marco Trevisan (Treviño) 2017-12-20 11:02:07 UTC
(In reply to Jonas Ådahl from comment #2)
> Review of attachment 365776 [details] [review] [review]:
> 
> Ugh, make sense. I'd really like for us to stop using all of the old
> deprecated cogl_* functions that rely on framebuffers pushed on a stack.
> 
> An alternative is to have maybe_setup_viewport() to call
> cogl_framebuffer_set_viewport(), but that I guess will just be "for good
> measure" since the viewport actually used here and there by clutter is the
> one actually pushed.

Yeah, I was also thinking about using cogl_framebuffer_set_viewport instead as it will make things more clear, maybe passing the fb... Let me know if you've any other code cleanup ideas.