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 760897 - A few clean ups to the shm handling code
A few clean ups to the shm handling code
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Wayland
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-01-20 17:56 UTC by Ray Strode [halfline]
Modified: 2016-01-20 19:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
wayland: unlink shm file earlier in create function (2.69 KB, patch)
2016-01-20 17:57 UTC, Ray Strode [halfline]
committed Details | Review
wayland: clean up stride calculation when creating shm surface (3.86 KB, patch)
2016-01-20 17:57 UTC, Ray Strode [halfline]
committed Details | Review
wayland: don't pass in width and height to create_shm_pool (5.10 KB, patch)
2016-01-20 17:57 UTC, Ray Strode [halfline]
committed Details | Review

Description Ray Strode [halfline] 2016-01-20 17:56:55 UTC
Here's a few clean ups to the shm handling code I found,
when skimming through the code.
Comment 1 Ray Strode [halfline] 2016-01-20 17:57:03 UTC
Created attachment 319452 [details] [review]
wayland: unlink shm file earlier in create function

create_shm_pool unlinks the temporary file a little,
too late. It should be unlinked before ftruncate()
is called for two reasons:

1) if ftruncate fails, the file is currently not
getting cleaned up at all
2) in theory, if the file is public some other process
could muck with it

This commit just moves the unlink call a little higher
up.
Comment 2 Ray Strode [halfline] 2016-01-20 17:57:08 UTC
Created attachment 319453 [details] [review]
wayland: clean up stride calculation when creating shm surface

Right now, we assume the stride for the image surface needs to
be 4 byte aligned.  This is, in fact, true, but it's better to
ask cairo for the alignment requirement directly rather than
assume we know the alignment rules.

This commit changes the code to use cairo_format_stride_for_width
to calculate a suitable rowstride for pixman.
Comment 3 Ray Strode [halfline] 2016-01-20 17:57:12 UTC
Created attachment 319454 [details] [review]
wayland: don't pass in width and height to create_shm_pool

create_shm_pool doesn't need the width or height, it just needs
the total size.  By passing it in, we're requiring it to redo
stride calculation unnecessarily.

This commit drops the width and height parameters and makes the
function just take the total size directly.
Comment 4 Matthias Clasen 2016-01-20 19:18:41 UTC
Review of attachment 319452 [details] [review]:

looks right
Comment 5 Matthias Clasen 2016-01-20 19:20:30 UTC
Review of attachment 319453 [details] [review]:

Feel free to push it with that change

::: gdk/wayland/gdkdisplay-wayland.c
@@ +1007,3 @@
   data->busy = FALSE;
 
+  stride = cairo_format_stride_for_width (format, width * scale);

Benjamin preferred to just open-code the format here, without a variable.
Comment 6 Matthias Clasen 2016-01-20 19:22:00 UTC
Review of attachment 319454 [details] [review]:

Looks good to me
Comment 7 Ray Strode [halfline] 2016-01-20 19:31:42 UTC
pushed after making that change and also making the spacing around operators consistent
with the rest of the code in the function.
Attachment 319452 [details] pushed as c8deaea - wayland: unlink shm file earlier in create function
Attachment 319453 [details] pushed as 1e001ea - wayland: clean up stride calculation when creating shm surface
Attachment 319454 [details] pushed as 5150849 - wayland: don't pass in width and height to create_shm_pool