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 711518 - mutter-wl: Support setting a NULL opaque region.
mutter-wl: Support setting a NULL opaque region.
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
git master
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2013-11-05 21:43 UTC by Andreas Heider
Modified: 2013-11-05 23:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
mutter-wl: Support setting a NULL opaque region. (2.53 KB, patch)
2013-11-05 21:43 UTC, Andreas Heider
needs-work Details | Review
mutter-wl: Support setting a NULL opaque region V2 (1.63 KB, patch)
2013-11-05 22:28 UTC, Andreas Heider
reviewed Details | Review
mutter-wl: Support setting a NULL surface regions. (2.43 KB, patch)
2013-11-05 23:08 UTC, Andreas Heider
committed Details | Review

Description Andreas Heider 2013-11-05 21:43:03 UTC
Created attachment 259047 [details] [review]
mutter-wl: Support setting a NULL opaque region.

According to the wayland spec (A.14.1.5. wl_surface::set_opaque_region),
setting a NULL opaque region is possible and should case the pending
opaque region to be set to empty. This implements the required
behavoir similar to how weston does it.

Previously the weston-simple-egl demo client caused mutter-wayland to
crash with a segfault in meta_wayland_surface_set_opaque_region, with
this patch it works as intended.

This is my first patch to mutter/gnome, so please tell me if there is anything I could do better when submitting patches.
Comment 1 Jasper St. Pierre (not reading bugmail) 2013-11-05 21:48:00 UTC
Review of attachment 259047 [details] [review]:

::: src/wayland/meta-wayland-surface.c
@@ +220,3 @@
+  else
+    {
+      empty_region (&surface->pending.opaque_region);

This doesn't make sense to me. First of all, surface->pending.opaque_region is already a cairo_region_t*, and this is passing a cairo_region_t**. How did this compile / work?

Also, why not simply set the opaque_region to NULL? That should work perfectly fine.
Comment 2 Andreas Heider 2013-11-05 22:28:30 UTC
Created attachment 259048 [details] [review]
mutter-wl: Support setting a NULL opaque region V2

Implements the review suggestion of simply setting the region to NULL, and this time I double checked that I actually upload the right version of the patch.
Comment 3 Jasper St. Pierre (not reading bugmail) 2013-11-05 22:44:01 UTC
Review of attachment 259048 [details] [review]:

Can you fix input_region too while you're at it? This looks correct.
Comment 4 Andreas Heider 2013-11-05 23:08:20 UTC
Created attachment 259052 [details] [review]
mutter-wl: Support setting a NULL surface regions.

Also fixes set_input_region. I did a quick test that it actually works with a simple program calling "wl_surface_set_input_region(window->surface, NULL);" and it looked good to me.