On Wed, 2018-08-08 at 17:49 +0100, Daniel Stone wrote:
> Hi Juan,
>
> On Wed, 8 Aug 2018 at 17:40, Juan A. Suarez Romero <jasua...@igalia.com>
> wrote:
> > On Wed, 2018-08-08 at 17:21 +0100, Daniel Stone wrote:
> > > On Thu, 2 Aug 2018 at 10:02, Juan A. Suarez Romero <jasua...@igalia.com>
> > > wrote:
> > > > If color buffer is locked, do not set its wayland buffer to NULL;
> > > > otherwise it can not be freed later.
> > >
> > > It can: see the 'if (i == ARRAY_SIZE(...))' branch inside
> > > wl_buffer_release.
> >
> > I think I didn't explain wrongly :)
> >
> > If color buffer is locked, we set color_buffer.wl_buffer to NULL, and thus
> > we
> > can't free wl_buffer later.
>
> If a surface is resized, we will orphan all its wl_buffers by clearing
> their pointers to NULL, hence losing the ability to directly free
> them. But when a new buffer has been attached and displayed, a
> 'release' event will be delivered for the old buffer (handled by
> wl_buffer_release as an event listener), which will detect that the
> wl_buffer is not in the list and should be immediately destroyed.
>
> In fact, I have to rescind my R-b since I'm pretty sure this breaks resizing:
> - buffer 1 is allocated with original size, committed to server
> (locked == true)
> - user resizes surface, release_buffers() is called but leaves
> wl_buffer intact in color_buffers list
> - buffers 2..4 are allocated with new size and committed to server
> (locked == true)
> - release event for buffer 1 is delivered, locked = false
> - get_back_bo() finds buffer 1 has a wl_buffer and is not locked,
> but dri_image is NULL so a new image is created
> - swap_buffers_with_damage() finds buffer 1 (with new DRIimage)
> still has wl_buffer (old) and attaches that buffer
>
> So we need to either find a way to still destroy the wl_buffer inside
> wl_buffer_release(), or at least do it later, e.g. in get_back_bo().
>
Investigating a bit more, I found the following:
1) dri2_wl_release_buffers() is invoked; one of the color_buffers is locked, so
the proper wl_buffer is not destroyed, but set to NULL. This will be destroyed
later through the release event.
2) Test is finished, and dri2_wl_destroy_surface() is invoked. This function
destroys all wl_buffers, no matter if they are locked or not. But can't destroy
the above one, because it is NULL.
More important, dri2_wl_destroy_surface() frees the surface itself, including
the event queue [wl_event_queue_destroy(dri2_surf->wl_queue);]. So when the
release event for the wl_buffer, there's no wl_queue.
A quick check of what would happen if the queue is not destroyrf reveald that
the tests pass without any problem. Of course, I don't think this is a proper
solution, as we would be leaking the queue itself.
J.A.
> Cheers,
> Daniel
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
