<html>
<head>
<base href="https://bugzilla.gnome.org/" />
</head>
<body>
<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#c23">Comment # 23</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>(In reply to Jonas Ådahl from <a href="show_bug.cgi?id=761312#c22">comment #22</a>)
<span class="quote">> Review of <span class=""><a href="attachment.cgi?id=320235&action=diff" name="attach_320235" title="wayland: stage uncommited changes to dedicated buffer">attachment 320235</a> <a href="attachment.cgi?id=320235&action=edit" title="wayland: stage uncommited changes to dedicated buffer">[details]</a></span> <a href='review?bug=761312&attachment=320235'>[review]</a> [review]:
>
> ::: gdk/wayland/gdkwindow-wayland.c
> @@ +404,3 @@
> + cairo_surface_destroy (impl->commited_surface);
> + impl->commited_surface = NULL;
> + }
>
> Shouldn't we do this on begin_paint instead? We might receive a .release
> after .frame but before we enter the paint loop.
>
> @@ +483,3 @@
> + g_warning ("Received buffer release notification for untracked
> buffer");
> + return;
> + }
>
> Is this true branch block really an error? If we
>
> 1. commit a buffer A
> 2. receive .frame
> 3. commit a buffer B
> 4. receive .frame
> 5. receive A.release
>
> At 5. impl->commited will be NULL, and it wouldn't be an error.
>
> The same would happen if we create and read back on .begin_paint.
>
> 1. draw frame on A
> 2. commit a buffer A
> 3. draw frame on B
> 4. commit a buffer B
> 5. receive A.release
>
> At 5. impl->commited would also be NULL, and it wouldn't be an error.
> </span >
Actually, it wouldn't since committed would only be NULL during paint, and we
wouldn't handle events during painting. We still need to handle the window tear
down situation though, i.e. a .release after the window was destroyed.
<span class="quote">> I think what we need to do is something like:
>
> cairo_surface_t *cairo_surface = wl_buffer_get_user_data (wl_buffer);
>
> /* Destroy any old released cairo surface we no longer will re-use. */
> if (impl->commited_surface != cairo_surface)
> {
> cairo_surface_destroy (cairo_surface);
> return;
> }
>
> if (impl->staging_surface == NULL)
> impl->staging_surface = cairo_surface;
> else
> cairo_surface_destroy (cairo_surface);
>
> impl->commited_surface = NULL;
> </span >
And I just realized you replaced the buffer listener, so this won't work
either. So I suppose what needs to be done is to add the GdkWindow the cairo
surface is attached to (assuming we don't attach the same cairo surface to
multiple GdkWindow's), and set the wl_buffer user data back to the associated
cairo_surface_t. That seems to me the most reasonable way to handle the above
described situation.</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>