<html>
<head>
<base href="https://bugzilla.gnome.org/" />
</head>
<body><span class="vcard"><a href="page.cgi?id=describeuser.html&login=jadahl%40gmail.com" title="Jonas Ådahl <jadahl@gmail.com>"> <span class="fn">Jonas Ådahl</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - memory leak"
href="https://bugzilla.gnome.org/show_bug.cgi?id=761312">bug 761312</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #320310 status</td>
<td>none
</td>
<td>reviewed
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - memory leak"
href="https://bugzilla.gnome.org/show_bug.cgi?id=761312#c45">Comment # 45</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - memory leak"
href="https://bugzilla.gnome.org/show_bug.cgi?id=761312">bug 761312</a>
from <span class="vcard"><a href="page.cgi?id=describeuser.html&login=jadahl%40gmail.com" title="Jonas Ådahl <jadahl@gmail.com>"> <span class="fn">Jonas Ådahl</span></a>
</span></b>
<pre>Review of <span class=""><a href="attachment.cgi?id=320310&action=diff" name="attach_320310" title="wayland: stage uncommitted changes to dedicated buffer">attachment 320310</a> <a href="attachment.cgi?id=320310&action=edit" title="wayland: stage uncommitted changes to dedicated buffer">[details]</a></span> <a href='review?bug=761312&attachment=320310'>[review]</a>:
Think I've mostly managed to wrap my head around the flow now. I see a couple
of issues, but for the most part it looks good. I do however think that the two
patches before this would be better squashed into this one, since the
intermediate state isn't really an improvement (looks like we'll just always
draw on the live buffer, but maybe I'm wrong).
::: gdk/wayland/gdkwindow-wayland.c
@@ +627,3 @@
}
+static const cairo_user_data_key_t gdk_wayland_cairo_key;
Please call this something other than the gdk_wayland_cairo_key in
gdkdisplay-wayland.c.
@@ +668,3 @@
+ * the old committed buffer again.
+ */
+ impl->staging_cairo_surface = g_steal_pointer
(&impl->committed_cairo_surface);
If we get a wl_buffer.release after a ..update_size() we'd set the staging
cairo surface to the committed surface which has the wrong size.
Simply unsetting impl->committed_cairo_surface on update_size() should fix that
I suppose, but you'd also have to unset the user_data so it can destroy
gracefully even on GdkWindow tear down.
Doing that I think could just drop impl->backfill_cairo_surface and just always
use impl->committed_surface (and always "detach" it i.e. unset user_data) when
we've read back its content. Not sure that's better than using a separate
reference ala fillback_cairo_surface though.
@@ +1961,3 @@
+ g_clear_pointer (&impl->staging_cairo_surface, cairo_surface_destroy);
+ g_clear_pointer (&impl->backfill_cairo_surface, cairo_surface_destroy);
Must we not unset the cairo surface's user data, and handle a post-GdkWindow
destruction wl_buffer.release event? Now, if the GdkWindow is destroyed before
the wl_buffer.release of the committed buffer it'll fetch a pointer pointing to
freed memory.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>