GNOME Bugzilla – Bug 787715
Screen cast stream properties and screen capture bug fix
Last modified: 2017-09-20 10:30:13 UTC
Two patches will accompany this bug: 1. Add a properties bag to org.gnome.Mutter.ScreenCast.Stream This is technically an API change, but this since the screen cast API is still experimental, and the fact that it's purely an addition, I'd argue that it's within reason. 1. Fix capturing on non-origin monitor There was a bug in the capture_into() code causing memory corruption when capturing from a non-origin stage view. It's an API change, but this function is only used by the screen cast code, and not exposed outside of mutter, so should also be fine.
Created attachment 359836 [details] [review] screen-cast-stream: Add stream properties For monitor streams, add position and size (in compositor coordinate space) properties.
Created attachment 359837 [details] [review] clutter: Fix capture_into on non-origin view The capture_into() function wrote out of bounds when capturing from a non-origin view (not positioned at (0,0)). At the time of implementation, this API was used to capture pixels across views into a single data buffer, but the way it's used, and the way views may work now, makes this impossible to do properly. So remove the ability to capture into a pre-allocated buffer from multiple views, and complain if the passed rectantgle overlapps with multiple views. This removes the broken offset calculation all together, fixing the bug motivating this change.
Review of attachment 359836 [details] [review]: just a naming nit but looks fine ::: src/org.gnome.Mutter.ScreenCast.xml @@ +114,3 @@ + coordinate space. + --> + <property name="Properties" type="a{sv}" access="read" /> maybe call this Parameters? it gets a bit confusing since this is also a DBus property and a GObject property called... properties.
Review of attachment 359837 [details] [review]: ::: clutter/clutter/clutter-stage.c @@ +4882,3 @@ + g_warn_if_fail (view_capture_rect.width == rect->width && + view_capture_rect.height == rect->height); why warn if it works? if this is a mutter bug we should assert @@ +4899,3 @@ + + view = get_view_at_rect (stage, rect); + g_return_if_fail (view); same, if it's a mutter bug we should assert otherwise just return silently
(In reply to Rui Matos from comment #3) > Review of attachment 359836 [details] [review] [review]: > > just a naming nit but looks fine > > ::: src/org.gnome.Mutter.ScreenCast.xml > @@ +114,3 @@ > + coordinate space. > + --> > + <property name="Properties" type="a{sv}" access="read" /> > > maybe call this Parameters? it gets a bit confusing since this is also a > DBus property and a GObject property called... properties. Good point. (In reply to Rui Matos from comment #4) > Review of attachment 359837 [details] [review] [review]: > > ::: clutter/clutter/clutter-stage.c > @@ +4882,3 @@ > > + g_warn_if_fail (view_capture_rect.width == rect->width && > + view_capture_rect.height == rect->height); > > why warn if it works? if this is a mutter bug we should assert > > @@ +4899,3 @@ > + > + view = get_view_at_rect (stage, rect); > + g_return_if_fail (view); > > same, if it's a mutter bug we should assert otherwise just return silently Good point. I'll remove this (it'd segfault nicely anyway), and change the one above to an assert.
Attachment 359837 [details] pushed as ba194bd - clutter: Fix capture_into on non-origin view