Patch for RPI crash

[Pekka Paalanen]

On Mon, 20 Oct 2014 12:36:57 +0200
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi,
> 
> On 9 October 2014 10:37, Yves De Muyter <yves at alfavisio.be> wrote:
> 
> > Here is a fix for an issue that pops up here when I close a window on a
> > Raspberry-pi that is using GLES2 to draw contents.
> > It also reproducible with the clients/simple-egl application by simply
> > pressing 'esc' on the window (closes the window) or doing 'ctrl-c' on
> > the terminal. The result is a Weston crash.
> >
> > What happens is that the buffer is set to NULL while still used.
> > The patch is against current master.

Do you perhaps have some sort of window closing animation in use?

So, are you getting rpir_view_computer_rects() called from
rpir_view_dmx_add() or rpir_view_dmx_move()?

Seeing the whole backtrace would help pinpointing the right place to
add the check, but I think I have a fair guess on what happens.

> Hmm, I don't think this is quite the right patch ... more a workaround than
> solving the root problem.
> 
> It seems like the correct patch would be to set surface->buffer_type to
> BUFFER_TYPE_NULL in, e.g., rpi_renderer_attach, and making sure it's not
> even considered by the draw stack to begin with. If we ever get to draw a
> surface with no buffer, we've screwed up somewhere.
> 
> Unfortunately I don't have a RPi with me, so can't test any of this.

Hmm, yeah. If the wl_buffer gets destroyed for any reason, be it client
exit or client explicitly destroying it, we should just skip the
surface in repaint completely.

I don't remember the code too well anymore, but I have a feeling the
check should be in rpir_view_update(). It should cause the same path to
be taken, when 'obscured' is true. That way we properly remove the
DispmanX Element from the screen, when we have nothing to show on it.

This naturally breaks any window closing animations, but I don't think
that can be helped, because we don't know if the client is going to
re-use the backing storage of the wl_buffer for something. In the
unlikely event that it does, we'd see garbage during the animation.
OTOH, is skipping the closing animation any better... I'm not sure.

If we wanted to take the (very) small risk of showing garbage instead,
then we'd just need to take the size from somewhere else than the
egl_front->buffer_ref.buffer, that is, we need to save the size when
buffer is attached, not rely on reading it out later. This would be
good to do anyway.

Then again, I think the rpi-renderer and especially its experimental EGL
support are just waiting to be replaced by proper kernel DRM drivers
and the Mesa VC4 driver, but that is a long way to go still.

So if you fix the coding style in this patch (spaces are off, braces
not needed), and add a backtrace and proper commit title to the patch
message, I can push it.


Thanks,
pq